Bug 1462840: Remove obsolete pref-based extension metadata localization. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 18 May 2018 17:45:27 -0700
changeset 797419 6d668c2a06bd9803d101c71d8ce3676594e44216
parent 797407 9edb251d64b5813bfec517d0aa5c468ae1718240
child 797420 8bfa606dcd07da56ad37db76d7b3b7c1b453d6ea
push id110472
push usermaglione.k@gmail.com
push dateSat, 19 May 2018 00:45:59 +0000
reviewersaswan
bugs1462840
milestone62.0a1
Bug 1462840: Remove obsolete pref-based extension metadata localization. r?aswan MozReview-Commit-ID: JMlsYjHqisH
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
toolkit/mozapps/extensions/test/xpcshell/test_locale.js
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -72,17 +72,16 @@ const ZipReader = Components.Constructor
 // (Requires AddonManager.jsm)
 var logger = Log.repository.getLogger(LOGGER_ID);
 
 const KEY_PROFILEDIR                  = "ProfD";
 const FILE_JSON_DB                    = "extensions.json";
 
 const PREF_DB_SCHEMA                  = "extensions.databaseSchema";
 const PREF_EM_AUTO_DISABLED_SCOPES    = "extensions.autoDisableScopes";
-const PREF_EM_EXTENSION_FORMAT        = "extensions.";
 const PREF_PENDING_OPERATIONS         = "extensions.pendingOperations";
 const PREF_XPI_PERMISSIONS_BRANCH     = "xpinstall.";
 const PREF_XPI_SIGNATURES_DEV_ROOT    = "xpinstall.signatures.dev-root";
 
 const TOOLKIT_ID                      = "toolkit@mozilla.org";
 
 const KEY_APP_SYSTEM_ADDONS           = "app-system-addons";
 const KEY_APP_SYSTEM_DEFAULTS         = "app-system-defaults";
@@ -101,20 +100,16 @@ const PENDING_INSTALL_METADATA =
      "updateDate", "applyBackgroundUpdates", "compatibilityOverrides"];
 
 const COMPATIBLE_BY_DEFAULT_TYPES = {
   extension: true,
   dictionary: true,
   "webextension-dictionary": true,
 };
 
