Bug 863474 - Centralise OpenSearch install failure prompts into SearchUIUtils. r=daleharvey,flod,geckoview-reviewers,esawin

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

browser/base/content/browser-pageActions.js

@@ -7,6 +7,11 @@ ChromeUtils.defineModuleGetter(
   "SiteSpecificBrowser",
   "resource:///modules/SiteSpecificBrowserService.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "SearchUIUtils",
+  "resource:///modules/SearchUIUtils.jsm"
+);
 
 var BrowserPageActions = {
   /**
@@ -1322,35 +1327,17 @@ BrowserPageActions.addSearchEngine = {
   },
 
   _installEngine(uri, image) {
-    Services.search.addOpenSearchEngine(uri, image).then(
+    SearchUIUtils.addOpenSearchEngine(
-      engine => {
+      uri,
-        showBrowserPageActionFeedback(this.action);
+      image,
-      },
+      gBrowser.selectedBrowser.browsingContext
-      errorCode => {
+    )
-        if (errorCode != Ci.nsISearchService.ERROR_DUPLICATE_ENGINE) {
+      .then(result => {
-          // Download error is shown by the search service
+        if (result) {
-          return;
+          showBrowserPageActionFeedback(this.action);
         }
-        const kSearchBundleURI =
+      })
-          "chrome://global/locale/search/search.properties";
+      .catch(console.error);
-        let searchBundle = Services.strings.createBundle(kSearchBundleURI);
-        let brandBundle = document.getElementById("bundle_brand");
-        let brandName = brandBundle.getString("brandShortName");
-        let title = searchBundle.GetStringFromName(
-          "error_invalid_engine_title"
-        );
-        let text = searchBundle.formatStringFromName(
-          "error_duplicate_engine_msg",
-          [brandName, uri]
-        );
-        Services.prompt.alertBC(
-          gBrowser.selectedBrowser.browsingContext,
-          Ci.nsIPrompt.MODAL_TYPE_CONTENT,
-          title,
-          text
-        );
-      }
-    );
   },
 };
 

browser/base/content/test/pageActions/browser.ini

@@ -5,6 +5,7 @@ support-files =
 [browser_PageActions_removeExtension.js]
 [browser_page_action_menu_add_search_engine.js]
 support-files =
+  page_action_menu_add_search_engine_invalid.html
   page_action_menu_add_search_engine_one.html
   page_action_menu_add_search_engine_many.html
   page_action_menu_add_search_engine_same_names.html

browser/base/content/test/pageActions/browser_PageActions_removeExtension.js

@@ -27,8 +27,12 @@ add_task(async function init() {
     url: "http://example.com/",
   });
 
+  // The prompt service is mocked later, so set it up to be restored.
+  let { prompt } = Services;
+
   registerCleanupFunction(async () => {
     BrowserTestUtils.removeTab(tab);
+    Services.prompt = prompt;
   });
 });
 
@@ -290,8 +294,6 @@ function promiseAddonUninstalled(addonId) {
 }
 
 function mockPromptService() {
-  let { prompt } = Services;
-
   let promptService = {
     // The prompt returns 1 for cancelled and 0 for accepted.
     _response: 0,
@@ -301,10 +303,6 @@ function mockPromptService() {
 
   Services.prompt = promptService;
 
-  registerCleanupFunction(() => {
-    Services.prompt = prompt;
-  });
-
   return promptService;
 }
 

browser/base/content/test/pageActions/browser_page_action_menu_add_search_engine.js

@@ -1,5 +1,9 @@
 "use strict";
 
+const { PromptTestUtils } = ChromeUtils.import(
+  "resource://testing-common/PromptTestUtils.jsm"
+);
+
 // Checks the panel button with a page that doesn't offer any engines.
 add_task(async function none() {
   let url = "http://mochi.test:8888/";
@@ -101,6 +105,53 @@ add_task(async function one() {
   });
 });
 
+// Checks the panel button with a page that offers an invalid engine.
+add_task(async function invalid() {
+  let url =
+    getRootDirectory(gTestPath) +
+    "page_action_menu_add_search_engine_invalid.html";
+  await BrowserTestUtils.withNewTab(url, async tab => {
+    // Open the panel.
+    await promisePageActionPanelOpen();
+
+    // The action should be present.
+    let actions = PageActions.actionsInPanel(window);
+    let action = actions.find(a => a.id == "addSearchEngine");
+    Assert.ok(action, "Action should be present in panel");
+    let expectedTitle = "Add Search Engine";
+    Assert.equal(action.getTitle(window), expectedTitle, "Action title");
+    let button = BrowserPageActions.panelButtonNodeForActionID(
+      "addSearchEngine"
+    );
+    Assert.ok(button, "Button should be in panel");
+    Assert.equal(button.label, expectedTitle, "Button label");
+    Assert.equal(
+      button.classList.contains("subviewbutton-nav"),
+      false,
+      "Button should not expand into a subview"
+    );
+
+    // Click the action's button.
+    let hiddenPromise = promisePageActionPanelHidden();
+    let promptPromise = PromptTestUtils.waitForPrompt(tab.linkedBrowser, {
+      modalType: Ci.nsIPromptService.MODAL_TYPE_CONTENT,
+      promptType: "alert",
+    });
+    EventUtils.synthesizeMouseAtCenter(button, {});
+    await hiddenPromise;
+    let prompt = await promptPromise;
+
+    Assert.ok(
+      prompt.ui.infoBody.textContent.includes(
+        "http://mochi.test:8888/browser/browser/base/content/test/pageActions/page_action_menu_add_search_engine_404.xml"
+      ),
+      "Should have included the url in the prompt body"
+    );
+
+    await PromptTestUtils.handlePrompt(prompt);
+  });
+});
+
 // Checks the panel button with a page that offers many engines.
 add_task(async function many() {
   let url =

browser/base/content/test/pageActions/page_action_menu_add_search_engine_invalid.html

@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<link rel="search" type="application/opensearchdescription+xml" title="page_action_menu_add_search_engine_0" href="http://mochi.test:8888/browser/browser/base/content/test/pageActions/page_action_menu_add_search_engine_404.xml">
+</head>
+<body></body>
+</html>

browser/components/search/SearchOneOffs.jsm

@@ -12,6 +12,7 @@ const { XPCOMUtils } = ChromeUtils.import(
 XPCOMUtils.defineLazyModuleGetters(this, {
   clearTimeout: "resource://gre/modules/Timer.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
+  SearchUIUtils: "resource:///modules/SearchUIUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   setTimeout: "resource://gre/modules/Timer.jsm",
 });
@@ -1196,38 +1197,17 @@ class SearchOneOffs {
     if (target.classList.contains("addengine-item")) {
       // On success, hide the panel and tell event listeners to reshow it to
       // show the new engine.
-      Services.search
+      SearchUIUtils.addOpenSearchEngine(
-        .addOpenSearchEngine(
+        target.getAttribute("uri"),
-          target.getAttribute("uri"),
+        target.getAttribute("image"),
-          target.getAttribute("image")
+        this.window.gBrowser.selectedBrowser.browsingContext
-        )
+      )
-        .then(engine => {
+        .then(result => {
-          this._rebuild();
+          if (result) {
-        })
+            this._rebuild();
-        .catch(errorCode => {
-          if (errorCode != Ci.nsISearchService.ERROR_DUPLICATE_ENGINE) {
-            // Download error is shown by the search service
-            return;
           }
-          const kSearchBundleURI =
+        })
-            "chrome://global/locale/search/search.properties";
+        .catch(console.error);
-          let searchBundle = Services.strings.createBundle(kSearchBundleURI);
-          let brandBundle = this.document.getElementById("bundle_brand");
-          let brandName = brandBundle.getString("brandShortName");
-          let title = searchBundle.GetStringFromName(
-            "error_invalid_engine_title"
-          );
-          let text = searchBundle.formatStringFromName(
-            "error_duplicate_engine_msg",
-            [brandName, target.getAttribute("uri")]
-          );
-          Services.prompt.alertBC(
-            this.window.gBrowser.selectedBrowser.browsingContext,
-            Ci.nsIPrompt.MODAL_TYPE_CONTENT,
-            title,
-            text
-          );
-        });
     }
 
     if (target.classList.contains("search-one-offs-context-open-in-new-tab")) {

browser/components/search/SearchUIUtils.jsm

@@ -0,0 +1,83 @@
+/* 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/. */
+
+/**
+ * Various utilities for search related UI.
+ */
+
+"use strict";
+
+const { XPCOMUtils } = ChromeUtils.import(
+  "resource://gre/modules/XPCOMUtils.jsm"
+);
+ChromeUtils.defineModuleGetter(
+  this,
+  "Services",
+  "resource://gre/modules/Services.jsm"
+);
+
+XPCOMUtils.defineLazyGetter(this, "SearchUIUtilsL10n", () => {
+  return new Localization(["browser/search.ftl", "branding/brand.ftl"]);
+});
+
+var EXPORTED_SYMBOLS = ["SearchUIUtils"];
+
+var SearchUIUtils = {
+  /**
+   * Adds an open search engine and handles error UI.
+   *
+   * @param {string} locationURL
+   *   The URL where the OpenSearch definition is located.
+   * @param {string} image
+   *   A URL string to an icon file to be used as the search engine's
+   *   icon. This value may be overridden by an icon specified in the
+   *   engine description file.
+   * @param {object} browsingContext
+   *  The browsing context any error prompt should be opened for.
+   */
+  async addOpenSearchEngine(locationURL, image, browsingContext) {
+    try {
+      await Services.search.addOpenSearchEngine(locationURL, image);
+    } catch (ex) {
+      let titleMsgName;
+      let descMsgName;
+      switch (ex.result) {
+        case Ci.nsISearchService.ERROR_DUPLICATE_ENGINE:
+          titleMsgName = "opensearch-error-duplicate-title";
+          descMsgName = "opensearch-error-duplicate-desc";
+          break;
+        case Ci.nsISearchService.ERROR_ENGINE_CORRUPTED:
+          titleMsgName = "opensearch-error-format-title";
+          descMsgName = "opensearch-error-format-desc";
+          break;
+        default:
+          // i.e. ERROR_DOWNLOAD_FAILURE
+          titleMsgName = "opensearch-error-download-title";
+          descMsgName = "opensearch-error-download-desc";
+          break;
+      }
+
+      let [title, text] = await SearchUIUtilsL10n.formatValues([
+        {
+          id: titleMsgName,
+        },
+        {
+          id: descMsgName,
+          args: {
+            "location-url": locationURL,
+          },
+        },
+      ]);
+
+      Services.prompt.alertBC(
+        browsingContext,
+        Ci.nsIPrompt.MODAL_TYPE_CONTENT,
+        title,
+        text
+      );
+      return false;
+    }
+    return true;
+  },
+};

browser/components/search/moz.build

@@ -7,6 +7,7 @@
 EXTRA_JS_MODULES += [
     'SearchOneOffs.jsm',
     'SearchTelemetry.jsm',
+    'SearchUIUtils.jsm',
 ]
 
 BROWSER_CHROME_MANIFESTS += [

browser/components/search/test/browser/browser.ini

@@ -34,6 +34,7 @@ skip-if = (verify && debug && (os == 'win' || os == 'linux'))
 [browser_oneOffContextMenu_setDefault.js]
 skip-if = (os == "win" && processor == "aarch64") # disabled on aarch64 due to Bug 1584886
 [browser_private_search_perwindowpb.js]
+[browser_searchbar_addEngine.js]
 [browser_searchbar_context.js]
 [browser_searchbar_default.js]
 [browser_searchbar_openpopup.js]

browser/components/search/test/browser/browser_searchbar_addEngine.js

@@ -0,0 +1,64 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Tests the Add Search Engine option in the search bar.
+ */
+
+"use strict";
+
+const { PromptTestUtils } = ChromeUtils.import(
+  "resource://testing-common/PromptTestUtils.jsm"
+);
+
+const searchPopup = document.getElementById("PopupSearchAutoComplete");
+let searchbar;
+
+add_task(async function setup() {
+  searchbar = await gCUITestUtils.addSearchBar();
+
+  registerCleanupFunction(async function() {
+    gCUITestUtils.removeSearchBar();
+  });
+});
+
+add_task(async function test_invalidEngine() {
+  let rootDir = getRootDirectory(gTestPath);
+  let tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    rootDir + "opensearch.html"
+  );
+  let promise = promiseEvent(searchPopup, "popupshown");
+  await EventUtils.synthesizeMouseAtCenter(
+    searchbar.querySelector(".searchbar-search-button"),
+    {}
+  );
+  await promise;
+
+  let addEngineList = searchPopup.querySelector(".search-add-engines");
+  let item = addEngineList.lastElementChild;
+
+  Assert.ok(
+    item.tooltipText.includes("engineInvalid"),
+    "Last item should be the invalid entry"
+  );
+
+  let promptPromise = PromptTestUtils.waitForPrompt(tab.linkedBrowser, {
+    modalType: Ci.nsIPromptService.MODAL_TYPE_CONTENT,
+    promptType: "alert",
+  });
+
+  await EventUtils.synthesizeMouseAtCenter(item, {});
+
+  let prompt = await promptPromise;
+
+  Assert.ok(
+    prompt.ui.infoBody.textContent.includes(
+      "http://mochi.test:8888/browser/browser/components/search/test/browser/testEngine_404.xml"
+    ),
+    "Should have included the url in the prompt body"
+  );
+
+  await PromptTestUtils.handlePrompt(prompt);
+  BrowserTestUtils.removeTab(tab);
+});

browser/components/search/test/browser/browser_searchbar_keyboard_navigation.js

@@ -579,7 +579,7 @@ add_task(async function test_open_search() {
   await promise;
 
   let engines = getOpenSearchItems();
-  is(engines.length, 2, "the opensearch.html page exposes 2 engines");
+  is(engines.length, 3, "the opensearch.html page exposes 3 engines");
 
   // Check that there's initially no selection.
   is(searchPopup.selectedIndex, -1, "no suggestion should be selected");

browser/components/search/test/browser/browser_searchbar_smallpanel_keyboard_navigation.js

@@ -378,7 +378,7 @@ add_task(async function test_open_search() {
   );
 
   let engines = getOpenSearchItems();
-  is(engines.length, 2, "the opensearch.html page exposes 2 engines");
+  is(engines.length, 3, "the opensearch.html page exposes 3 engines");
 
   // Check that there's initially no selection.
   is(searchPopup.selectedIndex, -1, "no suggestion should be selected");

browser/components/search/test/browser/opensearch.html

@@ -4,6 +4,7 @@
 <meta charset="UTF-8">
 <link rel="search" type="application/opensearchdescription+xml" title="engine1" href="http://mochi.test:8888/browser/browser/components/search/test/browser/testEngine.xml">
 <link rel="search" type="application/opensearchdescription+xml" title="engine2" href="http://mochi.test:8888/browser/browser/components/search/test/browser/testEngine_mozsearch.xml">
+<link rel="search" type="application/opensearchdescription+xml" title="engineInvalid" href="http://mochi.test:8888/browser/browser/components/search/test/browser/testEngine_404.xml">
 </head>
 <body></body>
 </html>

browser/locales/en-US/browser/search.ftl

@@ -0,0 +1,20 @@
+# 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/.
+
+## These strings are used for errors when installing OpenSearch engines, e.g.
+## via "Add Search Engine" on the address bar or search bar.
+## Variables
+## $location-url (String) - the URL of the OpenSearch engine that was attempted to be installed.
+
+opensearch-error-duplicate-title = Install Error
+opensearch-error-duplicate-desc = { -brand-short-name } could not install the search plugin from “{ $location-url }” because an engine with the same name already exists.
+
+opensearch-error-format-title = Invalid Format
+opensearch-error-format-desc = { -brand-short-name } could not install the search engine from: { $location-url }
+
+opensearch-error-download-title = Download Error
+opensearch-error-download-desc =
+    { -brand-short-name } could not download the search plugin from: { $location-url }
+
+##

mobile/android/locales/filter.py

@@ -24,7 +24,6 @@ def test(mod, path, entity=None):
             "chrome/global/commonDialogs.properties",
             "chrome/global/intl.properties",
             "chrome/global/intl.css",
-            "chrome/search/search.properties",
             "chrome/pluginproblem/pluginproblem.dtd",
             "chrome/global/aboutWebrtc.properties",
         ):

mobile/android/locales/jar.mn

@@ -28,7 +28,6 @@ relativesrcdir toolkit/locales:
   locale/@AB_CD@/browser/overrides/commonDialogs.properties        (%chrome/global/commonDialogs.properties)
   locale/@AB_CD@/browser/overrides/intl.properties                 (%chrome/global/intl.properties)
   locale/@AB_CD@/browser/overrides/intl.css                        (%chrome/global/intl.css)
-  locale/@AB_CD@/browser/overrides/search/search.properties        (%chrome/search/search.properties)
 # plugins
   locale/@AB_CD@/browser/overrides/plugins/pluginproblem.dtd       (%chrome/pluginproblem/pluginproblem.dtd)
 #about:webrtc

mobile/android/locales/l10n.toml

@@ -200,10 +200,6 @@ exclude-multi-locale = [
     reference = "toolkit/locales/en-US/chrome/global/intl.css"
     l10n = "{l}toolkit/chrome/global/intl.css"
 
-[[paths]]
-    reference = "toolkit/locales/en-US/chrome/search/search.properties"
-    l10n = "{l}toolkit/chrome/search/search.properties"
-
 [[paths]]
     reference = "toolkit/locales/en-US/chrome/pluginproblem/pluginproblem.dtd"
     l10n = "{l}toolkit/chrome/pluginproblem/pluginproblem.dtd"

mobile/locales/filter.py

@@ -24,7 +24,6 @@ def test(mod, path, entity=None):
             "chrome/global/commonDialogs.properties",
             "chrome/global/intl.properties",
             "chrome/global/intl.css",
-            "chrome/search/search.properties",
             "chrome/pluginproblem/pluginproblem.dtd",
             "chrome/global/aboutWebrtc.properties",
         ):

python/l10n/fluent_migrations/bug_863474_search_prompts.py

@@ -0,0 +1,93 @@
+# coding=utf8
+
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+from __future__ import absolute_import
+import fluent.syntax.ast as FTL
+from fluent.migrate.helpers import transforms_from
+from fluent.migrate import REPLACE
+from fluent.migrate.helpers import VARIABLE_REFERENCE, TERM_REFERENCE
+
+
+def migrate(ctx):
+    """Bug 863474 - Migrate search prompts to fluent, part {index}."""
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        transforms_from("""
+opensearch-error-duplicate-title = { COPY(from_path, "error_invalid_engine_title") }
+""", from_path="toolkit/chrome/search/search.properties"))
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        [
+            FTL.Message(
+                id=FTL.Identifier("opensearch-error-duplicate-desc"),
+                value=REPLACE(
+                    "toolkit/chrome/search/search.properties",
+                    "error_duplicate_engine_msg",
+                    {
+                        "%1$S": TERM_REFERENCE("brand-short-name"),
+                        "%2$S": VARIABLE_REFERENCE("location-url"),
+                    },
+                    normalize_printf=True
+                )
+            )
+        ]
+    )
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        transforms_from("""
+opensearch-error-format-title = { COPY(from_path, "error_invalid_format_title") }
+""", from_path="toolkit/chrome/search/search.properties"))
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        [
+            FTL.Message(
+                id=FTL.Identifier("opensearch-error-format-desc"),
+                value=REPLACE(
+                    "toolkit/chrome/search/search.properties",
+                    "error_invalid_engine_msg2",
+                    {
+                        "%1$S": TERM_REFERENCE("brand-short-name"),
+                        "%2$S": VARIABLE_REFERENCE("location-url"),
+                    },
+                    normalize_printf=True
+                )
+            )
+        ]
+    )
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        transforms_from("""
+opensearch-error-download-title = { COPY(from_path, "error_loading_engine_title") }
+""", from_path="toolkit/chrome/search/search.properties"))
+
+    ctx.add_transforms(
+        "browser/browser/search.ftl",
+        "browser/browser/search.ftl",
+        [
+            FTL.Message(
+                id=FTL.Identifier("opensearch-error-download-desc"),
+                value=REPLACE(
+                    "toolkit/chrome/search/search.properties",
+                    "error_loading_engine_msg2",
+                    {
+                        "%1$S": TERM_REFERENCE("brand-short-name"),
+                        "%2$S": VARIABLE_REFERENCE("location-url"),
+                        "\n": FTL.TextElement(" "),
+                    },
+                    normalize_printf=True
+                )
+            )
+        ]
+    )

