Use the languagechange event instead of a mozSettings observer

RESOLVED FIXED

Status

Firefox OS
Gaia::L10n
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: stas, Assigned: gandalf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

With bug 889335 fixed, we can stop listening to mozSettings' changes to language.current and use the new window.onlanguagechange event.
(Assignee)

Comment 1

3 years ago
Created attachment 8426696 [details] [diff] [review]
patch

What do you think Stas?
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Attachment #8426696 - Flags: feedback?(stas)
(Assignee)

Updated

3 years ago
Comment on attachment 8426696 [details] [diff] [review]
patch

Review of attachment 8426696 [details] [diff] [review]:
-----------------------------------------------------------------

I thought the patch would look exactly like this, but upon testing it turned out that the languagechange event was emitted twice: the first time navigator.language was en-US, and the second time it had the new desired value.

Here's what I see when I change the language from en-US to fr (not shown here, but I also tested navigator.language, just to be sure it's the same as the first value of navigator.languages; it was):

E/GeckoConsole( 1073): […] in l10n_langchange: navigator.languages: en-US, en
E/GeckoConsole( 1073): […] in l10n_langchange: navigator.languages: fr, en-US, en

Interestingly, here's what I see when I then change the language again, this time from fr to zh-TW:

E/GeckoConsole( 1073): […] in l10n_langchange: navigator.languages: en-US, en
E/GeckoConsole( 1073): […] in l10n_langchange: navigator.languages: zh-TW, en-US, en

Not sure why it says "en-US, en" again on the first line.

::: bindings/l20n/runtime.js
@@ +191,5 @@
>  
>  function initLocale() {
>    this.ctx.requestLocales(navigator.language);
> +  window.addEventListener('languagechange', function l10n_langchange() {
> +    navigator.mozL10n.langauge.code = navigator.language;

s/langauge/language/

:>
Attachment #8426696 - Flags: feedback?(stas) → feedback-
Depends on: 1016038
Comment on attachment 8426696 [details] [diff] [review]
patch

Review of attachment 8426696 [details] [diff] [review]:
-----------------------------------------------------------------

I got curious and dug into Gecko a little bit to see why the event was emitted twice.  Consequently, I filed and then fixed bug 1016038.  Changing my f- to f+ as the patch should work correctly now.  Let's just fix the typo before landing, please :)
Attachment #8426696 - Flags: feedback- → feedback+
As per https://groups.google.com/d/msg/mozilla.dev.gaia/LSL5d3EOPAM/_Gtmc-B1DWgJ, this will be useful for apps migrating away from being certified.  Gandalf, do you want to land this today?
Flags: needinfo?(gandalf)

Updated

3 years ago
Blocks: 1016503
(Assignee)

Comment 5

3 years ago
Comment on attachment 8426696 [details] [diff] [review]
patch

Review of attachment 8426696 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, stas. Can I get r+ on top of your f+? :)

I fixed the typo on the branch
Attachment #8426696 - Flags: review?(stas)
(Assignee)

Comment 6

3 years ago
Created attachment 8431783 [details] [review]
pull request

pull request
Flags: needinfo?(gandalf)
Comment on attachment 8426696 [details] [diff] [review]
patch

Review of attachment 8426696 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, r=me assuming Travis is green.
Attachment #8426696 - Flags: review?(stas) → review+
(Assignee)

Comment 8

3 years ago
Patch: https://github.com/mozilla-b2g/gaia/commit/8392d7ecda252cd8808b8d10eefa2c5ac94aa5bd
Merge: https://github.com/mozilla-b2g/gaia/commit/398fa8fe629e620685758734ba497277c8531d46

L20n: https://github.com/l20n/l20n.js/commit/5c06c3deeb4233c50c48953411d7df0c68f46ca3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

3 years ago
Blocks: 1018534
Blocks: 1033255
Blocks: 1033257
You need to log in before you can comment on or make changes to this bug.