-// Properties that exist in the extension manifest
-const PROP_LOCALE_SINGLE = ["name", "description", "creator", "homepageURL"];
-const PROP_LOCALE_MULTI  = ["developers", "translators", "contributors"];
-
 // Properties to save in JSON file
 const PROP_JSON_FIELDS = ["id", "syncGUID", "version", "type",
                           "updateURL", "optionsURL",
                           "optionsType", "optionsBrowserStyle", "aboutURL",
                           "defaultLocale", "visible", "active", "userDisabled",
                           "appDisabled", "pendingUninstall", "installDate",
                           "updateDate", "applyBackgroundUpdates", "path",
                           "skinnable", "size", "sourceURI", "releaseNotesURI",
@@ -1129,18 +1124,18 @@ AddonWrapper = class {
     return XPIInternal.getURIForResourceInFile(addon._sourceBundle, aPath);
   }
 };
 
 function chooseValue(aAddon, aObj, aProp) {
   let repositoryAddon = aAddon._repositoryAddon;
   let objValue = aObj[aProp];
 
-  if (repositoryAddon && (aProp in repositoryAddon) &&
-      (objValue === undefined || objValue === null)) {
+  if (repositoryAddon && aProp in repositoryAddon &&
+      (aProp === "creator" || objValue == null)) {
     return [repositoryAddon[aProp], true];
   }
 
   return [objValue, false];
 }
 
 function defineAddonWrapperProperty(name, getter) {
   Object.defineProperty(AddonWrapper.prototype, name, {
@@ -1195,74 +1190,34 @@ function defineAddonWrapperProperty(name
     if (!target)
       return null;
     if (fromRepo)
       return target;
     return Services.io.newURI(target);
   });
 });
 
-function getLocalizedPref(pref) {
-  if (Services.prefs.getPrefType(pref) != Ci.nsIPrefBranch.PREF_INVALID)
-    return Services.prefs.getComplexValue(pref, Ci.nsIPrefLocalizedString).data;
-  return null;
-}
-
-PROP_LOCALE_SINGLE.forEach(function(aProp) {
+["name", "description", "creator", "homepageURL"].forEach(function(aProp) {
   defineAddonWrapperProperty(aProp, function() {
     let addon = addonFor(this);
-    // Override XPI creator if repository creator is defined
-    if (aProp == "creator" &&
-        addon._repositoryAddon && addon._repositoryAddon.creator) {
-      return addon._repositoryAddon.creator;
-    }
-
-    let result = null;
-
-    if (addon.active) {
-      try {
-        let value = getLocalizedPref(`${PREF_EM_EXTENSION_FORMAT}${addon.id}.${aProp}`);
-        if (value)
-          result = value;
-      } catch (e) {
-      }
-    }
-
-    if (result == null)
-      [result] = chooseValue(addon, addon.selectedLocale, aProp);
-
-    if (aProp == "creator")
-      return result ? new AddonManagerPrivate.AddonAuthor(result) : null;
+
+    let [result, usedRepository] = chooseValue(addon, addon.selectedLocale, aProp);
+
+    if (result && !usedRepository && aProp == "creator")
+      return new AddonManagerPrivate.AddonAuthor(result);
 
     return result;
   });
 });
 
-PROP_LOCALE_MULTI.forEach(function(aProp) {
+["developers", "translators", "contributors"].forEach(function(aProp) {
   defineAddonWrapperProperty(aProp, function() {
     let addon = addonFor(this);
-    let results = null;
-    let usedRepository = false;
-
-    if (addon.active) {
-      let pref = `${PREF_EM_EXTENSION_FORMAT}${addon.id}.${aProp.slice(0, -1)}`;
-      let list = Services.prefs.getChildList(pref, {});
-      if (list.length > 0) {
-        list.sort();
-        results = [];
-        for (let childPref of list) {
-          let value = getLocalizedPref(childPref);
-          if (value)
-            results.push(value);
-        }
-      }
-    }
-
-    if (results == null)
-      [results, usedRepository] = chooseValue(addon, addon.selectedLocale, aProp);
+
+    let [results, usedRepository] = chooseValue(addon, addon.selectedLocale, aProp);
 
     if (results && !usedRepository) {
       results = results.map(function(aResult) {
         return new AddonManagerPrivate.AddonAuthor(aResult);
       });
     }
 
     return results;
--- a/toolkit/mozapps/extensions/test/xpcshell/test_locale.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_locale.js
@@ -1,135 +1,92 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
-async function run_test() {
-  do_test_pending();
-
+add_task(async function setup() {
   // Setup for test
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
   Services.locale.setRequestedLocales(["fr-FR"]);
 
   await promiseStartupManager();
-
-  run_test_1();
-}
+});
 
 // Tests that the localized properties are visible before installation
-async function run_test_1() {
+add_task(async function test_1() {
   let install = await AddonManager.getInstallForFile(do_get_addon("test_locale"));
   Assert.equal(install.addon.name, "fr-FR Name");
   Assert.equal(install.addon.description, "fr-FR Description");
 
-  prepare_test({
-    "addon1@tests.mozilla.org": [
-      ["onInstalling", false],
-      ["onInstalled", false],
-    ]
-  }, [
-    "onInstallStarted",
-    "onInstallEnded",
-  ], callback_soon(run_test_2));
-  install.install();
-}
+  await new Promise(resolve => {
+    prepare_test({
+      "addon1@tests.mozilla.org": [
+        ["onInstalling", false],
+        ["onInstalled", false],
+      ]
+    }, [
+      "onInstallStarted",
+      "onInstallEnded",
+    ], resolve);
+    install.install();
+  });
+});
 
 // Tests that the localized properties are visible after installation
-async function run_test_2() {
+add_task(async function test_2() {
   await promiseRestartManager();
 
   let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
   Assert.notEqual(addon, null);
 
   Assert.equal(addon.name, "fr-FR Name");
   Assert.equal(addon.description, "fr-FR Description");
 
   await addon.disable();
-  executeSoon(run_test_3);
-}
+});
 
 // Test that the localized properties are still there when disabled.
-async function run_test_3() {
+add_task(async function test_3() {
   await promiseRestartManager();
 
   let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
   Assert.notEqual(addon, null);
   Assert.equal(addon.name, "fr-FR Name");
-
-  executeSoon(run_test_4);
-}
+});
 
-// Localised preference values should be ignored when the add-on is disabled
-async function run_test_4() {
-  Services.prefs.setCharPref("extensions.addon1@tests.mozilla.org.name", "Name from prefs");
-  Services.prefs.setCharPref("extensions.addon1@tests.mozilla.org.contributor.1", "Contributor 1");
-  Services.prefs.setCharPref("extensions.addon1@tests.mozilla.org.contributor.2", "Contributor 2");
+add_task(async function test_4() {
   await promiseRestartManager();
 
   let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
   Assert.notEqual(addon, null);
   Assert.equal(addon.name, "fr-FR Name");
   let contributors = addon.contributors;
   Assert.equal(contributors.length, 3);
   Assert.equal(contributors[0], "Fr Contributor 1");
   Assert.equal(contributors[1], "Fr Contributor 2");
   Assert.equal(contributors[2], "Fr Contributor 3");
-
-  executeSoon(run_test_5);
-}
+});
 
 // Test that changing locale works
-async function run_test_5() {
+add_task(async function test_5() {
   Services.locale.setRequestedLocales(["de-DE"]);
   await promiseRestartManager();
 
   let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
   Assert.notEqual(addon, null);
 
   Assert.equal(addon.name, "de-DE Name");
   Assert.equal(addon.description, null);
-
-  executeSoon(run_test_6);
-}
+});
 
 // Test that missing locales use the fallbacks
-async function run_test_6() {
+add_task(async function test_6() {
   Services.locale.setRequestedLocales(["nl-NL"]);
   await promiseRestartManager();
 
   let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
   Assert.notEqual(addon, null);
 
   Assert.equal(addon.name, "Fallback Name");
   Assert.equal(addon.description, "Fallback Description");
 
   await addon.enable();
-  executeSoon(run_test_7);
-}
-
-// Test that the prefs will override the fallbacks
-async function run_test_7() {
-  await promiseRestartManager();
-
-  let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
-  Assert.notEqual(addon, null);
-
-  Assert.equal(addon.name, "Name from prefs");
-
-  executeSoon(run_test_8);
-}
-
-// Test that the prefs will override localized values from the manifest
-async function run_test_8() {
-  Services.locale.setRequestedLocales(["fr-FR"]);
-  await promiseRestartManager();
-
-  let addon = await AddonManager.getAddonByID("addon1@tests.mozilla.org");
-  Assert.notEqual(addon, null);
-
-  Assert.equal(addon.name, "Name from prefs");
-  let contributors = addon.contributors;
-  Assert.equal(contributors.length, 2);
-  Assert.equal(contributors[0], "Contributor 1");
-  Assert.equal(contributors[1], "Contributor 2");
-
-  executeSoon(do_test_finished);
-}
+});