Closed Bug 1016038 Opened 6 years ago Closed 6 years ago

languagechange is emitted twice for each language change

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stas, Assigned: stas)

References

Details

Attachments

(1 file)

I wanted to start using the languagechange event introduced in bug 889335 to handle language changes in the Gaia's l10n library (bug 1013929).

However, it looks like because of how we first reset the value of intl.language_code in http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#82 (bug 796079) in order to make sure the new value is prepended to the default value of the pref, the event is emitted twice, which doesn't make it very helpful.

This happens in Firefox OS only, because we use mozSettings' language.current there.
Pike told me about getDefaultBranch() which works well here.  Vivien, can you take a look?
Attachment #8428837 - Flags: review?(21)
Attachment #8428837 - Flags: feedback?(l10n)
Comment on attachment 8428837 [details] [diff] [review]
use getDefaultBranch instead of clearUserPref

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

LGTM.
Attachment #8428837 - Flags: review?(21) → review+
Note that in the future we might want to change what value we store in language.current.  Right now it's a single locale code, but with bug 1005927, it will make sense to store the whole accept_languages string, which will make the task of updating intl.accept_languages easier.
Comment on attachment 8428837 [details] [diff] [review]
use getDefaultBranch instead of clearUserPref

Try: https://tbpl.mozilla.org/?tree=Try&rev=fc5043fcda26
Attachment #8428837 - Flags: feedback?(l10n)
I realize now that I attached the output of a diff command instead of 'hg export.' My bad.  Let me know if you'd like me to attach a new patch, with correct commit metadata.
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/450492b8afb8

I was able to get the commit information from your Try push, so that wasn't a huge deal. One request, though. Please do try to use commit messages that explain what your patch is doing rather than restating the problem it's solving. Thanks :)
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
Assignee: nobody → stas
Keywords: checkin-needed
Thanks, Ryan!  I just copy&pasted the summary of this bug, but of course, it would have made sense to reword the commit messages as per the guidelines.  I'll keep that in mind!
https://hg.mozilla.org/mozilla-central/rev/450492b8afb8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.