Bug 1657801 - Exit search mode if the current search mode engine is removed. r=adw
authorHarry Twyford <htwyford@mozilla.com>
Mon, 14 Sep 2020 20:27:10 +0000
changeset 548641 af47bc7c002d4c84f857dc62b6d2046a891d2412
parent 548640 09212dec128ba4253d1dad630afd083cf4cbce0e
child 548642 b91bf98042fa3896bc60ace988b2c3ce1fbd29c7
push id126315
push userhtwyford@mozilla.com
push dateMon, 14 Sep 2020 23:52:56 +0000
treeherderautoland@af47bc7c002d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1657801
milestone82.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 1657801 - Exit search mode if the current search mode engine is removed. r=adw Differential Revision: https://phabricator.services.mozilla.com/D89827
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser.ini
browser/components/urlbar/tests/browser/browser_searchMode_engineRemoval.js
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -14,16 +14,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
   BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
   ExtensionSearchHandler: "resource://gre/modules/ExtensionSearchHandler.jsm",
   ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
   ReaderMode: "resource://gre/modules/ReaderMode.jsm",
   PartnerLinkAttribution: "resource:///modules/PartnerLinkAttribution.jsm",
+  SearchUtils: "resource://gre/modules/SearchUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   UrlbarController: "resource:///modules/UrlbarController.jsm",
   UrlbarEventBufferer: "resource:///modules/UrlbarEventBufferer.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.jsm",
   UrlbarQueryContext: "resource:///modules/UrlbarUtils.jsm",
   UrlbarTokenizer: "resource:///modules/UrlbarTokenizer.jsm",
   UrlbarUtils: "resource:///modules/UrlbarUtils.jsm",
