Closed Bug 689702 Opened 9 years ago Closed 9 years ago

Changing localepicker getLocales pref should not require a restart

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 10

People

(Reporter: wesj, Assigned: wesj)

References

Details

Attachments

(2 files)

We are caching the value of "extensions.getLocales.get.url" in LocaleRepository.jsm. That means when it changes, you have to restart your browser to get the new value. Users/testers shouldn't have to do that.
Blocks: 689706
Attached patch Patch and testSplinter Review
Instead of caching, we can just query this every time I think. This isn't going to be called THAT often.

This also adds some tests for the locale repository (not the picker... yet), which I realized as I type this, only kinda sorta actually test this exact bug. I can update them.. but I think I'd rather do it in a separate patch. I'd also like to test passing in a build id, and this provides a nice base for that.

Found two other bugs in the process. One when an invalid url is specified in the locales file, and another with us calling a non-existent "WARN" function (Stolen from the AddonRepository code). I changed that to a this.log call, which is turned off here right now.

If you'd rather get it all in one, feel free to cancel this and I'll get to it in a bit.
Assignee: nobody → wjohnston
Attachment #563164 - Flags: review?(mark.finkle)
This tests that changing the pref works. I'm dynamically building the list, so changing the pref changes what is returned.
Attachment #563209 - Flags: review?(mark.finkle)
Attachment #563164 - Flags: review?(mark.finkle) → review+
Comment on attachment 563209 [details] [diff] [review]
Test for changing pref


>diff --git a/mobile/chrome/tests/locales_list.sjs b/mobile/chrome/tests/locales_list.sjs

Looks like you have a lot of 1 space indents in this file. Can you make them 2 spaces?
Attachment #563209 - Flags: review?(mark.finkle) → review+
Comment on attachment 563164 [details] [diff] [review]
Patch and test

Noming for Aurora.

This patch is mostly a test. The fix is simple (we just recheck the pref each time the localerepository.getlocales is called), and will save people on aurora testing this feature out for us an annoying restart with no restart prompt.
Attachment #563164 - Flags: approval-mozilla-aurora?
Comment on attachment 563209 [details] [diff] [review]
Test for changing pref

Noming for Aurora.

purely a test for this fix.
Attachment #563209 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/8810d1986de2
https://hg.mozilla.org/mozilla-central/rev/239a1408d5a0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 10
Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20111004
Firefox/10.0a1 Fennec/10.0a1
Device: Samsung Galaxy S
OS: Android 2.2
Status: RESOLVED → VERIFIED
Attachment #563164 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Attachment #563209 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.