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)

(Reporter)

Description

3 years ago
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
(Reporter)

Comment 2

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-
(Reporter)

Updated

3 years ago
Depends on: 1016038
(Reporter)

Comment 3

3 years ago
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+
(Reporter)

Comment 4

3 years ago
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)
(Reporter)

Comment 7

3 years ago
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.