Bug 1943971 - Let moving tabs to start/end not keep or put them in groups. r=jswinarton,tabbrowser-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D236318

browser/components/tabbrowser/content/tabbrowser.js

@@ -805,7 +805,6 @@
       }
 
       this.showTab(aTab);
-      this.ungroupTab(aTab);
       if (this.tabContainer.verticalMode) {
         this._handleTabMove(aTab, () =>
           this.verticalPinnedTabsContainer.appendChild(aTab)
@@ -3532,19 +3531,14 @@
       let tabs = contextTab.multiselected ? this.selectedTabs : [contextTab];
       // Walk the array in reverse order so the tabs are kept in order.
       for (let i = tabs.length - 1; i >= 0; i--) {
-        let tab = tabs[i];
-        if (tab._tPos > 0) {
-          this.moveTabTo(tab, 0);
-        }
+        this.moveTabToStart(tabs[i]);
       }
     }
 
     moveTabsToEnd(contextTab) {
       let tabs = contextTab.multiselected ? this.selectedTabs : [contextTab];
       for (let tab of tabs) {
-        if (tab._tPos < this.tabs.length - 1) {
-          this.moveTabTo(tab, this.tabs.length - 1);
-        }
+        this.moveTabToEnd(tab);
       }
     }
 
@@ -5664,16 +5658,14 @@
      *   a tab group, since pinned tabs are presently not allowed in tab groups.
      * @returns {void}
      */
-    moveTabTo(aTab, aIndex, options = { forceStandaloneTab: false }) {
-      const { forceStandaloneTab } = options;
-
+    moveTabTo(aTab, aIndex, { forceStandaloneTab = false } = {}) {
       // Don't allow mixing pinned and unpinned tabs.
       if (aTab.pinned) {
         aIndex = Math.min(aIndex, this.pinnedTabCount - 1);
       } else {
         aIndex = Math.max(aIndex, this.pinnedTabCount);
       }
-      if (aTab._tPos == aIndex) {
+      if (aTab._tPos == aIndex && !(aTab.group && forceStandaloneTab)) {
         return;
       }
 
@@ -5684,7 +5676,7 @@
         if (forceStandaloneTab && neighbor.group) {
           neighbor = neighbor.group;
         }
-        if (neighbor && aIndex >= aTab._tPos) {
+        if (neighbor && aIndex > aTab._tPos) {
           neighbor.after(aTab);
         } else {
           this.tabContainer.insertBefore(aTab, neighbor);
@@ -5878,12 +5870,12 @@
       }
     }
 
-    moveTabToStart() {
-      this.moveTabTo(this.selectedTab, 0);
+    moveTabToStart(aTab = this.selectedTab) {
+      this.moveTabTo(aTab, 0, { forceStandaloneTab: true });
     }
 
-    moveTabToEnd() {
-      this.moveTabTo(this.selectedTab, this.tabs.length - 1);
+    moveTabToEnd(aTab = this.selectedTab) {
+      this.moveTabTo(aTab, this.tabs.length - 1, { forceStandaloneTab: true });
     }
 
     /**
@@ -8420,9 +8412,9 @@ var TabContextMenu = {
       }
     );
     let visibleTabs = gBrowser.visibleTabs;
-    let lastVisibleTab = visibleTabs[visibleTabs.length - 1];
+    let lastVisibleTab = visibleTabs.at(-1);
     let tabsToMove = contextTabs;
-    let lastTabToMove = tabsToMove[tabsToMove.length - 1];
+    let lastTabToMove = tabsToMove.at(-1);
 
     let isLastPinnedTab = false;
     if (lastTabToMove.pinned) {
@@ -8431,11 +8423,13 @@ var TabContextMenu = {
     }
     contextMoveTabToEnd.disabled =
       (lastTabToMove == lastVisibleTab || isLastPinnedTab) &&
+      !lastTabToMove.group &&
       allSelectedTabsAdjacent;
     let contextMoveTabToStart = document.getElementById("context_moveToStart");
     let isFirstTab =
-      tabsToMove[0] == visibleTabs[0] ||
-      tabsToMove[0] == visibleTabs[gBrowser.pinnedTabCount];
+      !tabsToMove[0].group &&
+      (tabsToMove[0] == visibleTabs[0] ||
+        tabsToMove[0] == visibleTabs[gBrowser.pinnedTabCount]);
     contextMoveTabToStart.disabled = isFirstTab && allSelectedTabsAdjacent;
 
     document.getElementById("context_openTabInWindow").disabled =

browser/components/tabbrowser/content/tabs.js

@@ -1162,8 +1162,8 @@
           }
         }
       } else if (draggedTab) {
-        // Move the tabs. To avoid multiple tab-switches in the original window,
-        // the selected tab should be adopted last.
+        // Move the tabs into this window. To avoid multiple tab-switches in
+        // the original window, the selected tab should be adopted last.
         const dropIndex = this._getDropIndex(event);
         let newIndex = dropIndex;
         let selectedTab;

browser/components/tabbrowser/test/browser/tabs/browser_tab_groups.js

@@ -600,6 +600,75 @@ add_task(async function test_moveTabBetweenGroups() {
   await removeTabGroup(group2);
 });
 
+add_task(async function test_moveTabToStartOrEnd() {
+  let tab1 = gBrowser.selectedTab;
+  let tab2 = BrowserTestUtils.addTab(gBrowser, "about:blank");
+  gBrowser.addTabGroup([tab1, tab2]);
+  Assert.equal(tab1._tPos, 0, "tab 1 starts at tab index 0");
+  Assert.equal(tab2._tPos, 1, "tab 2 starts at tab index 1");
+  Assert.equal(
+    tab1.elementIndex,
+    1,
+    "tab 1 starts at element index 1, after the group label"
+  );
+  Assert.equal(
+    tab2.elementIndex,
+    2,
+    "tab 2 starts at element index 2, after tab 1"
+  );
+
+  gBrowser.moveTabToStart(tab1);
+  Assert.ok(
+    !tab1.group,
+    "first tab is not grouped anymore after moving to start"
+  );
+  Assert.ok(
+    tab2.group,
+    "last tab is still grouped after moving first tab to start"
+  );
+  Assert.equal(
+    tab1._tPos,
+    0,
+    "tab 1 remains at tab index 0 after moving to start"
+  );
+  Assert.equal(
+    tab2._tPos,
+    1,
+    "tab 2 remains at tab index 1 after tab 1 moved to start"
+  );
+  Assert.equal(
+    tab1.elementIndex,
+    0,
+    "tab 1 moved to element index 0, before the group label"
+  );
+  Assert.equal(
+    tab2.elementIndex,
+    2,
+    "tab 2 remains at element index 2, after the group label"
+  );
+
+  gBrowser.moveTabToEnd(tab2);
+  Assert.ok(!tab2.group, "last tab is not grouped anymore after moving to end");
+  Assert.equal(
+    tab1._tPos,
+    0,
+    "tab 1 remains at tab index 0 after tab 2 moved to end"
+  );
+  Assert.equal(
+    tab2._tPos,
+    1,
+    "tab 2 remains at tab index 1 after moving to end"
+  );
+  Assert.equal(tab1.elementIndex, 0, "tab 1 remains at element index 0");
+  Assert.equal(
+    tab2.elementIndex,
+    1,
+    "tab 2 moved at element index 1 since the group label is gone"
+  );
+
+  BrowserTestUtils.removeTab(tab2);
+});
+
 add_task(async function test_tabGroupSelect() {
   let tab1 = BrowserTestUtils.addTab(gBrowser, "about:blank");
   let tab2 = BrowserTestUtils.addTab(gBrowser, "about:blank");