Address review feedback on language override of navigator.language/s. #11922
Conversation
lutien
force-pushed
the
address-feedback-on-language-override
branch
2 times, most recently
from
25c9273 to
322cfa0
Compare
source
Outdated
| languages, ordered by preference with the most preferred language first. <ref>BCP47</ref></li> | ||
| languages, ordered by preference with the most preferred language first. The same object must be | ||
| returned until the user agent needs to return different values, or values in a different order, | ||
| or <var>emulatedLanguage</var> is updated. <ref>BCP47</ref></li> |
There was a problem hiding this comment.
To do this properly we have to store the object we intend to return on Navigator in an associated concept of sorts. And then each time we run this algorithm we see if any of the values changed, and if not, we return the cached object. And if they have changed, we update the cached object and then return it.
There was a problem hiding this comment.
Hmm... I'm not so sure how to specify the storing of the value 😕 Like where I could keep it. I couldn't really find anything similar in the spec, only the same kind of sentences that the value should be returned until something has changed. Should I write something abstract that there should be some kind of cache for it? Could you maybe give me some pointers/examples?
There was a problem hiding this comment.
Ah okay. It would be similar to
Each Window has an associated Navigator, ...
Maybe something like:
Each object that implements NavigatorLanguage has an associated plausible languages array, which is a frozen array of BCP 47 language tags. It is initially empty.
And then in the getter algorithm you could do something like:
- Let languages be the result of computing plausible languages. (I.e., what the steps do today.)
- If languages and this's plausible languages array do not have the same values in the same order, then set this's plausible languages array to languages.
- Return this's plausible languages array.
Does that help?
There was a problem hiding this comment.
Yes! It does 🙂
I've updated the PR. Please let me know if it looks better now.
lutien
force-pushed
the
address-feedback-on-language-override
branch
3 times, most recently
from
841b0a3 to
5446bae
Compare
lutien
force-pushed
the
address-feedback-on-language-override
branch
from
5446bae to
d8a55d5
Compare
lutien
force-pushed
the
address-feedback-on-language-override
branch
from
d8a55d5 to
a483bd7
Compare
annevk
left a comment
annevk
left a comment
There was a problem hiding this comment.
This looks pretty good to me!
I realized there's another minor editorial issue. Normally we use explicit <p>s inside <li>s. So it's always <li><p> for each step. Can you change both algorithms to make use of that convention as well?
Maybe @zcorpan should also review since he did the initial round.
|
|
||
| <div algorithm> | ||
| <p>Each object that implements <code>NavigatorLanguage</code> has | ||
| an associated <dfn attribute for="NavigatorLanguage">plausible languages array</dfn>, |
There was a problem hiding this comment.
| an associated <dfn attribute for="NavigatorLanguage">plausible languages array</dfn>, | |
| an associated <dfn for="NavigatorLanguage">plausible languages array</dfn>, |
| <ol> | ||
| <li>Let <var>emulatedLanguage</var> be the <span>WebDriver BiDi emulated language</span> for | ||
| <span>this</span>'s <span>relevant settings object</span>.</li> | ||
| <li>Let <var>languages</var> be an empty array.</li> |
There was a problem hiding this comment.
| <li>Let <var>languages</var> be an empty array.</li> | |
| <li>Let <var>languages</var> be null.</li> |
(It's always initialized below, so better to have a dummy value.)
| languages, ordered by preference with the most preferred language first. <ref>BCP47</ref></li> | ||
| </ol> | ||
| <li>If <var>emulatedLanguage</var> is not null, then set <var>languages</var> to | ||
| <span>frozen array</span> containing <var>emulatedLanguage</var>.</li> |
There was a problem hiding this comment.
a frozen array* (applies below as well)
| <div algorithm> | ||
| <p>Each object that implements <code>NavigatorLanguage</code> has | ||
| an associated <dfn attribute for="NavigatorLanguage">plausible languages array</dfn>, | ||
| which is a frozen array of BCP 47 language tags. It is initially empty.</p> |
There was a problem hiding this comment.
Link frozen array here as you did below?
2 participants
Addresses post-merge review comments on #11793
/system-state.html ( diff )