Last Comment Bug 1013929 - Use the languagechange event instead of a mozSettings observer
: Use the languagechange event instead of a mozSettings observer
Status: RESOLVED FIXED
:
Product: Firefox OS
Classification: Client Software
Component: Gaia::L10n (show other bugs)
: unspecified
: All Linux
-- normal (vote)
: ---
Assigned To: Zibi Braniecki [:gandalf][:zibi]
:
:
Mentors:
https://github.com/zbraniecki/l20n.js...
Depends on: 1016038
Blocks: 1016503 1018534 1033255 1033257
  Show dependency treegraph
 
Reported: 2014-05-21 06:12 PDT by Staś Małolepszy :stas
Modified: 2014-07-07 18:56 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (708 bytes, patch)
2014-05-21 17:38 PDT, Zibi Braniecki [:gandalf][:zibi]
stas: review+
stas: feedback+
Details | Diff | Splinter Review
pull request (46 bytes, text/x-github-pull-request)
2014-05-30 12:27 PDT, Zibi Braniecki [:gandalf][:zibi]
no flags Details | Review | Splinter Review

Description User image Staś Małolepszy :stas 2014-05-21 06:12:26 PDT
With bug 889335 fixed, we can stop listening to mozSettings' changes to language.current and use the new window.onlanguagechange event.
Comment 1 User image Zibi Braniecki [:gandalf][:zibi] 2014-05-21 17:38:33 PDT
Created attachment 8426696 [details] [diff] [review]
patch

What do you think Stas?
Comment 2 User image Staś Małolepszy :stas 2014-05-22 06:43:08 PDT
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/

:>
Comment 3 User image Staś Małolepszy :stas 2014-05-28 08:36:58 PDT
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 :)
Comment 4 User image Staś Małolepszy :stas 2014-05-30 05:48:16 PDT
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?
Comment 5 User image Zibi Braniecki [:gandalf][:zibi] 2014-05-30 11:54:16 PDT
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
Comment 6 User image Zibi Braniecki [:gandalf][:zibi] 2014-05-30 12:27:44 PDT
Created attachment 8431783 [details] [review]
pull request

pull request
Comment 7 User image Staś Małolepszy :stas 2014-05-30 13:00:27 PDT
Comment on attachment 8426696 [details] [diff] [review]
patch

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

Thanks, r=me assuming Travis is green.

Note You need to log in before you can comment on or make changes to this bug.