Bug 1397975 - Show opt-in dialog for is_default with non built-in engines. r=aswan
☠☠ backed out by 84188868fc41 ☠ ☠
authorMichael Kaply <mozilla@kaply.com>
Thu, 07 Sep 2017 19:38:55 -0500
changeset 382244 7e8e47c972f41335e9109c51597efdc3bca56910
parent 382243 a5fec539b167cdedaf56e93b0a240fbb3c48f1c4
child 382245 3fadde636965fed90676c91088a9d81e2d38a078
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)
reviewersaswan
bugs1397975
milestone58.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 1397975 - Show opt-in dialog for is_default with non built-in engines. r=aswan MozReview-Commit-ID: 67SvzDcM7kK
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,14 +76,20 @@
       <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,113 +65,144 @@ 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)) {
-          // 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.
+          // 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
           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]);
-        }
       }
-      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);
+      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;
+            }
           }
         }
-      } catch (e) {
-        Components.utils.reportError(e);
+        // 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");
       }
     }
-    // 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) {
+    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");
     }
   }
-  async onShutdown(reason) {
+
+  addSearchEngine(searchProvider) {
     let {extension} = this;
-    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);
-          }
+    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]);
+      }
+    }
+    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,42 +50,16 @@ 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": {
@@ -170,66 +144,16 @@ 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,16 +140,26 @@ 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,16 +41,17 @@ 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();
@@ -232,18 +233,31 @@ 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;
 
-      resolve(this.showPermissionsPrompt(browser, strings, icon));
+      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;")
@@ -322,17 +336,62 @@ this.ExtensionsUI = {
               this.histogram.add(histkey + "Rejected");
             }
             resolve(false);
           },
         },
       ];
 
       win.PopupNotifications.show(browser, "addon-webext-permissions", "",
-      // eslint-disable-next-line no-unsanitized/property
+                                  "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", "",
                                   "addons-notification-icon",
                                   action, secondaryActions, popupOptions);
     });
   },
 
   showInstallNotification(target, addon) {
     let win = target.ownerGlobal;
     let popups = win.PopupNotifications;