From b6a1bff9e81d266fcfddce1ad01730ae2dfc14a3 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 21 Apr 2025 23:35:28 -0400 Subject: [PATCH] Fix #2747: open/reopen container tabs in tab groups when appropriate Firefox 137 introduced tab groups. Tab group web extension support is rolling out in Firefox 138 and later. Creating a new tab after the last tab in a tab group can inadvertently create the new tab outside of the tab group. When a user opens a new tab that should be in a container, this patch will make sure that the new tab resides in the same tab group as the original tab. --- src/js/background/assignManager.js | 85 +++++++++++++++++++++++++---- src/js/background/messageHandler.js | 8 ++- src/js/confirm-page.js | 29 +++++++--- src/js/popup.js | 6 +- src/js/utils.js | 20 ++++++- 5 files changed, 122 insertions(+), 26 deletions(-) diff --git a/src/js/background/assignManager.js b/src/js/background/assignManager.js index 0962a1d..fd781a2 100644 --- a/src/js/background/assignManager.js +++ b/src/js/background/assignManager.js @@ -1,3 +1,11 @@ +/** + * Firefox does not yet have the `tabGroups` API, which exposes this constant + * to indicate that a tab is not in a tab group. But the Firefox `tabs` API + * currently returns this constant value for `Tab.groupId`. + * @see https://searchfox.org/mozilla-central/rev/3b95c8dbe724b10390c96c1b9dd0f12c873e2f2e/browser/components/extensions/schemas/tabs.json#235 + */ +const TAB_GROUP_ID_NONE = -1; + window.assignManager = { MENU_ASSIGN_ID: "open-in-this-container", MENU_REMOVE_ID: "remove-open-in-this-container", @@ -326,7 +334,8 @@ window.assignManager = { options.url, tab.index + 1, tab.active, - openTabId + openTabId, + tab.groupId ); } else { this.reloadPageInContainer( @@ -336,7 +345,8 @@ window.assignManager = { tab.index + 1, tab.active, siteSettings.neverAsk, - openTabId + openTabId, + tab.groupId ); } this.calculateContextMenu(tab); @@ -727,7 +737,15 @@ window.assignManager = { }); }, - reloadPageInDefaultContainer(url, index, active, openerTabId) { + /** + * @param {string} url + * @param {number} index + * @param {boolean} active + * @param {number} [openerTabId] + * @param {number} [groupId] + * @returns {void} + */ + reloadPageInDefaultContainer(url, index, active, openerTabId, groupId) { // To create a new tab in the default container, it is easiest just to omit the // cookieStoreId entirely. // @@ -746,16 +764,58 @@ window.assignManager = { // does not automatically return to the original opener tab. To get this desired behaviour, // we MUST specify the openerTabId when creating the new tab. const cookieStoreId = "firefox-default"; - browser.tabs.create({url, cookieStoreId, index, active, openerTabId}); + this.createTabWrapper(url, cookieStoreId, index, active, openerTabId, groupId); }, - reloadPageInContainer(url, currentUserContextId, userContextId, index, active, neverAsk = false, openerTabId = null) { + + /** + * Wraps around `browser.tabs.create` and `browser.tabs.group` to create a + * tab and ensure that it ends up in the requested tab group, if applicable. + * + * @param {string} url + * @param {string} cookieStoreId + * @param {number} index + * @param {boolean} active + * @param {number} openerTabId + * @param {number} [groupId] + * @returns {Promise} + */ + async createTabWrapper(url, cookieStoreId, index, active, openerTabId, groupId) { + const newTab = await browser.tabs.create({ + url, + cookieStoreId, + index, + active, + openerTabId, + }); + + if (groupId && groupId !== TAB_GROUP_ID_NONE && browser.tabs.group) { + // If the original tab was in a tab group, make sure that the reopened tab + // stays in the same tab group. + await browser.tabs.group({ groupId, tabIds: newTab.id }); + } + + return newTab; + }, + + /** + * @param {string} url + * @param {string} currentUserContextId + * @param {string} userContextId + * @param {number} index + * @param {boolean} active + * @param {boolean} [neverAsk=false] + * @param {number} [openerTabId=null] + * @param {number} [groupId] + * @returns {Promise} + */ + reloadPageInContainer(url, currentUserContextId, userContextId, index, active, neverAsk = false, openerTabId = null, groupId = undefined) { const cookieStoreId = backgroundLogic.cookieStoreId(userContextId); const loadPage = browser.runtime.getURL("confirm-page.html"); // False represents assignment is not permitted // If the user has explicitly checked "Never Ask Again" on the warning page we will send them straight there if (neverAsk) { - return browser.tabs.create({url, cookieStoreId, index, active, openerTabId}); + return this.createTabWrapper(url, cookieStoreId, index, active, openerTabId, groupId); } else { let confirmUrl = `${loadPage}?url=${this.encodeURLProperty(url)}&cookieStoreId=${cookieStoreId}`; let currentCookieStoreId; @@ -763,13 +823,14 @@ window.assignManager = { currentCookieStoreId = backgroundLogic.cookieStoreId(currentUserContextId); confirmUrl += `¤tCookieStoreId=${currentCookieStoreId}`; } - return browser.tabs.create({ - url: confirmUrl, - cookieStoreId: currentCookieStoreId, - openerTabId, + return this.createTabWrapper( + confirmUrl, + currentCookieStoreId, index, - active - }).then(() => { + active, + openerTabId, + groupId + ).then(() => { // We don't want to sync this URL ever nor clutter the users history browser.history.deleteUrl({url: confirmUrl}); }).catch((e) => { diff --git a/src/js/background/messageHandler.js b/src/js/background/messageHandler.js index 96dbc68..0ad28b4 100644 --- a/src/js/background/messageHandler.js +++ b/src/js/background/messageHandler.js @@ -91,7 +91,9 @@ const messageHandler = { m.newUserContextId, m.tabIndex, m.active, - true + true, + null, + m.groupId ); break; case "assignAndReloadInContainer": @@ -101,7 +103,9 @@ const messageHandler = { m.newUserContextId, m.tabIndex, m.active, - true + true, + null, + m.groupId ); // m.tabId is used for where to place the in content message // m.url is the assignment to be removed/added diff --git a/src/js/confirm-page.js b/src/js/confirm-page.js index 453e68f..5cc9374 100644 --- a/src/js/confirm-page.js +++ b/src/js/confirm-page.js @@ -1,3 +1,11 @@ +/** + * Firefox does not yet have the `tabGroups` API, which exposes this constant + * to indicate that a tab is not in a tab group. But the Firefox `tabs` API + * currently returns this constant value for `Tab.groupId`. + * @see https://searchfox.org/mozilla-central/rev/3b95c8dbe724b10390c96c1b9dd0f12c873e2f2e/browser/components/extensions/schemas/tabs.json#235 + */ +const TAB_GROUP_ID_NONE = -1; + async function load() { const searchParams = new URL(window.location).searchParams; const redirectUrl = searchParams.get("url"); @@ -69,11 +77,15 @@ function confirmSubmit(redirectUrl, cookieStoreId) { openInContainer(redirectUrl, cookieStoreId); } -function getCurrentTab() { - return browser.tabs.query({ +/** + * @returns {Promise} + */ +async function getCurrentTab() { + const tabs = await browser.tabs.query({ active: true, windowId: browser.windows.WINDOW_ID_CURRENT }); + return tabs[0]; } async function denySubmit(redirectUrl, currentCookieStoreId) { @@ -93,7 +105,7 @@ async function denySubmit(redirectUrl, currentCookieStoreId) { await browser.runtime.sendMessage({ method: "exemptContainerAssignment", - tabId: tab[0].id, + tabId: tab.id, pageUrl: redirectUrl }); document.location.replace(redirectUrl); @@ -103,12 +115,15 @@ load(); async function openInContainer(redirectUrl, cookieStoreId) { const tab = await getCurrentTab(); - await browser.tabs.create({ - index: tab[0].index + 1, + const reopenedTab = await browser.tabs.create({ + index: tab.index + 1, cookieStoreId, url: redirectUrl }); - if (tab.length > 0) { - browser.tabs.remove(tab[0].id); + if (tab.groupId && tab.groupId !== TAB_GROUP_ID_NONE && browser.tabs.group) { + // If the original tab was in a tab group, make sure that the reopened tab + // stays in the same tab group. + browser.tabs.group({ groupId: tab.groupId, tabIds: reopenedTab.id }); } + browser.tabs.remove(tab.id); } diff --git a/src/js/popup.js b/src/js/popup.js index 61a3fda..e6c3d01 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -1306,7 +1306,8 @@ Logic.registerPanel(REOPEN_IN_CONTAINER_PICKER, { false, newUserContextId, currentTab.index + 1, - currentTab.active + currentTab.active, + currentTab.groupId ); window.close(); }; @@ -1336,7 +1337,8 @@ Logic.registerPanel(REOPEN_IN_CONTAINER_PICKER, { false, 0, currentTab.index + 1, - currentTab.active + currentTab.active, + currentTab.groupId ); window.close(); }); diff --git a/src/js/utils.js b/src/js/utils.js index 5745a47..8bfd76b 100644 --- a/src/js/utils.js +++ b/src/js/utils.js @@ -94,6 +94,9 @@ const Utils = { return result.join(""); }, + /** + * @returns {Promise} + */ async currentTab() { const activeTabs = await browser.tabs.query({ active: true, windowId: browser.windows.WINDOW_ID_CURRENT }); if (activeTabs.length > 0) { @@ -146,14 +149,24 @@ const Utils = { }); }, - async reloadInContainer(url, currentUserContextId, newUserContextId, tabIndex, active) { + /** + * @param {string} url + * @param {string} currentUserContextId + * @param {string} newUserContextId + * @param {number} tabIndex + * @param {boolean} active + * @param {number} [groupId] + * @returns {Promise} + */ + async reloadInContainer(url, currentUserContextId, newUserContextId, tabIndex, active, groupId = undefined) { return await browser.runtime.sendMessage({ method: "reloadInContainer", url, currentUserContextId, newUserContextId, tabIndex, - active + active, + groupId }); }, @@ -167,7 +180,8 @@ const Utils = { currentUserContextId: false, newUserContextId: assignedUserContextId, tabIndex: currentTab.index +1, - active:currentTab.active + active: currentTab.active, + groupId: currentTab.groupId }); } await Utils.setOrRemoveAssignment(