Improve background tab positioning logic (#202)

stoically · Jun 21, 2019 · 21b9366 · 21b9366
1 parent 640dbe5
commit 21b9366
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 14 deletions.
diff --git a/src/background/container.js b/src/background/container.js
@@ -40,6 +40,7 @@ class Container {
     this.removeContainerDelayQueue = new PQueue();
     this.removeContainerQueueMaybeDone = this.removeContainerQueueMaybeDone.bind(this);
     this.noContainerTabs = {};
+    this.lastCreatedInactiveTab = {};
   }
 
 
@@ -122,7 +123,21 @@ class Container {
         if (tab) {
           newTabOptions.active = tab.active;
           if (tab.index >= 0) {
-            newTabOptions.index = tab.index + 1;
+            if (!tab.active && this.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT]) {
+              debug('[createTabInTempContainer] lastCreatedInactiveTab id', this.lastCreatedInactiveTab);
+              try {
+                const lastCreatedInactiveTab = await browser.tabs.get(
+                  this.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT]
+                );
+                debug('[createTabInTempContainer] lastCreatedInactiveTab', lastCreatedInactiveTab);
+                newTabOptions.index = lastCreatedInactiveTab.index + 1;
+              } catch (error) {
+                debug('[createTabInTempContainer] failed to get lastCreatedInactiveTab', error);
+                newTabOptions.index = tab.index + 1;
+              }
+            } else {
+              newTabOptions.index = tab.index + 1;
+            }
           }
           if (tab.pinned && !dontPin) {
             newTabOptions.pinned = true;
@@ -134,16 +149,12 @@ class Container {
         if (active === false) {
           newTabOptions.active = false;
         }
-        if (!newTabOptions.active) {
-          if (!this.tabs.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT]) {
-            this.tabs.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT] = newTabOptions.index;
-          } else {
-            newTabOptions.index = ++this.tabs.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT];
-          }
-        }
 
         debug('[createTabInTempContainer] creating tab in temporary container', newTabOptions);
         const newTab = await browser.tabs.create(newTabOptions);
+        if (!newTabOptions.active) {
+          this.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT] = newTab.id;
+        }
         debug('[createTabInTempContainer] new tab in temp container created', newTab);
         if (url) {
           this.urlCreatedContainer[url] = contextualIdentity.cookieStoreId;

diff --git a/src/background/tabs.js b/src/background/tabs.js
@@ -3,7 +3,6 @@ class Tabs {
     this.background = background;
 
     this.creatingInSameContainer = false;
-    this.lastInactiveIndex = {};
   }
 
 
@@ -46,9 +45,20 @@ class Tabs {
       browser.browserAction.disable(tab.id);
       return;
     }
-    if (!tab.active && this.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT] &&
-      this.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT] > tab.index) {
-      browser.tabs.move(tab.id, {index: this.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT]++});
+    if (!tab.active && this.container.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT] &&
+      this.container.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT] !== tab.id) {
+      try {
+        const lastCreatedInactiveTab = await browser.tabs.get(
+          this.container.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT]
+        );
+        if (lastCreatedInactiveTab.index > tab.index) {
+          debug('[onCreated] moving tab', lastCreatedInactiveTab, tab);
+          browser.tabs.move(tab.id, {index: lastCreatedInactiveTab.index});
+          this.container.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT] = tab.id;
+        }
+      } catch (error) {
+        debug('[onCreated] getting lastCreatedInactiveTab failed', error);
+      }
     }
     if (tab && tab.cookieStoreId && !this.container.tabContainerMap[tab.id] &&
         this.storage.local.tempContainers[tab.cookieStoreId]) {
@@ -60,7 +70,7 @@ class Tabs {
 
 
   async onUpdated(tabId, changeInfo, tab) {
-    debug('[onUpdated] tab updated', tab);
+    debug('[onUpdated] tab updated', tab, changeInfo);
     if (tab.incognito) {
       debug('[onUpdated] tab incognito, we ignore that');
       browser.browserAction.disable(tab.id);
@@ -98,6 +108,7 @@ class Tabs {
 
 
   async onRemoved(tabId) {
+    debug('[onRemoved]', tabId);
     if (this.container.noContainerTabs[tabId]) {
       delete this.container.noContainerTabs[tabId];
     }
@@ -111,7 +122,8 @@ class Tabs {
   async onActivated(activeInfo) {
     debug('[onActivated]', activeInfo);
     this.contextmenu.remove();
-    this.lastInactiveIndex[browser.windows.WINDOW_ID_CURRENT] = false;
+
+    this.container.lastCreatedInactiveTab[browser.windows.WINDOW_ID_CURRENT] = false;
     const activatedTab = await browser.tabs.get(activeInfo.tabId);
     if (!activatedTab.incognito) {
       this.contextmenu.add();