Bug 1534796 Tweak default search prompts r=aswan a=lizzard FENNEC_66_0_BUILD2 FIREFOX_66_0_BUILD2
authorMike Kaply <mozilla@kaply.com>
Wed, 13 Mar 2019 21:15:26 +0200
changeset 516374 7c987c3a58404a6c480e2952a4f5ef4a70aee058
parent 516373 bf49080b9793f3048554c3c63ce1d58d93a8c21c
child 516375 657a558c0585f9e6ddc0339b298357f81bfd49c6
push id1967
push userbtara@mozilla.com
push dateWed, 13 Mar 2019 19:34:04 +0000
treeherdermozilla-release@7c987c3a5840 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, lizzard
bugs1534796
milestone66.0
Bug 1534796 Tweak default search prompts r=aswan a=lizzard Reviewers: aswan Reviewed By: aswan Subscribers: aryx, opoprus, jcristau, pascalc, mixedpuppy, mkaply, aswan, ddurst, TheOne Tags: #secure-revision, #bmo-mozilla-employee-confidential Bug #: 1534796 Differential Revision: https://phabricator.services.mozilla.com/D23227
browser/components/extensions/parent/ext-chrome-settings-overrides.js
browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js
browser/components/extensions/test/browser/browser_ext_user_events.js
browser/components/extensions/test/browser/head.js
browser/modules/ExtensionsUI.jsm
--- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js
+++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js
@@ -223,17 +223,17 @@ this.chrome_settings_overrides = class e
                 // 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.defaultEngine.name,
                 newEngine: engineName,
-                resolve(allow) {
+                respond(allow) {
                   if (allow) {
                     ExtensionSettingsStore.addSetting(
                       extension.id, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME, engineName, () => Services.search.defaultEngine.name);
                     Services.search.defaultEngine = Services.search.getEngineByName(engineName);
                   }
                 },
               },
             };
--- 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
@@ -36,16 +36,91 @@ add_task(async function test_extension_s
 
   is(Services.search.defaultEngine.name, "DuckDuckGo", "Default engine is DuckDuckGo");
 
   await ext1.unload();
 
   is(Services.search.defaultEngine.name, defaultEngineName, `Default engine is ${defaultEngineName}`);
 });
 
