diff --git a/lib/ASRouterTriggerListeners.jsm b/lib/ASRouterTriggerListeners.jsm index 0834f29364..a45d48ac8b 100644 --- a/lib/ASRouterTriggerListeners.jsm +++ b/lib/ASRouterTriggerListeners.jsm @@ -30,6 +30,20 @@ async function checkStartupFinished(win) { } } +/** + * Check if the current browser location matches the location received by the + * navigation listener. + */ +function isFocusedTab(aBrowser, aLocationURI) { + try { + if (aBrowser.ownerGlobal.gBrowser.currentURI.spec === aLocationURI.spec) { + return true; + } + } catch (e) {} // nsIURI.host can throw for non-nsStandardURL nsIURIs + + return false; +} + function isPrivateWindow(win) { return !(win instanceof Ci.nsIDOMWindow) || win.closed || PrivateBrowsingUtils.isWindowPrivate(win); } @@ -179,6 +193,12 @@ this.ASRouterTriggerListeners = new Map([ }, onLocationChange(aBrowser, aWebProgress, aRequest, aLocationURI, aFlags) { + // If a new tab was opened in the background we don't want it to increment + // the visits count. That will happen anyway because of the `onTabSwitch` + // listener that fires when the tab is selected. + if (!isFocusedTab(aBrowser, aLocationURI)) { + return; + } // Some websites trigger redirect events after they finish loading even // though the location remains the same. This results in onLocationChange // events to be fired twice. diff --git a/test/browser/browser_asrouter_cfr.js b/test/browser/browser_asrouter_cfr.js index c6b96fc276..9a9cebbf2f 100644 --- a/test/browser/browser_asrouter_cfr.js +++ b/test/browser/browser_asrouter_cfr.js @@ -377,9 +377,17 @@ add_task(async function test_matchPattern() { let count = 0; const triggerHandler = () => ++count; const frequentVisitsTrigger = ASRouterTriggerListeners.get("frequentVisits"); - frequentVisitsTrigger.init(triggerHandler, [], ["*://*.example.com/"]); + await frequentVisitsTrigger.init(triggerHandler, [], ["*://*.example.com/"]); const browser = gBrowser.selectedBrowser; + // Add a background tab that matches the pattern but shouldn't trigger the handler + const backgroundTab = BrowserTestUtils.addTab(gBrowser, "http://example.com"); + gBrowser.removeTab(backgroundTab); + + // No visits should be registered + Assert.ok(!frequentVisitsTrigger._visits.get("www.example.com")); + Assert.ok(!frequentVisitsTrigger._visits.get("example.com")); + await BrowserTestUtils.loadURI(browser, "http://example.com/"); await BrowserTestUtils.browserLoaded(browser, false, "http://example.com/"); @@ -400,6 +408,11 @@ add_task(async function test_matchPattern() { await BrowserTestUtils.waitForCondition(() => frequentVisitsTrigger._visits.get("www.example.com").length === 1, "www.example.com is a different host that also matches the pattern."); await BrowserTestUtils.waitForCondition(() => frequentVisitsTrigger._visits.get("example.com").length === 1, "www.example.com is a different host that also matches the pattern."); + + Assert.equal(frequentVisitsTrigger._visits.get("www.example.com").length, 1); + Assert.equal(frequentVisitsTrigger._visits.get("example.com").length, 1); + + frequentVisitsTrigger.uninit(); }); add_task(async function test_providerNames() { diff --git a/test/unit/asrouter/ASRouterTriggerListeners.test.js b/test/unit/asrouter/ASRouterTriggerListeners.test.js index 32825c887c..9f0a1d4940 100644 --- a/test/unit/asrouter/ASRouterTriggerListeners.test.js +++ b/test/unit/asrouter/ASRouterTriggerListeners.test.js @@ -214,9 +214,9 @@ describe("ASRouterTriggerListeners", () => { const newTriggerHandler = sinon.stub(); await trigger.init(newTriggerHandler, hosts); - const browser = {}; const webProgress = {isTopLevel: true}; const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"}; + const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}}; const aRequest = { QueryInterface: sandbox.stub().returns({ originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"}, @@ -231,9 +231,9 @@ describe("ASRouterTriggerListeners", () => { const newTriggerHandler = sinon.stub(); await openURLListener.init(newTriggerHandler, hosts); - const browser = {}; const webProgress = {isTopLevel: true}; const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"}; + const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}}; const aRequest = { QueryInterface: sandbox.stub().returns({ originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"}, @@ -247,9 +247,9 @@ describe("ASRouterTriggerListeners", () => { const newTriggerHandler = sinon.stub(); await trigger.init(newTriggerHandler, hosts); - const browser = {}; const webProgress = {isTopLevel: true}; const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"}; + const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}}; const aRequest = { QueryInterface: sandbox.stub().returns({ originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"}, @@ -291,6 +291,21 @@ describe("ASRouterTriggerListeners", () => { assert.calledOnce(aRequest.QueryInterface); assert.notCalled(newTriggerHandler); }); + it("should not call triggerHandler if the tab opened is not focused", async () => { + const newTriggerHandler = sinon.stub(); + await frequentVisitsListener.init(newTriggerHandler, hosts); + + const webProgress = {isTopLevel: true}; + const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"}; + const browser = {ownerGlobal: {gBrowser: {currentURI: {spec: "different location"}}}}; + const aRequest = { + QueryInterface: sandbox.stub().returns({ + originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"}, + }), + }; + frequentVisitsListener.onLocationChange(browser, webProgress, aRequest, aLocationURI); + assert.notCalled(newTriggerHandler); + }); }); describe("delayed startup finished", () => {