@@ -104,17 +105,21 @@ class UrlbarInput {
     this._lastValidURLStr = "";
     this._valueOnLastSearch = "";
     this._resultForCurrentValue = null;
     this._suppressStartQuery = false;
     this._suppressPrimaryAdjustment = false;
     this._untrimmedValue = "";
     this._searchModesByBrowser = new WeakMap();
 
-    UrlbarPrefs.addObserver(this);
+    this.QueryInterface = ChromeUtils.generateQI([
+      "nsIObserver",
+      "nsISupportsWeakReference",
+    ]);
+    this._addObservers();
 
     // This exists only for tests.
     this._enableAutofillPlaceholder = true;
 
     // Forward certain methods and properties.
     const CONTAINER_METHODS = [
       "getAttribute",
       "hasAttribute",
@@ -1255,18 +1260,23 @@ class UrlbarInput {
    *   The name of the search engine to restrict to.
    * @param {UrlbarUtils.RESULT_SOURCE} source
    *   A result source to restrict to.
    * @param {string} entry
    *   How search mode was entered. This is recorded in event telemetry. One of
    *   the values in UrlbarUtils.SEARCH_MODE_ENTRY.
    */
   setSearchMode({ engineName, source, entry }) {
-    if (!UrlbarPrefs.get("update2")) {
-      // Exit search mode.
+    // Exit search mode if update2 is disabled or the passed-in engine is
+    // invalid or hidden.
+    let engine = Services.search.getEngineByName(engineName);
+    if (
+      !UrlbarPrefs.get("update2") ||
+      (engineName && (!engine || engine.hidden))
+    ) {
       engineName = null;
       source = null;
     }
 
     // As an optimization, bail if the given search mode is already active.
     // Otherwise browser_preferences_usage.js fails due to accessing the
     // browser.urlbar.placeholderName pref (via BrowserSearch.initPlaceHolder
     // below) too many times.  That test does not enter search mode, but it
@@ -1583,17 +1593,38 @@ class UrlbarInput {
     }
 
     this.setSearchMode(searchMode);
     this._setValue(result.payload.query?.trimStart() || "", false);
     this.startQuery({ allowAutofill: false });
     return true;
   }
 
+  observe(subject, topic, data) {
+    switch (topic) {
+      case SearchUtils.TOPIC_ENGINE_MODIFIED: {
+        switch (data) {
+          case SearchUtils.MODIFIED_TYPE.CHANGED:
+          case SearchUtils.MODIFIED_TYPE.REMOVED:
+            if (this.searchMode?.engineName == subject.name) {
+              // We exit search mode if the current search mode engine was removed.
+              this.setSearchMode(this.searchMode);
+            }
+            break;
+        }
+        break;
+      }
+    }
+  }
+
   // Private methods below.
+  _addObservers() {
+    UrlbarPrefs.addObserver(this);
+    Services.obs.addObserver(this, SearchUtils.TOPIC_ENGINE_MODIFIED, true);
+  }
 
   _getURIFixupInfo(searchString) {
     let flags =
       Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
       Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
     if (this.isPrivate) {
       flags |= Ci.nsIURIFixup.FIXUP_FLAG_PRIVATE_CONTEXT;
     }
--- a/browser/components/urlbar/tests/browser/browser.ini
+++ b/browser/components/urlbar/tests/browser/browser.ini
@@ -148,16 +148,17 @@ run-if = e10s
 [browser_searchMode_alias_replacement.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_searchMode_autofill.js]
 [browser_searchMode_clickLink.js]
 support-files =
   dummy_page.html
+[browser_searchMode_engineRemoval.js]
 [browser_searchMode_no_results.js]
 [browser_searchMode_pickResult.js]
 [browser_searchMode_setURI.js]
 [browser_searchMode_suggestions.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_searchMode_switchTabs.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/urlbar/tests/browser/browser_searchMode_engineRemoval.js
@@ -0,0 +1,103 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that we exit search mode when the search mode engine is removed.
+ */
+
+"use strict";
+
+// Tests that we exit search mode in the active tab when the search mode engine
+// is removed.
+add_task(async function activeTab() {
+  let engine = await Services.search.addEngineWithDetails("Test", {
+    template: "http://example.com/?search={searchTerms}",
+  });
+  await Services.search.moveEngine(engine, 0);
+
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    value: "ex",
+  });
+  await UrlbarTestUtils.enterSearchMode(window);
+  // Sanity check: we are in the correct search mode.
+  await UrlbarTestUtils.assertSearchMode(window, {
+    engineName: engine.name,
+    entry: "oneoff",
+  });
+  await Services.search.removeEngine(engine);
+  // Check that we are no longer in search mode.
+  await UrlbarTestUtils.assertSearchMode(window, null);
+});
+
+// Tests that we exit search mode in a background tab when the search mode
+// engine is removed.
+add_task(async function backgroundTab() {
+  let engine = await Services.search.addEngineWithDetails("Test", {
+    template: "http://example.com/?search={searchTerms}",
+  });
+  await Services.search.moveEngine(engine, 0);
+
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    value: "ex",
+  });
+  await UrlbarTestUtils.enterSearchMode(window);
+  let tab1 = gBrowser.selectedTab;
+  let tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+
+  // Sanity check: tab1 is still in search mode.
+  await BrowserTestUtils.switchTab(gBrowser, tab1);
+  await UrlbarTestUtils.assertSearchMode(window, {
+    engineName: engine.name,
+    entry: "oneoff",
+  });
+
+  // Switch back to tab2 so tab1 is in the background when the engine is
+  // removed.
+  await BrowserTestUtils.switchTab(gBrowser, tab2);
+  // tab2 shouldn't be in search mode.
+  await UrlbarTestUtils.assertSearchMode(window, null);
+  await Services.search.removeEngine(engine);
+
+  // tab1 should have exited search mode.
+  await BrowserTestUtils.switchTab(gBrowser, tab1);
+  await UrlbarTestUtils.assertSearchMode(window, null);
+  BrowserTestUtils.removeTab(tab2);
+});
+
+// Tests that we exit search mode in a background window when the search mode
+// engine is removed.
+add_task(async function backgroundWindow() {
+  let engine = await Services.search.addEngineWithDetails("Test", {
+    template: "http://example.com/?search={searchTerms}",
+  });
+  await Services.search.moveEngine(engine, 0);
+
+  let win1 = window;
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window: win1,
+    value: "ex",
+  });
+  await UrlbarTestUtils.enterSearchMode(win1);
+  let win2 = await BrowserTestUtils.openNewBrowserWindow();
+
+  // Sanity check: win1 is still in search mode.
+  win1.focus();
+  await UrlbarTestUtils.assertSearchMode(win1, {
+    engineName: engine.name,
+    entry: "oneoff",
+  });
+
+  // Switch back to win2 so win1 is in the background when the engine is
+  // removed.
+  win2.focus();
+  // win2 shouldn't be in search mode.
+  await UrlbarTestUtils.assertSearchMode(win2, null);
+  await Services.search.removeEngine(engine);
+
+  // win1 should not be in search mode.
+  win1.focus();
+  await UrlbarTestUtils.assertSearchMode(win1, null);
+  await BrowserTestUtils.closeWindow(win2);
+});