Fix selection of new active tab when stashing the active tab

When we stash the active tab, we need to choose a new tab to become the
active tab.  We were doing this calculation improperly, because we were
slicing into a filtered list of tabs rather than the actual list of tabs
present in the window.  This would cause us to choose a tab that was
offset by the number of pinned+hidden tabs in the window.

Fix this and add some tests to make sure we choose the correct tab.

Closes #369.

src/model/fixtures.testlib.ts

@@ -36,6 +36,11 @@ export const STASH_ROOT_NAME = `${TEST_TAG} - Stash Root`;
  * minimum since in real-world tests, all of these windows will be created for
  * each test (where windows are used). */
 const WINDOWS = {
+  small: [
+    {id: "small_pinned", url: `${B}#pinned`, pinned: true},
+    {id: "small_active", url: `${B}#active`, active: true},
+    {id: "small_hidden", url: `${B}#hidden`, hidden: true},
+  ],
   left: [
     {id: "left_alice", url: `${B}#alice`, active: true},
     {id: "left_betty", url: `${B}#betty`},

src/model/index.test.ts

@@ -239,6 +239,7 @@ describe("model", () => {
 
         const hidden = await browser.tabs.query({hidden: true});
         expect(hidden.map(t => t.id!)).to.deep.equal([
+          tabs.small_hidden.id,
           tabs.real_doug_2.id,
           tabs.real_harry.id,
           tabs.real_helen.id,
@@ -261,6 +262,7 @@ describe("model", () => {
 
         const hidden = await browser.tabs.query({hidden: true});
         expect(hidden.map(t => t.id!)).to.deep.equal([
+          tabs.small_hidden.id,
           tabs.real_harry.id,
           tabs.real_helen.id,
         ]);
@@ -271,6 +273,7 @@ describe("model", () => {
 
         const hidden = await browser.tabs.query({hidden: true});
         expect(hidden.map(t => t.id!)).to.deep.equal([
+          tabs.small_hidden.id,
           tabs.real_doug_2.id,
           tabs.real_harry.id,
           tabs.real_helen.id,

src/model/tabs.test.ts

@@ -509,4 +509,87 @@ describe("model/tabs", () => {
     expect(model.tab(tabs.left_alice.id)!.active).to.equal(false);
     expect(model.tab(tabs.left_charlotte.id)!.active).to.equal(true);
   });
+
+  describe("refocusAwayFromTabs()", () => {
+    function test(options: {
+      window: keyof TabFixture["windows"];
+      activeTab: keyof TabFixture["tabs"];
+      closingTab: keyof TabFixture["tabs"];
+      newActiveTab?: keyof TabFixture["tabs"];
+    }) {
+      it.only(JSON.stringify(options), async () => {
+        if (!tabs[options.activeTab].active) {
+          await browser.tabs.update(tabs[options.activeTab].id, {active: true});
+          await events.next(browser.tabs.onActivated);
+          await events.next(browser.tabs.onHighlighted);
+        }
+
+        const closingTab = model.tab(tabs[options.closingTab].id)!;
+        const activeTab = model.tab(tabs[options.activeTab].id)!;
+        let newActiveTab = options.newActiveTab
+          ? model.tab(tabs[options.newActiveTab].id)!
+          : undefined;
+
+        expect(closingTab, "closingTab").to.not.be.undefined;
+        expect(activeTab, "activeTab").to.not.be.undefined;
+        if (options.newActiveTab) {
+          expect(newActiveTab, "newActiveTab").to.not.be.undefined;
+        }
+
+        await model.refocusAwayFromTabs([closingTab]);
+
+        if (activeTab !== newActiveTab) {
+          if (!options.newActiveTab) {
+            const ev = await events.next(browser.tabs.onCreated);
+            newActiveTab = model.tab(ev[0].id!);
+            expect(newActiveTab, "newActiveTab [opened]").to.not.be.undefined;
+            await events.next(browser.tabs.onUpdated);
+          }
+
+          await events.next(browser.tabs.onActivated);
+          await events.next(browser.tabs.onHighlighted);
+          expect(activeTab.active, "activeTab is not active").to.be.false;
+        }
+
+        expect(newActiveTab!.active, "newActiveTab is active").to.be.true;
+      });
+    }
+
+    test({
+      window: "real",
+      activeTab: "real_blank",
+      closingTab: "real_blank",
+      newActiveTab: "real_bob",
+    });
+    test({
+      window: "real",
+      activeTab: "real_blank",
+      closingTab: "real_bob",
+      newActiveTab: "real_blank",
+    });
+    test({
+      window: "real",
+      activeTab: "real_blank",
+      closingTab: "real_estelle",
+      newActiveTab: "real_blank",
+    });
+    test({
+      window: "real",
+      activeTab: "real_estelle",
+      closingTab: "real_estelle",
+      newActiveTab: "real_francis",
+    });
+    test({
+      window: "real",
+      activeTab: "real_unstashed",
+      closingTab: "real_unstashed",
+      newActiveTab: "real_francis",
+    });
+    test({
+      window: "small",
+      activeTab: "small_active",
+      closingTab: "small_active",
+      newActiveTab: undefined,
+    });
+  });
 });

src/model/tabs.ts

@@ -401,12 +401,12 @@ export class Model {
         () => `Couldn't find position of active tab ${active_tab.id}`,
       );
       const win = pos.parent;
-      const tabs_in_window = win.children.filter(t => !t.hidden && !t.pinned);
+      const visible_tabs = win.children.filter(t => !t.hidden && !t.pinned);
       const closing_tabs_in_window = tabs.filter(
         t => t.position?.parent === active_tab.position?.parent,
       );
 
-      if (closing_tabs_in_window.length >= tabs_in_window.length) {
+      if (closing_tabs_in_window.length >= visible_tabs.length) {
         // If we are about to close all visible tabs in the window, we
         // should open a new tab so the window doesn't close.
         trace("creating new empty tab in window", win.id);
@@ -423,14 +423,16 @@ export class Model {
         // from, to mimic the browser's behavior when closing the front
         // tab.
 
-        let candidates = tabs_in_window.slice(pos.index + 1);
+        let candidates = win.children.slice(pos.index + 1);
         let focus_tab = candidates.find(
-          c => c.id !== undefined && !tabs.includes(c),
+          c =>
+            c.id !== undefined && !c.hidden && !c.pinned && !tabs.includes(c),
         );
         if (!focus_tab) {
-          candidates = tabs_in_window.slice(0, pos.index).reverse();
+          candidates = win.children.slice(0, pos.index).reverse();
           focus_tab = candidates.find(
-            c => c.id !== undefined && !tabs.includes(c),
+            c =>
+              c.id !== undefined && !c.hidden && !c.pinned && !tabs.includes(c),
           );
         }