From a8354f1215cfb33b97acc0d7bd6f2e021e9685cc Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Thu, 5 Mar 2020 09:12:44 +0000 Subject: [PATCH] Bug 555694 - Address bar should not suggest switching to the current tab. r=harry Differential Revision: https://phabricator.services.mozilla.com/D65408 --HG-- extra : moz-landing-system : lando --- .../test/browser_tabs_in_urlbar.js | 15 +++---- browser/components/urlbar/UrlbarInput.jsm | 1 + browser/components/urlbar/UrlbarUtils.jsm | 1 + browser/components/urlbar/docs/overview.rst | 2 + .../urlbar/tests/browser/browser.ini | 1 + .../browser_autocomplete_a11y_label.js | 9 ++-- .../browser/browser_switchTab_currentTab.js | 42 +++++++++++++++++++ .../browser/browser_tabMatchesInAwesomebar.js | 1 + toolkit/components/places/UnifiedComplete.jsm | 5 +++ 9 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 browser/components/urlbar/tests/browser/browser_switchTab_currentTab.js diff --git a/browser/components/sessionstore/test/browser_tabs_in_urlbar.js b/browser/components/sessionstore/test/browser_tabs_in_urlbar.js index 1b2343cb1576..44c13265ac45 100644 --- a/browser/components/sessionstore/test/browser_tabs_in_urlbar.js +++ b/browser/components/sessionstore/test/browser_tabs_in_urlbar.js @@ -62,9 +62,9 @@ add_task(async function test_unrestored_tabs_listed() { ], }; - const tabsForEnsure = {}; + const tabsForEnsure = new Set(); state.windows[0].tabs.forEach(function(tab) { - tabsForEnsure[tab.entries[0].url] = 1; + tabsForEnsure.add(tab.entries[0].url); }); let tabsRestoring = 0; @@ -103,6 +103,9 @@ add_task(async function test_unrestored_tabs_listed() { ss.setBrowserState(JSON.stringify(state)); }); + // Remove the current tab from tabsForEnsure, because switch to tab doesn't + // suggest it. + tabsForEnsure.delete(gBrowser.currentURI.spec); info("Searching open pages."); await UrlbarTestUtils.promiseAutocompleteResultPopup({ window, @@ -121,14 +124,12 @@ add_task(async function test_unrestored_tabs_listed() { } const url = result.url; Assert.ok( - url in tabsForEnsure, + tabsForEnsure.has(url), `Should have the found result '${url}' in the expected list of entries` ); // Remove the found entry from expected results. - delete tabsForEnsure[url]; + tabsForEnsure.delete(url); } // Make sure there is no reported open page that is not open. - for (const entry in tabsForEnsure) { - Assert.ok(!entry, `'${entry}' should have been in autocomplete results.`); - } + Assert.equal(tabsForEnsure.size, 0, "Should have found all the tabs"); }); diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index 50d2385e70e1..c5679401ba6a 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -858,6 +858,7 @@ class UrlbarInput { userContextId: this.window.gBrowser.selectedBrowser.getAttribute( "usercontextid" ), + currentPage: this.window.gBrowser.currentURI.spec, }) ); } diff --git a/browser/components/urlbar/UrlbarUtils.jsm b/browser/components/urlbar/UrlbarUtils.jsm index 4a1175f4099f..b4554f609c33 100644 --- a/browser/components/urlbar/UrlbarUtils.jsm +++ b/browser/components/urlbar/UrlbarUtils.jsm @@ -601,6 +601,7 @@ class UrlbarQueryContext { ["providers", v => Array.isArray(v) && v.length], ["sources", v => Array.isArray(v) && v.length], ["engineName", v => typeof v == "string" && !!v.length], + ["currentPage", v => typeof v == "string" && !!v.length], ]) { if (options[prop]) { if (!checkFn(options[prop])) { diff --git a/browser/components/urlbar/docs/overview.rst b/browser/components/urlbar/docs/overview.rst index 9787d9578629..bf4cc3610ce8 100644 --- a/browser/components/urlbar/docs/overview.rst +++ b/browser/components/urlbar/docs/overview.rst @@ -50,6 +50,8 @@ It is augmented as it progresses through the system, with various information: // property can be used to pick a specific search engine, by // setting it to the name under which the engine is registered // with the search service. + currentPage: // {string} url of the page that was loaded when the search + // began. // Properties added by the Model. results; // {array} list of UrlbarResult objects. diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index 00de23ac17ef..081310d620ba 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -103,6 +103,7 @@ support-files = support-files = slow-page.sjs [browser_switchTab_closesUrlbarPopup.js] +[browser_switchTab_currentTab.js] [browser_switchTab_decodeuri.js] [browser_switchTab_override.js] [browser_switchToTab_closes_newtab.js] diff --git a/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js b/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js index c4d02cc1fe7c..b2b7ec2eed87 100644 --- a/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js +++ b/browser/components/urlbar/tests/browser/browser_autocomplete_a11y_label.js @@ -50,15 +50,12 @@ async function initAccessibilityService() { } add_task(async function switchToTab() { - let tab = await BrowserTestUtils.openNewForegroundTab( - gBrowser, - "about:about" - ); + let tab = BrowserTestUtils.addTab(gBrowser, "about:robots"); await UrlbarTestUtils.promiseAutocompleteResultPopup({ window, waitForFocus: SimpleTest.waitForFocus, - value: "% about", + value: "% robots", }); let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 1); Assert.equal( @@ -70,7 +67,7 @@ add_task(async function switchToTab() { let element = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 1); is( await getResultText(element), - "about : about— Switch to Tab", + "about: robots— Switch to Tab", "Result a11y label should be: — Switch to Tab" ); diff --git a/browser/components/urlbar/tests/browser/browser_switchTab_currentTab.js b/browser/components/urlbar/tests/browser/browser_switchTab_currentTab.js new file mode 100644 index 000000000000..67472f25c361 --- /dev/null +++ b/browser/components/urlbar/tests/browser/browser_switchTab_currentTab.js @@ -0,0 +1,42 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * This test ensures that switch to tab still works when the URI contains an + * encoded part. + */ + +"use strict"; + +add_task(async function test_switchTab_currentTab() { + registerCleanupFunction(PlacesUtils.history.clear); + await BrowserTestUtils.withNewTab( + { gBrowser, url: "about:robots#1" }, + async () => { + await BrowserTestUtils.withNewTab( + { gBrowser, url: "about:robots#2" }, + async () => { + let context = await UrlbarTestUtils.promiseAutocompleteResultPopup({ + window, + waitForFocus: SimpleTest.waitForFocus, + value: "robot", + }); + Assert.ok( + context.results.some( + result => + result.type == UrlbarUtils.RESULT_TYPE.TAB_SWITCH && + result.payload.url == "about:robots#1" + ) + ); + Assert.ok( + !context.results.some( + result => + result.type == UrlbarUtils.RESULT_TYPE.TAB_SWITCH && + result.payload.url == "about:robots#2" + ) + ); + } + ); + } + ); +}); diff --git a/browser/components/urlbar/tests/browser/browser_tabMatchesInAwesomebar.js b/browser/components/urlbar/tests/browser/browser_tabMatchesInAwesomebar.js index d05082b946eb..c2bfbb6a5b95 100644 --- a/browser/components/urlbar/tests/browser/browser_tabMatchesInAwesomebar.js +++ b/browser/components/urlbar/tests/browser/browser_tabMatchesInAwesomebar.js @@ -41,6 +41,7 @@ add_task(async function step_2() { gBrowser.removeCurrentTab(); gBrowser.selectTabAtIndex(1); gBrowser.removeCurrentTab(); + gBrowser.selectTabAtIndex(0); let promises = []; for (let i = 1; i < gBrowser.tabs.length; i++) { diff --git a/toolkit/components/places/UnifiedComplete.jsm b/toolkit/components/places/UnifiedComplete.jsm index d0ccf447d888..27f58cfa3497 100644 --- a/toolkit/components/places/UnifiedComplete.jsm +++ b/toolkit/components/places/UnifiedComplete.jsm @@ -686,6 +686,7 @@ function Search( this._prohibitAutoFill = !queryContext.allowAutofill; this._maxResults = queryContext.maxResults; this._userContextId = queryContext.userContextId; + this._currentPage = queryContext.currentPage; } else { let params = new Set(searchParam.split(" ")); this._enableActions = params.has("enable-actions"); @@ -2491,6 +2492,10 @@ Search.prototype = { openPageCount > 0 && this.hasBehavior("openpage") ) { + if (this._currentPage == match.value) { + // Don't suggest switching to the current tab. + return; + } // Actions are enabled and the page is open. Add a switch-to-tab result. match.value = makeActionUrl("switchtab", { url: match.value }); match.style = "action switchtab";