diff --git a/browser/components/extensions/parent/ext-browser.js b/browser/components/extensions/parent/ext-browser.js index 496739c2f848..e7bed3723760 100644 --- a/browser/components/extensions/parent/ext-browser.js +++ b/browser/components/extensions/parent/ext-browser.js @@ -142,8 +142,8 @@ global.TabContext = class extends EventEmitter { windowTracker.addListener("progress", this); windowTracker.addListener("TabSelect", this); - this.tabDetached = this.tabDetached.bind(this); - tabTracker.on("tab-detached", this.tabDetached); + this.tabAdopted = this.tabAdopted.bind(this); + tabTracker.on("tab-adopted", this.tabAdopted); } /** @@ -188,14 +188,25 @@ global.TabContext = class extends EventEmitter { this.emit("location-change", tab, fromBrowse); } - tabDetached(eventType, {nativeTab, adoptedBy}) { - if (!this.tabData.has(nativeTab)) { + /** + * Persists context data when a tab is moved between windows. + * + * @param {string} eventType + * Event type, should be "tab-adopted". + * @param {NativeTab} adoptingTab + * The tab which is being opened and adopting `adoptedTab`. + * @param {NativeTab} adoptedTab + * The tab which is being closed and adopted by `adoptingTab`. + */ + tabAdopted(eventType, adoptingTab, adoptedTab) { + if (!this.tabData.has(adoptedTab)) { return; } // Create a new object (possibly with different inheritance) when a tab is moved // into a new window. But then reassign own properties from the old object. - let newData = this.get(adoptedBy); - let oldData = this.tabData.get(nativeTab); + let newData = this.get(adoptingTab); + let oldData = this.tabData.get(adoptedTab); + this.tabData.delete(adoptedTab); Object.assign(newData, oldData); } @@ -205,7 +216,7 @@ global.TabContext = class extends EventEmitter { shutdown() { windowTracker.removeListener("progress", this); windowTracker.removeListener("TabSelect", this); - tabTracker.off("tab-detached", this.tabDetached); + tabTracker.off("tab-adopted", this.tabAdopted); } }; @@ -304,6 +315,24 @@ class TabTracker extends TabTrackerBase { this._tabIds.set(id, nativeTab); } + /** + * Handles tab adoption when a tab is moved between windows. + * Ensures the new tab will have the same ID as the old one, + * and emits a "tab-adopted" event. + * + * @param {NativeTab} adoptingTab + * The tab which is being opened and adopting `adoptedTab`. + * @param {NativeTab} adoptedTab + * The tab which is being closed and adopted by `adoptingTab`. + */ + adopt(adoptingTab, adoptedTab) { + if (!this.adoptedTabs.has(adoptedTab)) { + this.adoptedTabs.set(adoptedTab, adoptingTab); + this.setId(adoptingTab, this.getId(adoptedTab)); + this.emit("tab-adopted", adoptingTab, adoptedTab); + } + } + _handleTabDestroyed(event, {nativeTab}) { let id = this._tabs.get(nativeTab); if (id) { @@ -363,11 +392,9 @@ class TabTracker extends TabTrackerBase { case "TabOpen": let {adoptedTab} = event.detail; if (adoptedTab) { - this.adoptedTabs.set(adoptedTab, event.target); - // This tab is being created to adopt a tab from a different window. - // Copy the ID from the old tab to the new. - this.setId(nativeTab, this.getId(adoptedTab)); + // Handle the adoption. + this.adopt(nativeTab, adoptedTab); adoptedTab.linkedBrowser.messageManager.sendAsyncMessage("Extension:SetFrameData", { windowId: windowTracker.getId(nativeTab.ownerGlobal), @@ -394,10 +421,10 @@ class TabTracker extends TabTrackerBase { let {adoptedBy} = event.detail; if (adoptedBy) { // This tab is being closed because it was adopted by a new window. - // Copy its ID to the new tab, in case it was created as the first tab - // of a new window, and did not have an `adoptedTab` detail when it was + // Handle the adoption in case it was created as the first tab of a + // new window, and did not have an `adoptedTab` detail when it was // opened. - this.setId(adoptedBy, this.getId(nativeTab)); + this.adopt(adoptedBy, nativeTab); this.emitDetached(nativeTab, adoptedBy); } else { @@ -448,9 +475,7 @@ class TabTracker extends TabTrackerBase { // process, so if we run later than it, we're too late. let nativeTab = tabToAdopt; let adoptedBy = window.gBrowser.tabs[0]; - - this.adoptedTabs.set(nativeTab, adoptedBy); - this.setId(adoptedBy, this.getId(nativeTab)); + this.adopt(adoptedBy, nativeTab); // We need to be sure to fire this event after the onDetached event // for the original tab. diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_context.js b/browser/components/extensions/test/browser/browser_ext_browserAction_context.js index 6662d4a80b20..5f696fe076f1 100644 --- a/browser/components/extensions/test/browser/browser_ext_browserAction_context.js +++ b/browser/components/extensions/test/browser/browser_ext_browserAction_context.js @@ -666,13 +666,19 @@ add_task(async function testMultipleWindows() { expect(details[3], details[2], null, details[0]); }, async expect => { - browser.test.log("Close the tab, expect window values."); - await browser.tabs.remove(tabs[1]); - expect(null, details[2], null, details[0]); + browser.test.log("Close the initial tab of the new window."); + let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0}); + await browser.tabs.remove(id); + expect(details[3], details[2], null, details[0]); }, async expect => { - browser.test.log("Close the new window and go back to the previous one."); - await browser.windows.remove(windows[1]); + browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close."); + await browser.windows.create({tabId: tabs[1]}); + expect(details[3], null, null, details[0]); + }, + async expect => { + browser.test.log("Close the tab, go back to the 1st window."); + await browser.tabs.remove(tabs[1]); expect(null, details[1], null, details[0]); }, async expect => { diff --git a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js index 5638a1db80d6..e726b30efa0b 100644 --- a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js +++ b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js @@ -256,8 +256,19 @@ add_task(async function testMultipleWindows() { expect(details[1]); }, async expect => { - browser.test.log("Close the new window and go back to the previous one."); - await browser.windows.remove(windows[1]); + browser.test.log("Close the initial tab of the new window."); + let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0}); + await browser.tabs.remove(id); + expect(details[1]); + }, + async expect => { + browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close."); + await browser.windows.create({tabId: tabs[1]}); + expect(details[1]); + }, + async expect => { + browser.test.log("Close the tab, go back to the 1st window."); + await browser.tabs.remove(tabs[1]); expect(null); }, ]; diff --git a/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js b/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js index 69694f6180cb..497c27286d5f 100644 --- a/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js +++ b/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js @@ -565,13 +565,19 @@ add_task(async function testMultipleWindows() { expect(details[3], details[2], null, details[0]); }, async expect => { - browser.test.log("Close the tab, expect window values."); - await browser.tabs.remove(tabs[1]); - expect(null, details[2], null, details[0]); + browser.test.log("Close the initial tab of the new window."); + let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0}); + await browser.tabs.remove(id); + expect(details[3], details[2], null, details[0]); }, async expect => { - browser.test.log("Close the new window and go back to the previous one."); - await browser.windows.remove(windows[1]); + browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close."); + await browser.windows.create({tabId: tabs[1]}); + expect(details[3], null, null, details[0]); + }, + async expect => { + browser.test.log("Close the tab, go back to the 1st window."); + await browser.tabs.remove(tabs[1]); expect(null, details[1], null, details[0]); }, async expect => {