+// Test the popup displayed when trying to add a non-built-in default
+// search engine.
+add_task(async function test_extension_setting_default_engine_external() {
+  const NAME = "Example Engine";
+
+  // Load an extension that tries to set the default engine,
+  // and wait for the ensuing prompt.
+  async function startExtension(win = window) {
+    let extension = ExtensionTestUtils.loadExtension({
+      manifest: {
+        "chrome_settings_overrides": {
+          "search_provider": {
+            "name": NAME,
+            "search_url": "https://example.com/?q={searchTerms}",
+            "is_default": true,
+          },
+        },
+      },
+      useAddonManager: "temporary",
+    });
+
+    let [panel] = await Promise.all([
+      promisePopupNotificationShown("addon-webext-defaultsearch", win),
+      extension.startup(),
+    ]);
+
+    isnot(panel, null, "Doorhanger was displayed for non-built-in default engine");
+
+    return {panel, extension};
+  }
+
+  // First time around, don't accept the default engine.
+  let {panel, extension} = await startExtension();
+  panel.secondaryButton.click();
+
+  // There is no explicit event we can wait for to know when the click
+  // callback has been fully processed.  One spin through the Promise
+  // microtask queue should be enough.  If this wait isn't long enough,
+  // the test below where we accept the prompt will fail.
+  await Promise.resolve();
+
+  is(Services.search.defaultEngine.name, defaultEngineName,
+     "Default engine was not changed after rejecting prompt");
+
+  await extension.unload();
+
+  // Do it again, this time accept the prompt.
+  ({panel, extension} = await startExtension());
+
+  panel.button.click();
+  await Promise.resolve();
+
+  is(Services.search.defaultEngine.name, NAME,
+     "Default engine was changed after accepting prompt");
+
+  await extension.unload();
+
+  is(Services.search.defaultEngine.name, defaultEngineName,
+     "Default engine is reverted after uninstalling extension.");
+
+  // One more time, this time close the window where the prompt
+  // appears instead of explicitly accepting or denying it.
+  let win = await BrowserTestUtils.openNewBrowserWindow();
+  await BrowserTestUtils.openNewForegroundTab(win.gBrowser, "about:blank");
+
+  ({extension} = await startExtension(win));
+
+  await BrowserTestUtils.closeWindow(win);
+
+  is(Services.search.defaultEngine.name, defaultEngineName,
+     "Default engine is unchanged when prompt is dismissed");
+
+  await extension.unload();
+});
+
 /* This tests that uninstalling add-ons maintains the proper
  * search default. */
 add_task(async function test_extension_setting_multiple_default_engine() {
   let ext1 = ExtensionTestUtils.loadExtension({
     manifest: {
       "chrome_settings_overrides": {
         "search_provider": {
           "name": "DuckDuckGo",
--- a/browser/components/extensions/test/browser/browser_ext_user_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_user_events.js
@@ -1,36 +1,10 @@
 "use strict";
 
-/**
- * Wait for the given PopupNotification to display
- *
- * @param {string} name
- *        The name of the notification to wait for.
- *
- * @returns {Promise}
- *          Resolves with the notification window.
- */
-function promisePopupNotificationShown(name) {
-  return new Promise(resolve => {
-    function popupshown() {
-      let notification = PopupNotifications.getNotification(name);
-      if (!notification) { return; }
-
-      ok(notification, `${name} notification shown`);
-      ok(PopupNotifications.isPanelOpen, "notification panel open");
-
-      PopupNotifications.panel.removeEventListener("popupshown", popupshown);
-      resolve(PopupNotifications.panel.firstElementChild);
-    }
-
-    PopupNotifications.panel.addEventListener("popupshown", popupshown);
-  });
-}
-
 // Test that different types of events are all considered
 // "handling user input".
 add_task(async function testSources() {
   let extension = ExtensionTestUtils.loadExtension({
     async background() {
       async function request(perm) {
         try {
           let result = await browser.permissions.request({
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -3,17 +3,17 @@
 "use strict";
 
 /* exported CustomizableUI makeWidgetId focusWindow forceGC
  *          getBrowserActionWidget
  *          clickBrowserAction clickPageAction
  *          getBrowserActionPopup getPageActionPopup getPageActionButton
  *          openBrowserActionPanel
  *          closeBrowserAction closePageAction
- *          promisePopupShown promisePopupHidden
+ *          promisePopupShown promisePopupHidden promisePopupNotificationShown
  *          toggleBookmarksToolbar
  *          openContextMenu closeContextMenu
  *          openContextMenuInSidebar openContextMenuInPopup
  *          openExtensionContextMenu closeExtensionContextMenu
  *          openActionContextMenu openSubmenu closeActionContextMenu
  *          openTabContextMenu closeTabContextMenu
  *          openToolsMenu closeToolsMenu
  *          imageBuffer imageBufferFromDataURI
@@ -148,16 +148,44 @@ function promisePopupHidden(popup) {
     let onPopupHidden = event => {
       popup.removeEventListener("popuphidden", onPopupHidden);
       resolve();
     };
     popup.addEventListener("popuphidden", onPopupHidden);
   });
 }
 
+/**
+ * Wait for the given PopupNotification to display
+ *
+ * @param {string} name
+ *        The name of the notification to wait for.
+ * @param {Window} [win]
+ *        The chrome window in which to wait for the notification.
+ *
+ * @returns {Promise}
+ *          Resolves with the notification window.
+ */
+function promisePopupNotificationShown(name, win = window) {
+  return new Promise(resolve => {
+    function popupshown() {
+      let notification = win.PopupNotifications.getNotification(name);
+      if (!notification) { return; }
+
+      ok(notification, `${name} notification shown`);
+      ok(win.PopupNotifications.isPanelOpen, "notification panel open");
+
+      win.PopupNotifications.panel.removeEventListener("popupshown", popupshown);
+      resolve(win.PopupNotifications.panel.firstElementChild);
+    }
+
+    win.PopupNotifications.panel.addEventListener("popupshown", popupshown);
+  });
+}
+
 function promisePossiblyInaccurateContentDimensions(browser) {
   return ContentTask.spawn(browser, null, async function() {
     function copyProps(obj, props) {
       let res = {};
       for (let prop of props) {
         res[prop] = obj[prop];
       }
       return res;
--- a/browser/modules/ExtensionsUI.jsm
+++ b/browser/modules/ExtensionsUI.jsm
@@ -242,29 +242,30 @@ var ExtensionsUI = {
 
       // 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 {browser, name, icon, respond, 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");
       strings.addonName = name;
       strings.text = bundle.formatStringFromName("webext.defaultSearch.description",
                                                  ["<>", currentEngine, newEngine], 3);
-      resolve(this.showDefaultSearchPrompt(browser, strings, icon));
+
+      this.showDefaultSearchPrompt(browser, strings, icon).then(respond);
     }
   },
 
   // Create a set of formatted strings for a permission prompt
   _buildStrings(info) {
     let bundle = Services.strings.createBundle(BROWSER_PROPERTIES);
     let brandBundle = Services.strings.createBundle(BRAND_PROPERTIES);
     let appName = brandBundle.GetStringFromName("brandShortName");