author | Harry Twyford <htwyford@mozilla.com> |
Mon, 14 Sep 2020 20:27:10 +0000 | |
changeset 548641 | af47bc7c002d4c84f857dc62b6d2046a891d2412 |
parent 548640 | 09212dec128ba4253d1dad630afd083cf4cbce0e |
child 548642 | b91bf98042fa3896bc60ace988b2c3ce1fbd29c7 |
push id | 126315 |
push user | htwyford@mozilla.com |
push date | Mon, 14 Sep 2020 23:52:56 +0000 |
treeherder | autoland@af47bc7c002d [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | adw |
bugs | 1657801 |
milestone | 82.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
|
--- 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); +});