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

@@ -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<Tab>}
+   */
+  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<Tab>}
+   */
+  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 += `&currentCookieStoreId=${currentCookieStoreId}`;
       }
-      return browser.tabs.create({
+      return this.createTabWrapper(
-        url: confirmUrl,
+        confirmUrl,
-        cookieStoreId: currentCookieStoreId,
+        currentCookieStoreId,
-        openerTabId,
         index,
-        active
+        active,
-      }).then(() => {
+        openerTabId,
+        groupId
+      ).then(() => {
         // We don't want to sync this URL ever nor clutter the users history
         browser.history.deleteUrl({url: confirmUrl});
       }).catch((e) => {

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

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<Tab>}
+ */
+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({
+  const reopenedTab = await browser.tabs.create({
-    index: tab[0].index + 1,
+    index: tab.index + 1,
     cookieStoreId,
     url: redirectUrl
   });
-  if (tab.length > 0) {
+  if (tab.groupId && tab.groupId !== TAB_GROUP_ID_NONE && browser.tabs.group) {
-    browser.tabs.remove(tab[0].id);
+    // 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);
 }

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();
       });

src/js/utils.js

@@ -94,6 +94,9 @@ const Utils = {
     return result.join("");
   },
 
+  /**
+   * @returns {Promise<Tab|false>}
+   */
   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<any>}
+   */
+  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(