Backed out changeset 7e8e47c972f4 (bug 1397975) for eslint failure at browser/components/extensions/ext-chrome-settings-overrides.js:124: windowTracker is not defined. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 21 Sep 2017 22:56:07 +0200
changeset 382248 84188868fc418b5d79c218a6abc33296f57a5ad6
parent 382247 587c58121d4502556ff25d674820bd523ab313f8
child 382249 1252fd56a1c9aec98f1b52dd14fd35d6145751f0
push id32551
push userkwierso@gmail.com
push dateThu, 21 Sep 2017 23:29:53 +0000
treeherdermozilla-central@d6d6fd889f7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1397975
milestone58.0a1
backs out7e8e47c972f41335e9109c51597efdc3bca56910
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
Backed out changeset 7e8e47c972f4 (bug 1397975) for eslint failure at browser/components/extensions/ext-chrome-settings-overrides.js:124: windowTracker is not defined. r=backout
browser/base/content/popup-notifications.inc
browser/components/extensions/ext-chrome-settings-overrides.js
browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
browser/locales/en-US/chrome/browser/browser.properties
browser/modules/ExtensionsUI.jsm
--- a/browser/base/content/popup-notifications.inc
+++ b/browser/base/content/popup-notifications.inc
@@ -76,20 +76,14 @@
       <popupnotificationcontent class="addon-webext-perm-notification-content" orient="vertical">
         <description id="addon-webext-perm-header" class="addon-webext-perm-header"/>
         <description id="addon-webext-perm-text" class="addon-webext-perm-text"/>
         <label id="addon-webext-perm-intro" class="addon-webext-perm-text"/>
         <html:ul id="addon-webext-perm-list" class="addon-webext-perm-list"/>
       </popupnotificationcontent>
     </popupnotification>
 
-    <popupnotification id="addon-webext-defaultsearch-notification" hidden="true">
-      <popupnotificationcontent class="addon-webext-defaultsearch-notification-content" orient="vertical">
-        <description id="addon-webext-defaultsearch-text" class="addon-webext-perm-header"/>
-      </popupnotificationcontent>
-    </popupnotification>
-
     <popupnotification id="addon-installed-notification" hidden="true">
       <popupnotificationcontent class="addon-installed-notification-content" orient="vertical">
         <description id="addon-installed-notification-header"/>
         <description id="addon-installed-notification-message"/>
       </popupnotificationcontent>
     </popupnotification>
