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 309421 b5c0cd56381547fe527c724d86eb955c209e0a6e
parent 309420 36d6bc68fe0f21d87b306c7712e939d8ae537b88
child 309422 5592b9a4cad6e73e5f502a83f3d895300dd896e1
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-esr52@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1249742
milestone47.0a1
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");
 });