From 6e3ae0cd212c5b5444bee23777dca371b895ec33 Mon Sep 17 00:00:00 2001 From: Yazan Al Macki Date: Tue, 4 Jun 2024 16:42:57 +0000 Subject: [PATCH] Bug 1890761 - Checking currentPage and userContextId to prevent Switch To Tab suggestion from showing for current tab and/or current container based on pref. r=mak Differential Revision: https://phabricator.services.mozilla.com/D210824 --- .../urlbar/UrlbarProviderTopSites.sys.mjs | 30 ++++- .../urlbar/tests/browser/browser_top_sites.js | 105 +++++++++++++++++- 2 files changed, 129 insertions(+), 6 deletions(-) diff --git a/browser/components/urlbar/UrlbarProviderTopSites.sys.mjs b/browser/components/urlbar/UrlbarProviderTopSites.sys.mjs index 4bc48ad69561..10d5408feff5 100644 --- a/browser/components/urlbar/UrlbarProviderTopSites.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderTopSites.sys.mjs @@ -39,6 +39,18 @@ const TOP_SITES_ENABLED_PREFS = [ "browser.newtabpage.activity-stream.feeds.system.topsites", ]; +// Helper function to compare 2 URLs without refs. +function sameUrlIgnoringRef(url1, url2) { + if (!url1 || !url2) { + return false; + } + + let cleanUrl1 = url1.replace(/#.*$/, ""); + let cleanUrl2 = url2.replace(/#.*$/, ""); + + return cleanUrl1 == cleanUrl2; +} + /** * A provider that returns the Top Sites shown on about:newtab. */ @@ -238,7 +250,19 @@ class ProviderTopSites extends UrlbarProvider { ...(tabUrlsToContextIds.get(site.url.replace(/#.*$/, "")) ?? []), ]); if (tabUserContextIds.size) { + let switchToTabResultAdded = false; for (let userContextId of tabUserContextIds) { + // Normally we could skip the whole for loop, but if searchAllContainers + // is set then the current page userContextId may differ, then we should + // allow switching to other ones. + if ( + sameUrlIgnoringRef(queryContext.currentPage, site.url) && + (!lazy.UrlbarPrefs.get("switchTabs.searchAllContainers") || + queryContext.userContextId == userContextId) + ) { + // Don't suggest switching to the current tab. + continue; + } payload.userContextId = userContextId; let result = new lazy.UrlbarResult( UrlbarUtils.RESULT_TYPE.TAB_SWITCH, @@ -249,8 +273,12 @@ class ProviderTopSites extends UrlbarProvider { ) ); addCallback(this, result); + switchToTabResultAdded = true; + } + // Avoid adding url result if Switch to Tab result was added. + if (switchToTabResultAdded) { + break; } - break; } } diff --git a/browser/components/urlbar/tests/browser/browser_top_sites.js b/browser/components/urlbar/tests/browser/browser_top_sites.js index 9f72547501a4..f0c03150a5fc 100644 --- a/browser/components/urlbar/tests/browser/browser_top_sites.js +++ b/browser/components/urlbar/tests/browser/browser_top_sites.js @@ -10,7 +10,7 @@ ChromeUtils.defineESModuleGetters(this, { }); const EN_US_TOPSITES = - "https://www.youtube.com/,https://www.facebook.com/,https://www.amazon.com/,https://www.reddit.com/,https://www.wikipedia.org/,https://twitter.com/"; + "https://www.youtube.com/,https://www.facebook.com/,https://www.amazon.com/,https://www.reddit.com/,about:robots,https://twitter.com/"; async function addTestVisits() { // Add some visits to a URL. @@ -222,10 +222,11 @@ add_task(async function topSitesBookmarksAndTabs() { "http://example.com/", "The example.com Top Site should be the first result." ); + Assert.equal( exampleResult.source, - UrlbarUtils.RESULT_SOURCE.TABS, - "The example.com Top Site should appear in the view as an open tab result." + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + "The example.com Top Site should not appear in the view as an open tab result since it is the current tab." ); let youtubeResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 1); @@ -333,8 +334,8 @@ add_task(async function topSitesPinned() { Assert.equal( exampleResult.source, - UrlbarUtils.RESULT_SOURCE.TABS, - "The example.com Top Site should be an open tab result." + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + "The example.com Top Site should not appear in the view as an open tab result since it is the current tab." ); Assert.ok( @@ -489,3 +490,97 @@ add_task(async function topSitesNumber() { await SpecialPowers.popPrefEnv(); await PlacesUtils.history.clear(); }); + +add_task(async function tabSwitchBehavior() { + let exampleTab = gBrowser.selectedTab; + let aboutRobotsTab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + "about:robots" + ); + + registerCleanupFunction(() => { + BrowserTestUtils.removeTab(aboutRobotsTab); + }); + + await BrowserTestUtils.switchTab(gBrowser, exampleTab); + + let sites = AboutNewTab.getTopSites(); + Assert.equal( + sites.length, + 6, + "The test suite browser should have 6 Top Sites." + ); + + await UrlbarTestUtils.promisePopupOpen(window, () => { + if (gURLBar.getAttribute("pageproxystate") == "invalid") { + gURLBar.handleRevert(); + } + EventUtils.synthesizeMouseAtCenter(gURLBar.inputField, {}); + }); + Assert.ok(gURLBar.view.isOpen, "UrlbarView should be open."); + await UrlbarTestUtils.promiseSearchComplete(window); + + Assert.equal( + UrlbarTestUtils.getResultCount(window), + 6, + "The number of results should be the same as the number of Top Sites (6)." + ); + + let aboutRobotsResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 4); + Assert.equal( + aboutRobotsResult.url, + "about:robots", + "The about:robots Top Site should be the 5th result." + ); + Assert.equal( + aboutRobotsResult.source, + UrlbarUtils.RESULT_SOURCE.TABS, + "The about:robots Top Site should appear as an open tab result." + ); + + await UrlbarTestUtils.promisePopupClose(window, () => { + gURLBar.blur(); + }); + + await BrowserTestUtils.switchTab(gBrowser, aboutRobotsTab); + + sites = AboutNewTab.getTopSites(); + Assert.equal( + sites.length, + 6, + "The test suite browser should have 6 Top Sites." + ); + + await UrlbarTestUtils.promisePopupOpen(window, () => { + if (gURLBar.getAttribute("pageproxystate") == "invalid") { + gURLBar.handleRevert(); + } + EventUtils.synthesizeMouseAtCenter(gURLBar.inputField, {}); + }); + Assert.ok(gURLBar.view.isOpen, "UrlbarView should be open."); + await UrlbarTestUtils.promiseSearchComplete(window); + + Assert.equal( + UrlbarTestUtils.getResultCount(window), + 6, + "The number of results should be the same as the number of Top Sites (6)." + ); + + aboutRobotsResult = await UrlbarTestUtils.getDetailsOfResultAt(window, 4); + Assert.equal( + aboutRobotsResult.url, + "about:robots", + "The about:robots Top Site should be the 5th result." + ); + Assert.equal( + aboutRobotsResult.source, + UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL, + "The about:robots Top Site should appear as a regular result." + ); + + await UrlbarTestUtils.promisePopupClose(window, () => { + gURLBar.blur(); + }); + + await PlacesUtils.history.clear(); +});