Bug 1249742 - Don't set prefs at all if overridden by lang or locale; r=mixedpuppy
authorMichael Kaply <mozilla@kaply.com>
Fri, 26 Feb 2016 14:47:53 -0600
changeset 322099 b5c0cd56381547fe527c724d86eb955c209e0a6e
parent 322098 36d6bc68fe0f21d87b306c7712e939d8ae537b88
child 322100 5592b9a4cad6e73e5f502a83f3d895300dd896e1
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1249742
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1249742 - Don't set prefs at all if overridden by lang or locale; r=mixedpuppy
browser/components/distribution.js
browser/components/tests/unit/distribution.ini
browser/components/tests/unit/test_distribution.js
--- a/browser/components/distribution.js
+++ b/browser/components/distribution.js
@@ -372,44 +372,60 @@ DistributionCustomizer.prototype = {
 
     // We eval() the localizable prefs as well (even though they'll
     // always get set as a string) to keep the INI format consistent:
     // string prefs always need to be in quotes
 
     let localizedStr = Cc["@mozilla.org/pref-localizedstring;1"].
       createInstance(Ci.nsIPrefLocalizedString);
 
-    if (sections["LocalizablePreferences"]) {
-      for (let key of enumerate(this._ini.getKeys("LocalizablePreferences"))) {
+    var usedLocalizablePreferences = [];
+
+    if (sections["LocalizablePreferences-" + this._locale]) {
+      for (let key of enumerate(this._ini.getKeys("LocalizablePreferences-" + this._locale))) {
         try {
-          let value = eval(this._ini.getString("LocalizablePreferences", key));
-          value = value.replace(/%LOCALE%/g, this._locale);
-          value = value.replace(/%LANGUAGE%/g, this._language);
-          localizedStr.data = "data:text/plain," + key + "=" + value;
-          defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          let value = eval(this._ini.getString("LocalizablePreferences-" + this._locale, key));
+          if (value !== undefined) {
+            localizedStr.data = "data:text/plain," + key + "=" + value;
+            defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          }
+          usedLocalizablePreferences.push(key);
         } catch (e) { /* ignore bad prefs and move on */ }
       }
     }
 
     if (sections["LocalizablePreferences-" + this._language]) {
       for (let key of enumerate(this._ini.getKeys("LocalizablePreferences-" + this._language))) {
+        if (usedLocalizablePreferences.indexOf(key) > -1) {
+          continue;
+        }
         try {
           let value = eval(this._ini.getString("LocalizablePreferences-" + this._language, key));
-          localizedStr.data = "data:text/plain," + key + "=" + value;
-          defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          if (value !== undefined) {
+            localizedStr.data = "data:text/plain," + key + "=" + value;
+            defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          }
+          usedLocalizablePreferences.push(key);
         } catch (e) { /* ignore bad prefs and move on */ }
       }
     }
 
-    if (sections["LocalizablePreferences-" + this._locale]) {
-      for (let key of enumerate(this._ini.getKeys("LocalizablePreferences-" + this._locale))) {
+    if (sections["LocalizablePreferences"]) {
+      for (let key of enumerate(this._ini.getKeys("LocalizablePreferences"))) {
+        if (usedLocalizablePreferences.indexOf(key) > -1) {
+          continue;
+        }
         try {
-          let value = eval(this._ini.getString("LocalizablePreferences-" + this._locale, key));
-          localizedStr.data = "data:text/plain," + key + "=" + value;
-          defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          let value = eval(this._ini.getString("LocalizablePreferences", key));
+          if (value !== undefined) {
+            value = value.replace(/%LOCALE%/g, this._locale);
+            value = value.replace(/%LANGUAGE%/g, this._language);
+            localizedStr.data = "data:text/plain," + key + "=" + value;
+            defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
+          }
         } catch (e) { /* ignore bad prefs and move on */ }
       }
     }
 
     return this._checkCustomizationComplete();
   },
 
   _checkCustomizationComplete: function DIST__checkCustomizationComplete() {
--- a/browser/components/tests/unit/distribution.ini
+++ b/browser/components/tests/unit/distribution.ini
@@ -10,23 +10,26 @@ about=Test distribution file
 distribution.test.string="Test String"
 distribution.test.string.noquotes=Test String
 distribution.test.int=777
 distribution.test.bool.true=true
 distribution.test.bool.false=false
 
 [LocalizablePreferences]
 distribution.test.locale="%LOCALE%"
-distribution.test.reset="Set"
-distribution.test.locale.set="First Set"
-distribution.test.language.set="First Set"
+distribution.test.language.reset="Preference Set"
+distribution.test.locale.reset="Preference Set"
+distribution.test.locale.set="Preference Set"
+distribution.test.language.set="Preference Set"
 
 [LocalizablePreferences-en]
 distribution.test.language.en="en"
-distribution.test.language.set="Second Set"
+distribution.test.language.reset=
+distribution.test.language.set="Language Set"
+distribution.test.locale.set="Language Set"
 
 [LocalizablePreferences-en-US]
 distribution.test.locale.en-US="en-US"
-distribution.test.reset=
-distribution.test.locale.set="Second Set"
+distribution.test.locale.reset=
+distribution.test.locale.set="Locale Set"
 
 [LocalizablePreferences-de]
 distribution.test.locale.de="de"
--- a/browser/components/tests/unit/test_distribution.js
+++ b/browser/components/tests/unit/test_distribution.js
@@ -66,16 +66,19 @@ add_task(function* () {
   Assert.throws(() => Services.prefs.getCharPref("distribution.test.string.noquotes"));
   Assert.equal(Services.prefs.getIntPref("distribution.test.int"), 777);
   Assert.equal(Services.prefs.getBoolPref("distribution.test.bool.true"), true);
   Assert.equal(Services.prefs.getBoolPref("distribution.test.bool.false"), false);
   Assert.equal(Services.prefs.getComplexValue("distribution.test.locale", Ci.nsIPrefLocalizedString).data, "en-US");
   Assert.equal(Services.prefs.getComplexValue("distribution.test.language.en", Ci.nsIPrefLocalizedString).data, "en");
   Assert.equal(Services.prefs.getComplexValue("distribution.test.locale.en-US", Ci.nsIPrefLocalizedString).data, "en-US");
   Assert.throws(() => Services.prefs.getComplexValue("distribution.test.locale.de", Ci.nsIPrefLocalizedString));
+  // This value was never set because of the empty language specific pref
+  Assert.throws(() => Services.prefs.getComplexValue("distribution.test.language.reset", Ci.nsIPrefLocalizedString));
   // This value was never set because of the empty locale specific pref
-  // This testcase currently fails - the value is set to "undefined" - it should not be set at all (throw)
-  // Assert.throws(() => Services.prefs.getComplexValue("distribution.test.reset", Ci.nsIPrefLocalizedString));
-  // This value was overriden by a locale specific setting
-  Assert.equal(Services.prefs.getComplexValue("distribution.test.locale.set", Ci.nsIPrefLocalizedString).data, "Second Set");
-  // This value was overriden by a language specific setting
-  Assert.equal(Services.prefs.getComplexValue("distribution.test.language.set", Ci.nsIPrefLocalizedString).data, "Second Set");
+  Assert.throws(() => Services.prefs.getComplexValue("distribution.test.locale.reset", Ci.nsIPrefLocalizedString));
+  // This value was overridden by a locale specific setting
+  Assert.equal(Services.prefs.getComplexValue("distribution.test.locale.set", Ci.nsIPrefLocalizedString).data, "Locale Set");
+  // This value was overridden by a language specific setting
+  Assert.equal(Services.prefs.getComplexValue("distribution.test.language.set", Ci.nsIPrefLocalizedString).data, "Language Set");
+  // Language should not override locale
+  Assert.notEqual(Services.prefs.getComplexValue("distribution.test.locale.set", Ci.nsIPrefLocalizedString).data, "Language Set");
 });