toolkit/components/search/OpenSearchEngine.jsm

@@ -29,9 +29,6 @@ XPCOMUtils.defineLazyGetter(this, "logConsole", () => {
   });
 });
 
-const SEARCH_BUNDLE = "chrome://global/locale/search/search.properties";
-const BRAND_BUNDLE = "chrome://branding/locale/brand.properties";
-
 const OPENSEARCH_NS_10 = "http://a9.com/-/spec/opensearch/1.0/";
 const OPENSEARCH_NS_11 = "http://a9.com/-/spec/opensearch/1.1/";
 
@@ -239,41 +236,21 @@ class OpenSearchEngine extends SearchEngine {
      * Handle an error during the load of an engine by notifying the engine's
      * error callback, if any.
      *
-     * @param {number} [errorCode]
+     * @param {number} errorCode
      *   The relevant error code.
      */
-    function onError(errorCode = Ci.nsISearchService.ERROR_UNKNOWN_FAILURE) {
+    function onError(errorCode) {
+      if (engine._engineToUpdate) {
+        logConsole.warn("Failed to update", engine._engineToUpdate.name);
+      }
       // Notify the callback of the failure
       if (engine._installCallback) {
         engine._installCallback(errorCode);
       }
     }
 
-    function promptError(strings = {}, error = undefined) {
-      onError(error);
-
-      if (engine._engineToUpdate) {
-        // We're in an update, so just fail quietly
-        logConsole.warn("Failed to update", engine._engineToUpdate.name);
-        return;
-      }
-      var brandBundle = Services.strings.createBundle(BRAND_BUNDLE);
-      var brandName = brandBundle.GetStringFromName("brandShortName");
-
-      var searchBundle = Services.strings.createBundle(SEARCH_BUNDLE);
-      var msgStringName = strings.error || "error_loading_engine_msg2";
-      var titleStringName = strings.title || "error_loading_engine_title";
-      var title = searchBundle.GetStringFromName(titleStringName);
-      var text = searchBundle.formatStringFromName(msgStringName, [
-        brandName,
-        engine._location,
-      ]);
-
-      Services.ww.getNewPrompter(null).alert(title, text);
-    }
-
     if (!bytes) {
-      promptError();
+      onError(Ci.nsISearchService.ERROR_DOWNLOAD_FAILURE);
       return;
     }
 
@@ -288,12 +265,9 @@ class OpenSearchEngine extends SearchEngine {
       logConsole.error("_onLoad: Failed to init engine!", ex);
       // Report an error to the user
       if (ex.result == Cr.NS_ERROR_FILE_CORRUPTED) {
-        promptError({
+        onError(Ci.nsISearchService.ERROR_ENGINE_CORRUPTED);
-          error: "error_invalid_engine_msg2",
-          title: "error_invalid_format_title",
-        });
       } else {
-        promptError();
+        onError(Ci.nsISearchService.ERROR_DOWNLOAD_FAILURE);
       }
       return;
     }

toolkit/components/search/nsISearchService.idl

@@ -216,8 +216,9 @@ interface nsISearchParseSubmissionResult : nsISupports
 [scriptable, uuid(0301834b-2630-440e-8b98-db8dc55f34b9)]
 interface nsISearchService : nsISupports
 {
-  const unsigned long ERROR_UNKNOWN_FAILURE = 0x1;
+  const unsigned long ERROR_DOWNLOAD_FAILURE = 0x1;
   const unsigned long ERROR_DUPLICATE_ENGINE = 0x2;
+  const unsigned long ERROR_ENGINE_CORRUPTED = 0x3;
 
   /**
    * Start asynchronous initialization.

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

@@ -1,90 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- *    http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/*
- * Tests covering nsISearchService::addEngine's optional callback.
- */
-
-const { MockRegistrar } = ChromeUtils.import(
-  "resource://testing-common/MockRegistrar.jsm"
-);
-
-("use strict");
-
-// Only need to stub the methods actually called by nsSearchService
-var promptService = {
-  QueryInterface: ChromeUtils.generateQI(["nsIPromptService"]),
-  confirmEx() {},
-};
-var prompt = {
-  QueryInterface: ChromeUtils.generateQI(["nsIPrompt"]),
-  alert() {},
-};
-// 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
-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() {
-  useHttpServer();
-  await AddonTestUtils.promiseStartupManager();
-});
-
-// Simple test of the search callback
-add_task(async function simple_callback_test() {
-  let engine = await Services.search.addOpenSearchEngine(
-    gDataUrl + "engine.xml",
-    null
-  );
-  Assert.ok(!!engine);
-  Assert.notEqual(engine.name, (await Services.search.getDefault()).name);
-  Assert.ok(
-    !engine.isAppProvided,
-    "Should not be shown as an app-provided engine"
-  );
-  Assert.equal(
-    engine.wrappedJSObject._loadPath,
-    "[http]localhost/test-search-engine.xml"
-  );
-});
-
-// Test of the search callback on duplicate engine failures
-add_task(async function duplicate_failure_test() {
-  // Re-add the same engine added in the previous test
-  let engine;
-  try {
-    engine = await Services.search.addOpenSearchEngine(
-      gDataUrl + "engine.xml",
-      null
-    );
-  } catch (ex) {
-    let errorCode = ex.result;
-    Assert.ok(!!errorCode);
-    Assert.equal(errorCode, Ci.nsISearchService.ERROR_DUPLICATE_ENGINE);
-  } finally {
-    Assert.ok(!engine);
-  }
-});
-
-// Test of the search callback on failure to load the engine failures
-add_task(async function load_failure_test() {
-  // Try adding an engine that doesn't exist.
-  let engine;
-  try {
-    engine = await Services.search.addOpenSearchEngine(
-      "http://invalid/data/engine.xml",
-      null
-    );
-  } catch (ex) {
-    let errorCode = ex.result;
-    Assert.ok(!!errorCode);
-    Assert.equal(errorCode, Ci.nsISearchService.ERROR_UNKNOWN_FAILURE);
-  } finally {
-    Assert.ok(!engine);
-  }
-});

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

@@ -84,6 +84,12 @@ for (const test of tests) {
       test.description,
       "Should have a description"
     );
+
+    Assert.equal(
+      engine.wrappedJSObject._loadPath,
+      `[http]localhost/${test.file}`
+    );
+
     let submission = engine.getSubmission("foo");
     Assert.equal(
       submission.uri.spec,

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

@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Test that various install failures are handled correctly.
+ */
+
+add_task(async function setup() {
+  useHttpServer();
+  await AddonTestUtils.promiseStartupManager();
+  await Services.search.init();
+});
+
+add_task(async function test_invalid_path_fails() {
+  await Assert.rejects(
+    Services.search.addOpenSearchEngine("http://invalid/data/engine.xml", null),
+    error => {
+      Assert.equal(
+        error.result,
+        Ci.nsISearchService.ERROR_DOWNLOAD_FAILURE,
+        "Should have returned download failure."
+      );
+      return true;
+    },
+    "Should fail to install an engine with an invalid path."
+  );
+});
+
+add_task(async function test_install_duplicate_fails() {
+  let engine = await Services.search.addOpenSearchEngine(
+    gDataUrl + "engine.xml",
+    null
+  );
+  Assert.equal(
+    engine.name,
+    "Test search engine",
+    "Should have installed the engine."
+  );
+
+  await Assert.rejects(
+    Services.search.addOpenSearchEngine(gDataUrl + "engine.xml", null),
+    error => {
+      Assert.equal(
+        error.result,
+        Ci.nsISearchService.ERROR_DUPLICATE_ENGINE,
+        "Should have returned duplicate failure."
+      );
+      return true;
+    },
+    "Should fail to install a duplicate engine."
+  );
+});
+
+add_task(async function test_invalid_engine_from_dir() {
+  await Assert.rejects(
+    Services.search.addOpenSearchEngine(gDataUrl + "invalid-engine.xml", null),
+    error => {
+      Assert.equal(
+        error.result,
+        Ci.nsISearchService.ERROR_ENGINE_CORRUPTED,
+        "Should have returned corruption failure."
+      );
+      return true;
+    },
+    "Should fail to install an invalid engine."
+  );
+});

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

@@ -1,50 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/*
- * Test that invalid engine files correctly fail to install.
- */
-const mockPrompter = {
-  promptCount: 0,
-  alert() {
-    this.promptCount++;
-  },
-  QueryInterface: ChromeUtils.generateQI(["nsIPrompt"]),
-};
-
-add_task(async function setup() {
-  useHttpServer();
-  await AddonTestUtils.promiseStartupManager();
-  await Services.search.init();
-
-  // Mock the modal error dialog
-  let windowWatcher = {
-    getNewPrompter: () => mockPrompter,
-    QueryInterface: ChromeUtils.generateQI(["nsIWindowWatcher"]),
-  };
-  let origWindowWatcher = Services.ww;
-  Services.ww = windowWatcher;
-  registerCleanupFunction(() => {
-    Services.ww = origWindowWatcher;
-  });
-});
-
-add_task(async function test_invalid_engine_from_dir() {
-  console.log(gDataUrl + "data/invalid-engine.xml");
-  await Assert.rejects(
-    Services.search.addOpenSearchEngine(
-      gDataUrl + "data/invalid-engine.xml",
-      null
-    ),
-    error => {
-      Assert.ok(error.result == Ci.nsISearchService.ERROR_UNKNOWN_FAILURE);
-      return true;
-    },
-    "Should fail to install an invalid engine."
-  );
-  // The prompt happens after addEngine rejects, so wait here just in case.
-  await TestUtils.waitForCondition(
-    () => mockPrompter.promptCount == 1,
-    "Should have prompted the user"
-  );
-});

toolkit/components/search/tests/xpcshell/xpcshell.ini

@@ -55,7 +55,6 @@ support-files =
   test-extensions/multilocale/_locales/af/messages.json
   test-extensions/multilocale/_locales/an/messages.json
 
-[test_addEngine_callback.js]
 [test_addEngineWithDetails.js]
 [test_addEngineWithDetailsObject.js]
 [test_addEngineWithExtensionID.js]
@@ -110,7 +109,7 @@ support-files =
   data/remoteIcon.ico
   data/svgIcon.svg
 [test_opensearch_icons_invalid.js]
-[test_opensearch_invalid.js]
+[test_opensearch_install_errors.js]
 support-files = data/invalid-engine.xml
 [test_opensearch_update.js]
 [test_opensearch.js]

toolkit/locales/en-US/chrome/search/search.properties

@@ -1,13 +0,0 @@
-# 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/.
-
-error_loading_engine_title=Download Error
-# LOCALIZATION NOTE (error_loading_engine_msg2): %1$S = brandShortName, %2$S = location
-error_loading_engine_msg2=%S could not download the search plugin from:\n%S
-error_duplicate_engine_msg=%S could not install the search plugin from “%S” because an engine with the same name already exists.
-
-error_invalid_engine_title=Install Error
-error_invalid_format_title=Invalid Format
-# LOCALIZATION NOTE (error_invalid_engine_msg2): %1$S = brandShortName, %2$S = location (url)
-error_invalid_engine_msg2=%1$S could not install the search engine from: %2$S

toolkit/locales/jar.mn

@@ -89,7 +89,6 @@
   locale/@AB_CD@/autoconfig/autoconfig.properties   (%chrome/autoconfig/autoconfig.properties)
 % locale places @AB_CD@ %locale/@AB_CD@/places/
   locale/@AB_CD@/places/places.properties                 (%chrome/places/places.properties)
-  locale/@AB_CD@/global/search/search.properties          (%chrome/search/search.properties)
 
 # Pick up l10n files for pdfviewer from browser
 # See https://bugzilla.mozilla.org/show_bug.cgi?id=1618465