Bug 1397975 - Show opt-in dialog for is_default with non built-in engines. r?aswan draft
authorMichael Kaply <mozilla@kaply.com>
Thu, 07 Sep 2017 19:38:55 -0500
changeset 668624 8f3b55b23cfba1746c0abf35b780f10218247e50
parent 666416 42151fcd6cfc216d147730d0f2c6a2acd52d22fd
child 669281 4b22c72920da0b2d77bd4018ee38517cec04a252
push id81090
push usermozilla@kaply.com
push dateThu, 21 Sep 2017 21:19:33 +0000
reviewersaswan
bugs1397975
milestone57.0a1
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
@@ -1,29 +1,31 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+ /* globals windowTracker */
+
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionPreferencesManager",
                                   "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 +67,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;