Bug 615761 - Made window.external.AddSearchProvider prompts tab modal. r=johannh,Standard8

Differential Revision: https://phabricator.services.mozilla.com/D69838

browser/components/search/test/browser/browser_webapi.js

@@ -21,7 +21,12 @@ function AddSearchProvider(...args) {
   );
 }
 
-function promiseDialogOpened() {
+const tabModalEnabled =
+  Services.prefs.getIntPref("prompts.modalType.searchService") !==
+    Services.prompt.MODAL_TYPE_WINDOW &&
+  Services.prefs.getBoolPref("prompts.tab_modal.enabled");
+
+function promiseWindowDialogOpened() {
   return new Promise((resolve, reject) => {
     Services.wm.addListener({
       onOpenWindow(xulWin) {
@@ -40,6 +45,54 @@ function promiseDialogOpened() {
   });
 }
 
+function promiseTabDialogOpened() {
+  return new Promise(resolve => {
+    let browser = gBrowser.selectedBrowser;
+    BrowserTestUtils.waitForEvent(browser, "DOMWillOpenModalDialog").then(
+      () => {
+        // Wait for the next event tick to make sure the prompt has opened.
+        SimpleTest.executeSoon(() => {
+          let promptBox = gBrowser.getTabModalPromptBox(
+            gBrowser.selectedBrowser
+          );
+          resolve(promptBox.listPrompts()[0].Dialog);
+        });
+      }
+    );
+  });
+}
+
+function promiseDialogOpened() {
+  if (tabModalEnabled) {
+    return promiseTabDialogOpened();
+  }
+  return promiseWindowDialogOpened();
+}
+
+function acceptDialog(dialog) {
+  if (tabModalEnabled) {
+    dialog.ui.button0.click();
+  } else {
+    dialog.document.getElementById("commonDialog").acceptDialog();
+  }
+  return BrowserTestUtils.waitForEvent(
+    gBrowser.selectedBrowser,
+    "DOMModalDialogClosed"
+  );
+}
+
+function cancelDialog(dialog) {
+  if (tabModalEnabled) {
+    dialog.ui.button1.click();
+  } else {
+    dialog.document.getElementById("commonDialog").cancelDialog();
+  }
+  return BrowserTestUtils.waitForEvent(
+    gBrowser.selectedBrowser,
+    "DOMModalDialogClosed"
+  );
+}
+
 add_task(async function test_working() {
   gBrowser.selectedTab = AddSearchProvider(ROOT + "testEngine.xml");
 
@@ -54,7 +107,7 @@ add_task(async function test_working() {
     getString("addEngineConfirmation", "Foo", "example.com"),
     "Should have seen the right install message"
   );
-  dialog.document.getElementById("commonDialog").cancelDialog();
+  await cancelDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });
@@ -75,7 +128,7 @@ add_task(async function test_HTTP() {
     getString("addEngineConfirmation", "Foo", "example.com"),
     "Should have seen the right install message"
   );
-  dialog.document.getElementById("commonDialog").cancelDialog();
+  await cancelDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });
@@ -94,7 +147,7 @@ add_task(async function test_relative() {
     getString("addEngineConfirmation", "Foo", "example.com"),
     "Should have seen the right install message"
   );
-  dialog.document.getElementById("commonDialog").cancelDialog();
+  await cancelDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });
@@ -110,7 +163,7 @@ add_task(async function test_invalid() {
     getString("error_invalid_engine_msg2", brandName, url),
     "Should have seen the right error message"
   );
-  dialog.document.getElementById("commonDialog").acceptDialog();
+  await acceptDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });
@@ -126,7 +179,7 @@ add_task(async function test_missing() {
     getString("error_loading_engine_msg2", brandName, url),
     "Should have seen the right error message"
   );
-  dialog.document.getElementById("commonDialog").acceptDialog();
+  await acceptDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });
@@ -142,7 +195,7 @@ add_task(async function test_missing_namespace() {
     getString("error_invalid_engine_msg2", brandName, url),
     "Should have seen the right error message"
   );
-  dialog.document.getElementById("commonDialog").acceptDialog();
+  await acceptDialog(dialog);
 
   gBrowser.removeCurrentTab();
 });

modules/libpref/init/all.js

@@ -4614,6 +4614,10 @@ pref("browser.sanitizer.loglevel", "Warn");
 // To disable blocking of auth prompts, set the limit to -1.
 pref("prompts.authentication_dialog_abuse_limit", 2);
 
+// The prompt type to use for search service prompts.
+// See nsIPromptService::MODAL_TYPE fields for possible values.
+pref("prompts.modalType.searchService", 2);
+
 // Payment Request API preferences
 pref("dom.payments.loglevel", "Warn");
 pref("dom.payments.defaults.saveCreditCard", false);

toolkit/actors/SidebarSearchParent.jsm

@@ -7,6 +7,9 @@
 var EXPORTED_SYMBOLS = ["SidebarSearchParent"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const { XPCOMUtils } = ChromeUtils.import(
+  "resource://gre/modules/XPCOMUtils.jsm"
+);
 
 ChromeUtils.defineModuleGetter(
   this,
@@ -14,6 +17,13 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/NetUtil.jsm"
 );
 
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "promptModalType",
+  "prompts.modalType.searchService",
+  Services.prompt.MODAL_TYPE_WINDOW
+);
+
 class SidebarSearchParent extends JSWindowActorParent {
   receiveMessage(msg) {
     if (msg.name == "Search:AddEngine") {
@@ -69,12 +79,23 @@ class SidebarSearchParent extends JSWindowActorParent {
         brandName,
         engineURL.spec,
       ]);
-      Services.ww.getNewPrompter(browser.ownerGlobal).alert(title, msg);
+      Services.prompt.asyncAlert(
+        this.browsingContext,
+        promptModalType,
+        title,
+        msg
+      );
       return;
     }
 
     Services.search
-      .addEngine(engineURL.spec, iconURL ? iconURL.spec : null, true)
+      .addEngine(
+        engineURL.spec,
+        iconURL ? iconURL.spec : null,
+        true,
+        null,
+        this.browsingContext
+      )
       .catch(ex =>
         Cu.reportError(
           "Unable to add search engine to the search service: " + ex

toolkit/components/search/SearchEngine.jsm

@@ -20,6 +20,13 @@ XPCOMUtils.defineLazyServiceGetters(this, {
   gChromeReg: ["@mozilla.org/chrome/chrome-registry;1", "nsIChromeRegistry"],
 });
 
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "promptModalType",
+  "prompts.modalType.searchService",
+  Services.prompt.MODAL_TYPE_WINDOW
+);
+
 const BinaryInputStream = Components.Constructor(
   "@mozilla.org/binaryinputstream;1",
   "nsIBinaryInputStream",
@@ -759,6 +766,9 @@ EngineURL.prototype = {
  * @param {boolean} options.isBuiltin
  *   Indicates whether the engine is a app-provided or not. If it is, it will
  *   be treated as read-only.
+ * @param {BrowsingContext} [options.reqContext]
+ *   The browsing context of the site the request to add the engine came from.
+ *   If provided, this will act as the parent for any prompt dialogs.
  */
 function SearchEngine(options = {}) {
   if (!("isBuiltin" in options)) {
@@ -771,6 +781,7 @@ function SearchEngine(options = {}) {
   this._definedAlias = null;
   this._urls = [];
   this._metaData = {};
+  this._reqContext = options.reqContext;
 
   let file, uri;
   if ("name" in options) {
@@ -1012,7 +1023,7 @@ SearchEngine.prototype = {
     return null;
   },
 
-  _confirmAddEngine() {
+  async _confirmAddEngine() {
     var stringBundle = Services.strings.createBundle(SEARCH_BUNDLE);
     var titleMessage = stringBundle.GetStringFromName("addEngineConfirmTitle");
 
@@ -1037,17 +1048,17 @@ SearchEngine.prototype = {
       "addEngineAddButtonLabel"
     );
 
-    var ps = Services.prompt;
-    var buttonFlags =
+    let ps = Services.prompt;
+    let buttonFlags =
       ps.BUTTON_TITLE_IS_STRING * ps.BUTTON_POS_0 +
       ps.BUTTON_TITLE_CANCEL * ps.BUTTON_POS_1 +
       ps.BUTTON_POS_0_DEFAULT;
 
-    var checked = { value: false };
-    // confirmEx returns the index of the button that was pressed.  Since "Add"
-    // is button 0, we want to return the negation of that value.
-    var confirm = !ps.confirmEx(
-      null,
+    let promptResult;
+    try {
+      promptResult = await ps.asyncConfirmEx(
+        this._reqContext,
+        promptModalType,
         titleMessage,
         dialogMessage,
         buttonFlags,
@@ -1055,10 +1066,19 @@ SearchEngine.prototype = {
         null,
         null, // button 1 & 2 names not used
         checkboxMessage,
-      checked
+        false
       );
+    } catch (error) {
+      // Prompting failed or the user navigated away, deny the request.
+      return { confirmed: false, useNow: false };
+    }
 
-    return { confirmed: confirm, useNow: checked.value };
+    // confirmEx returns the index of the button that was pressed.  Since "Add"
+    // is button 0, we want to return the negation of that value.
+    return {
+      confirmed: !promptResult.getProperty("buttonNumClicked"),
+      useNow: promptResult.getProperty("checked"),
+    };
   },
 
   /**
@@ -1071,7 +1091,7 @@ SearchEngine.prototype = {
    * @param {nsISearchEngine} engine
    *  The engine being loaded.
    */
-  _onLoad(bytes, engine) {
+  async _onLoad(bytes, engine) {
     /**
      * Handle an error during the load of an engine by notifying the engine's
      * error callback, if any.
@@ -1106,7 +1126,12 @@ SearchEngine.prototype = {
         engine._location,
       ]);
 
-      Services.ww.getNewPrompter(null).alert(title, text);
+      Services.prompt.asyncAlert(
+        engine._reqContext,
+        promptModalType,
+        title,
+        text
+      );
     }
 
     if (!bytes) {
@@ -1179,7 +1204,7 @@ SearchEngine.prototype = {
       // This property is only ever true for engines added via
       // nsISearchService::addEngine.
       if (engine._confirm) {
-        var confirmation = engine._confirmAddEngine();
+        var confirmation = await engine._confirmAddEngine();
         SearchUtils.log(
           "_onLoad: confirm is " +
             confirmation.confirmed +

toolkit/components/search/SearchService.jsm

@@ -2820,12 +2820,13 @@ SearchService.prototype = {
     return params;
   },
 
-  async addEngine(engineURL, iconURL, confirm, extensionID) {
+  async addEngine(engineURL, iconURL, confirm, extensionID, reqContext) {
     SearchUtils.log('addEngine: Adding "' + engineURL + '".');
     await this.init();
     let errCode;
     try {
       var engine = new SearchEngine({
+        reqContext,
         uri: engineURL,
         isBuiltin: false,
       });

toolkit/components/search/nsISearchService.idl

@@ -7,6 +7,8 @@
 interface nsIURI;
 interface nsIInputStream;
 
+webidl BrowsingContext;
+
 [scriptable, uuid(5799251f-5b55-4df7-a9e7-0c27812c469a)]
 interface nsISearchSubmission : nsISupports
 {
@@ -301,11 +303,16 @@ interface nsISearchService : nsISupports
    * @param extensionID [optional]
    *        Optional: The correct extensionID if called by an add-on.
    *
+   * @param reqContext [optional]
+   *        Optional: The browsing context of the site the request to add the
+   *        engine came from. If provided, this will act as the prompt parent.
+   *
    * @throws NS_ERROR_FAILURE if the description file cannot be successfully
    *         loaded.
    */
   Promise addEngine(in AString engineURL, in AString iconURL, in boolean confirm,
-                    [optional] in AString extensionID);
+                    [optional] in AString extensionID,
+                    [optional] in BrowsingContext reqContext);
 
   /**
    * Adds a new search engine, without asking the user for confirmation and

toolkit/components/search/tests/xpcshell/test_addEngine_callback.js

@@ -14,12 +14,10 @@ const { MockRegistrar } = ChromeUtils.import(
 // Only need to stub the methods actually called by nsSearchService
 var promptService = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsIPromptService]),
-  confirmEx() {},
-};
-var prompt = {
-  QueryInterface: ChromeUtils.generateQI([Ci.nsIPrompt]),
-  alert() {},
+  asyncConfirmEx() {},
+  asyncAlert() {},
 };
+
 // Override the prompt service and nsIPrompt, since the search service currently
 // prompts in response to certain installation failures we test here
 // XXX this should disappear once bug 863474 is fixed
@@ -27,7 +25,6 @@ MockRegistrar.register(
   "@mozilla.org/embedcomp/prompt-service;1",
   promptService
 );
-MockRegistrar.register("@mozilla.org/prompter;1", prompt);
 
 // First test inits the search service
 add_task(async function init_search_service() {