--- a/browser/components/extensions/ext-chrome-settings-overrides.js
+++ b/browser/components/extensions/ext-chrome-settings-overrides.js
@@ -10,20 +10,20 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/ExtensionPreferencesManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionSettingsStore",
                                   "resource://gre/modules/ExtensionSettingsStore.jsm");
 
 const DEFAULT_SEARCH_STORE_TYPE = "default_search";
 const DEFAULT_SEARCH_SETTING_NAME = "defaultSearch";
 
 const searchInitialized = () => {
-  if (Services.search.isInitialized) {
-    return;
-  }
   return new Promise(resolve => {
+    if (Services.search.isInitialized) {
+      resolve();
+    }
     const SEARCH_SERVICE_TOPIC = "browser-search-service";
     Services.obs.addObserver(function observer(subject, topic, data) {
       if (data != "init-complete") {
         return;
       }
 
       Services.obs.removeObserver(observer, SEARCH_SERVICE_TOPIC);
       resolve();
@@ -65,144 +65,113 @@ this.chrome_settings_overrides = class e
 
     await ExtensionSettingsStore.initialize();
     if (manifest.chrome_settings_overrides.homepage) {
       ExtensionPreferencesManager.setSetting(extension, "homepage_override",
                                              manifest.chrome_settings_overrides.homepage);
     }
     if (manifest.chrome_settings_overrides.search_provider) {
       await searchInitialized();
-      extension.callOnClose({
-        close: () => {
-          if (extension.shutdownReason == "ADDON_DISABLE" ||
-              extension.shutdownReason == "ADDON_UNINSTALL") {
-            switch (extension.shutdownReason) {
-              case "ADDON_DISABLE":
-                this.processDefaultSearchSetting("disable");
-                break;
-
-              case "ADDON_UNINSTALL":
-                this.processDefaultSearchSetting("removeSetting");
-                break;
-            }
-            // We shouldn't need to wait for search initialized here
-            // because the search service should be ready to go.
-            let engines = Services.search.getEnginesByExtensionID(extension.id);
-            for (let engine of engines) {
-              try {
-                Services.search.removeEngine(engine);
-              } catch (e) {
-                Components.utils.reportError(e);
-              }
-            }
-          }
-        },
-      });
-
       let searchProvider = manifest.chrome_settings_overrides.search_provider;
-      let engineName = searchProvider.name.trim();
       if (searchProvider.is_default) {
+        let engineName = searchProvider.name.trim();
         let engine = Services.search.getEngineByName(engineName);
         if (engine && Services.search.getDefaultEngines().includes(engine)) {
-          // Needs to be called every time to handle reenabling, but
-          // only sets default for install or enable.
-          await this.setDefault(engineName);
-          // For built in search engines, we don't do anything further
+          // Only add onclose handlers if we would definitely
+          // be setting the default engine.
+          extension.callOnClose({
+            close: () => {
+              switch (extension.shutdownReason) {
+                case "ADDON_DISABLE":
+                  this.processDefaultSearchSetting("disable");
+                  break;
+
+                case "ADDON_UNINSTALL":
+                  this.processDefaultSearchSetting("removeSetting");
+                  break;
+              }
+            },
+          });
+          if (extension.startupReason === "ADDON_INSTALL") {
+            let item = await ExtensionSettingsStore.addSetting(
+              extension, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME, engineName, () => {
+                return Services.search.currentEngine.name;
+              });
+            Services.search.currentEngine = Services.search.getEngineByName(item.value);
+          } else if (extension.startupReason === "ADDON_ENABLE") {
+            this.processDefaultSearchSetting("enable");
+          }
+          // If we would have set the default engine,
+          // we don't allow a search provider to be added.
           return;
         }
+        Components.utils.reportError("is_default can only be used for built-in engines.");
+      }
+      let isCurrent = false;
+      let index = -1;
+      if (extension.startupReason === "ADDON_UPGRADE") {
+        let engines = Services.search.getEnginesByExtensionID(extension.id);
+        if (engines.length > 0) {
+          // There can be only one engine right now
+          isCurrent = Services.search.currentEngine == engines[0];
+          // Get position of engine and store it
+          index = Services.search.getEngines().indexOf(engines[0]);
+          Services.search.removeEngine(engines[0]);
+        }
       }
-      this.addSearchEngine(searchProvider);
-      if (searchProvider.is_default) {
-        if (extension.startupReason === "ADDON_INSTALL") {
-          // Don't ask if it already the current engine
-          let engine = Services.search.getEngineByName(engineName);
-          if (Services.search.currentEngine != engine) {
-            let allow = await new Promise(resolve => {
-              let subject = {
-                wrappedJSObject: {
-                  // This is a hack because we don't have the browser of
-                  // the actual install. This means the popup might show
-                  // in a different window. Will be addressed in a followup bug.
-                  browser: windowTracker.topWindow.gBrowser.selectedBrowser,
-                  name: this.extension.name,
-                  icon: this.extension.iconURL,
-                  currentEngine: Services.search.currentEngine.name,
-                  newEngine: engineName,
-                  resolve,
-                },
-              };
-              Services.obs.notifyObservers(subject, "webextension-defaultsearch-prompt");
-            });
-            if (!allow) {
-              return;
-            }
+      try {
+        let params = {
+          template: searchProvider.search_url,
+          iconURL: searchProvider.favicon_url,
+          alias: searchProvider.keyword,
+          extensionID: extension.id,
+          suggestURL: searchProvider.suggest_url,
+        };
+        Services.search.addEngineWithDetails(searchProvider.name.trim(), params);
+        if (extension.startupReason === "ADDON_UPGRADE") {
+          let engine = Services.search.getEngineByName(searchProvider.name.trim());
+          if (isCurrent) {
+            Services.search.currentEngine = engine;
+          }
+          if (index != -1) {
+            Services.search.moveEngine(engine, index);
           }
         }
-        // Needs to be called every time to handle reenabling, but
-        // only sets default for install or enable.
-        await this.setDefault(engineName);
-      } else if (ExtensionSettingsStore.hasSetting(
-                extension, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME)) {
-        // is_default has been removed, but we still have a setting. Remove it.
-        // This won't cover the case where the entire search_provider is removed.
-        this.processDefaultSearchSetting("removeSetting");
+      } catch (e) {
+        Components.utils.reportError(e);
       }
     }
+    // If the setting exists for the extension, but is missing from the manifest,
+    // remove it. This can happen if the extension removes is_default.
+    // There's really no good place to put this, because the entire search section
+    // could be removed.
+    // We'll never get here in the normal case because we always return early
+    // if we have an is_default value that we use.
+    if (ExtensionSettingsStore.hasSetting(
+               extension, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME)) {
+      await searchInitialized();
+      this.processDefaultSearchSetting("removeSetting");
+    }
   }
-
-  async setDefault(engineName) {
+  async onShutdown(reason) {
     let {extension} = this;
-    if (extension.startupReason === "ADDON_INSTALL") {
-      let item = await ExtensionSettingsStore.addSetting(
-        extension, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME, engineName, () => {
-          return Services.search.currentEngine.name;
-        });
-      Services.search.currentEngine = Services.search.getEngineByName(item.value);
-    } else if (extension.startupReason === "ADDON_ENABLE") {
-      this.processDefaultSearchSetting("enable");
-    }
-  }
-
-  addSearchEngine(searchProvider) {
-    let {extension} = this;
-    let isCurrent = false;
-    let index = -1;
-    if (extension.startupReason === "ADDON_UPGRADE") {
-      let engines = Services.search.getEnginesByExtensionID(extension.id);
-      if (engines.length > 0) {
-        // There can be only one engine right now
-        isCurrent = Services.search.currentEngine == engines[0];
-        // Get position of engine and store it
-        index = Services.search.getEngines().indexOf(engines[0]);
-        Services.search.removeEngine(engines[0]);
+    if (reason == "ADDON_DISABLE" ||
+        reason == "ADDON_UNINSTALL") {
+      if (extension.manifest.chrome_settings_overrides.search_provider) {
+        await searchInitialized();
+        let engines = Services.search.getEnginesByExtensionID(extension.id);
+        for (let engine of engines) {
+          try {
+            Services.search.removeEngine(engine);
+          } catch (e) {
+            Components.utils.reportError(e);
+          }
+        }
       }
     }
-    try {
-      let params = {
-        template: searchProvider.search_url,
-        iconURL: searchProvider.favicon_url,
-        alias: searchProvider.keyword,
-        extensionID: extension.id,
-        suggestURL: searchProvider.suggest_url,
-      };
-      Services.search.addEngineWithDetails(searchProvider.name.trim(), params);
-      if (extension.startupReason === "ADDON_UPGRADE") {
-        let engine = Services.search.getEngineByName(searchProvider.name.trim());
-        if (isCurrent) {
-          Services.search.currentEngine = engine;
-        }
-        if (index != -1) {
-          Services.search.moveEngine(engine, index);
-        }
-      }
-    } catch (e) {
-      Components.utils.reportError(e);
-      return false;
-    }
-    return true;
   }
 };
 
 ExtensionPreferencesManager.addSetting("homepage_override", {
   prefNames: [
     "browser.startup.homepage",
   ],
   setCallback(value) {
--- a/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
+++ b/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
@@ -50,16 +50,42 @@ add_task(async function test_extension_s
 
   is(Services.search.currentEngine.name, "DuckDuckGo", "Default engine is DuckDuckGo");
 
   await ext1.unload();
 
   is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
 });
 
+/* This tests that using an invalid engine does nothing. */
+add_task(async function test_extension_setting_invalid_name_default_engine() {
+  let defaultEngineName = Services.search.currentEngine.name;
+
+  let ext1 = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "chrome_settings_overrides": {
+        "search_provider": {
+          "name": "InvalidName",
+          "search_url": "https://example.com/?q={searchTerms}",
+          "is_default": true,
+        },
+      },
+    },
+    useAddonManager: "temporary",
+  });
+
+  await ext1.startup();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+
+  await ext1.unload();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+});
+
 /* This tests that uninstalling add-ons maintains the proper
  * search default. */
 add_task(async function test_extension_setting_multiple_default_engine() {
   let defaultEngineName = Services.search.currentEngine.name;
   let ext1 = ExtensionTestUtils.loadExtension({
     manifest: {
       "chrome_settings_overrides": {
         "search_provider": {
@@ -144,16 +170,66 @@ add_task(async function test_extension_s
 
   is(Services.search.currentEngine.name, "Twitter", "Default engine is Twitter");
 
   await ext2.unload();
 
   is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
 });
 
+/* This tests adding an engine with one add-on and trying to make it the
+ *default with anoth. */
+add_task(async function test_extension_setting_invalid_default_engine() {
+  let defaultEngineName = Services.search.currentEngine.name;
+  let ext1 = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "chrome_settings_overrides": {
+        "search_provider": {
+          "name": "MozSearch",
+          "keyword": "MozSearch",
+          "search_url": "https://example.com/?q={searchTerms}",
+        },
+      },
+    },
+    useAddonManager: "temporary",
+  });
+
+  let ext2 = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "chrome_settings_overrides": {
+        "search_provider": {
+          "name": "MozSearch",
+          "search_url": "https://example.com/?q={searchTerms}",
+          "is_default": true,
+        },
+      },
+    },
+    useAddonManager: "temporary",
+  });
+
+  await ext1.startup();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+
+  let engine = Services.search.getEngineByName("MozSearch");
+  ok(engine, "Engine should exist.");
+
+  await ext2.startup();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+
+  await ext2.unload();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+
+  await ext1.unload();
+
+  is(Services.search.currentEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
+});
+
 /* This tests that when the user changes the search engine and the add-on
  * is unistalled, search stays with the user's choice. */
 add_task(async function test_user_changing_default_engine() {
   let ext1 = ExtensionTestUtils.loadExtension({
     manifest: {
       "chrome_settings_overrides": {
         "search_provider": {
           "name": "DuckDuckGo",
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -140,26 +140,16 @@ webextPerms.hostDescription.oneSite=Acce
 
 # LOCALIZATION NOTE (webextPerms.hostDescription.tooManySites)
 # Semi-colon list of plural forms.
 # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
 # #1 will be replaced by an integer indicating the number of additional
 # hosts for which this webextension is requesting permission.
 webextPerms.hostDescription.tooManySites=Access your data on #1 other site;Access your data on #1 other sites
 
-# LOCALIZATION NOTE (webext.defaultSearch.description)
-# %1$S is replaced with the localized named of the extension that is asking to change the default search engine.
-# %2$S is replaced with the name of the current search engine
-# %3$S is replaced with the name of the new search engine
-webext.defaultSearch.description=%1$S would like to change your default search engine from %2$S to %3$S. Is that OK?
-webext.defaultSearchYes.label=Yes
-webext.defaultSearchYes.accessKey=Y
-webext.defaultSearchNo.label=No
-webext.defaultSearchNo.accessKey=N
-
 # LOCALIZATION NOTE (addonPostInstall.message)
 # %1$S is replaced with the localized named of the extension that was
 # just installed.
 # %2$S is replaced with the localized name of the application.
 addonPostInstall.message1=%1$S has been added to %2$S.
 
 # LOCALIZATION NOTE (addonPostInstall.messageDetail)
 # %1$S is replaced with the icon for the add-ons menu.
--- a/browser/modules/ExtensionsUI.jsm
+++ b/browser/modules/ExtensionsUI.jsm
@@ -41,17 +41,16 @@ this.ExtensionsUI = {
 
   async init() {
     this.histogram = Services.telemetry.getHistogramById("EXTENSION_INSTALL_PROMPT_RESULT");
 
     Services.obs.addObserver(this, "webextension-permission-prompt");
     Services.obs.addObserver(this, "webextension-update-permissions");
     Services.obs.addObserver(this, "webextension-install-notify");
     Services.obs.addObserver(this, "webextension-optional-permission-prompt");
-    Services.obs.addObserver(this, "webextension-defaultsearch-prompt");
 
     await Services.wm.getMostRecentWindow("navigator:browser").delayedStartupPromise;
 
     this._checkForSideloaded();
   },
 
   async _checkForSideloaded() {
     let sideloaded = await AddonManagerPrivate.getNewSideloads();
@@ -233,31 +232,18 @@ this.ExtensionsUI = {
         permissions,
       });
 
       // If we don't have any promptable permissions, just proceed
       if (strings.msgs.length == 0) {
         resolve(true);
         return;
       }
+
       resolve(this.showPermissionsPrompt(browser, strings, icon));
-    } else if (topic == "webextension-defaultsearch-prompt") {
-      let {browser, name, icon, resolve, currentEngine, newEngine} = subject.wrappedJSObject;
-
-      let bundle = Services.strings.createBundle(BROWSER_PROPERTIES);
-
-      let strings = {};
-      strings.acceptText = bundle.GetStringFromName("webext.defaultSearchYes.label");
-      strings.acceptKey = bundle.GetStringFromName("webext.defaultSearchYes.accessKey");
-      strings.cancelText = bundle.GetStringFromName("webext.defaultSearchNo.label");
-      strings.cancelKey = bundle.GetStringFromName("webext.defaultSearchNo.accessKey");
-      let addonName = `<span class="addon-webext-name">${this._sanitizeName(name)}</span>`;
-      strings.text = bundle.formatStringFromName("webext.defaultSearch.description",
-                                               [addonName, currentEngine, newEngine], 3);
-      resolve(this.showDefaultSearchPrompt(browser, strings, icon));
     }
   },
 
   // Escape &, <, and > characters in a string so that it may be
   // injected as part of raw markup.
   _sanitizeName(name) {
     return name.replace(/&/g, "&amp;")
                .replace(/</g, "&lt;")
@@ -336,62 +322,17 @@ this.ExtensionsUI = {
               this.histogram.add(histkey + "Rejected");
             }
             resolve(false);
           },
         },
       ];
 
       win.PopupNotifications.show(browser, "addon-webext-permissions", "",
-                                  "addons-notification-icon",
-                                  action, secondaryActions, popupOptions);
-    });
-  },
-
-  showDefaultSearchPrompt(browser, strings, icon) {
-//    const kDefaultSearchHistKey = "defaultSearch";
-    return new Promise(resolve => {
-      let popupOptions = {
-        hideClose: true,
-        popupIconURL: icon || DEFAULT_EXTENSION_ICON,
-        persistent: false,
-        removeOnDismissal: true,
-        eventCallback(topic) {
-          if (topic == "showing") {
-            let doc = this.browser.ownerDocument;
-            // eslint-disable-next-line no-unsanitized/property
-            doc.getElementById("addon-webext-defaultsearch-text").innerHTML = strings.text;
-          } else if (topic == "removed") {
-            resolve(false);
-          }
-        }
-      };
-
-      let action = {
-        label: strings.acceptText,
-        accessKey: strings.acceptKey,
-        disableHighlight: true,
-        callback: () => {
-//          this.histogram.add(kDefaultSearchHistKey + "Accepted");
-          resolve(true);
-        },
-      };
-      let secondaryActions = [
-        {
-          label: strings.cancelText,
-          accessKey: strings.cancelKey,
-          callback: () => {
-//            this.histogram.add(kDefaultSearchHistKey + "Rejected");
-            resolve(false);
-          },
-        },
-      ];
-
-      let win = browser.ownerGlobal;
-      win.PopupNotifications.show(browser, "addon-webext-defaultsearch", "",
+      // eslint-disable-next-line no-unsanitized/property
                                   "addons-notification-icon",
                                   action, secondaryActions, popupOptions);
     });
   },
 
   showInstallNotification(target, addon) {
     let win = target.ownerGlobal;
     let popups = win.PopupNotifications;