Bug 1354055 - Don't cache wrong result in OSPreferences::ReadSystemLocales on Android. r=rnewman
☠☠ backed out by 19e555dcd7e2 ☠ ☠
authorZibi Braniecki <gandalf@mozilla.com>
Thu, 06 Apr 2017 13:00:02 +0200
changeset 558340 5d3bc5f2c41fe7f8ff0c81de23ebbfc7bce7da21
parent 558339 a6efc0c4b4097edaa2a37cc8b9492617d585fcd9
child 558341 31159cce08ef418fb0626f33d31259540cac12d7
push id52860
push userbmo:walkingice0204@gmail.com
push dateFri, 07 Apr 2017 13:29:26 +0000
reviewersrnewman
bugs1354055
milestone55.0a1
Bug 1354055 - Don't cache wrong result in OSPreferences::ReadSystemLocales on Android. r=rnewman MozReview-Commit-ID: Li7wUQnC9Gz
intl/locale/android/OSPreferences_android.cpp
intl/locale/tests/unit/test_localeService.js
--- a/intl/locale/android/OSPreferences_android.cpp
+++ b/intl/locale/android/OSPreferences_android.cpp
@@ -10,20 +10,22 @@
 using namespace mozilla::intl;
 
 bool
 OSPreferences::ReadSystemLocales(nsTArray<nsCString>& aLocaleList)
 {
   //XXX: This is a quite sizable hack to work around the fact that we cannot
   //     retrieve OS locale in C++ without reaching out to JNI.
   //     Once we fix this (bug 1337078), this hack should not be necessary.
-  nsAutoCString locale;
-  if (!NS_SUCCEEDED(Preferences::GetCString("intl.locale.os", &locale)) ||
-      locale.IsEmpty()) {
-    locale.AssignLiteral("en-US");
+  //
+  //XXX: Notice, this value may be empty on an early read. In that case
+  //     we won't add anything to the return list so that it doesn't get
+  //     cached in mSystemLocales.
+  nsAdoptingCString locale = Preferences::GetCString("intl.locale.os");
+  if (!locale.IsEmpty()) {
     aLocaleList.AppendElement(locale);
   }
   return true;
 }
 
 bool
 OSPreferences::ReadDateTimePattern(DateTimeFormatStyle aDateStyle,
                                    DateTimeFormatStyle aTimeStyle,
--- a/intl/locale/tests/unit/test_localeService.js
+++ b/intl/locale/tests/unit/test_localeService.js
@@ -40,16 +40,17 @@ add_test(function test_getAppLocalesAsLa
   const enUSLocales = appLocales.filter(loc => loc === "en-US");
   do_check_true(enUSLocales.length == 1, "en-US is present exactly one time");
 
   run_next_test();
 });
 
 const PREF_MATCH_OS_LOCALE = "intl.locale.matchOS";
 const PREF_SELECTED_LOCALE = "general.useragent.locale";
+const PREF_OS_LOCALE       = "intl.locale.os";
 const REQ_LOC_CHANGE_EVENT = "intl:requested-locales-changed";
 
 add_test(function test_getRequestedLocales() {
   const requestedLocales = localeService.getRequestedLocales();
   do_check_true(Array.isArray(requestedLocales), "requestedLocales returns an array");
 
   run_next_test();
 });
@@ -62,16 +63,17 @@ add_test(function test_getRequestedLocal
  * Then, we test that when the matchOS is set to true, we will retrieve
  * OS locale from getRequestedLocales.
  */
 add_test(function test_getRequestedLocales_matchOS() {
   do_test_pending();
 
   Services.prefs.setBoolPref(PREF_MATCH_OS_LOCALE, false);
   Services.prefs.setCharPref(PREF_SELECTED_LOCALE, "ar-IR");
+  Services.prefs.setCharPref(PREF_OS_LOCALE, "en-US");
 
   const observer = {
     observe: function (aSubject, aTopic, aData) {
       switch (aTopic) {
         case REQ_LOC_CHANGE_EVENT:
           const reqLocs = localeService.getRequestedLocales();
           do_check_true(reqLocs[0] === osPrefs.systemLocale);
           Services.obs.removeObserver(observer, REQ_LOC_CHANGE_EVENT);
@@ -133,11 +135,12 @@ add_test(function test_setRequestedLocal
 
 add_test(function test_isAppLocaleRTL() {
   do_check_true(typeof localeService.isAppLocaleRTL === 'boolean');
 
   run_next_test();
 });
 
 do_register_cleanup(() => {
-    Services.prefs.clearUserPref(PREF_SELECTED_LOCALE);
-    Services.prefs.clearUserPref(PREF_MATCH_OS_LOCALE);
+  Services.prefs.clearUserPref(PREF_SELECTED_LOCALE);
+  Services.prefs.clearUserPref(PREF_MATCH_OS_LOCALE);
+  Services.prefs.clearUserPref(PREF_OS_LOCALE, "en-US");
 });