Bug 1249742 - Don't set prefs at all if overridden by lang or locale; r=mixedpuppy

browser/components/distribution.js

@@ -377,34 +377,50 @@ DistributionCustomizer.prototype = {
     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 */ }
       }
     }

browser/components/tests/unit/distribution.ini

@@ -15,18 +15,21 @@ 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"

browser/components/tests/unit/test_distribution.js

@@ -71,11 +71,14 @@ add_task(function* () {
   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");
 });