Bug 1612338 - Switch moveTabTo method to using tab as first argument. r=frg
authorIan Neal <iann_cvs@blueyonder.co.uk>
Wed, 18 Mar 2020 23:44:02 +0100
changeset 38524 8c6e06c57b4ed35172cb8d63f404fcd37bf86c24
parent 38523 22016eb593173db238091e9670d83b298c4b77fb
child 38525 fd76d1b09482266776695945e291fa484ac30ef7
push id400
push userclokep@gmail.com
push dateMon, 04 May 2020 18:56:09 +0000
reviewersfrg
bugs1612338
Bug 1612338 - Switch moveTabTo method to using tab as first argument. r=frg
suite/browser/tabbrowser.xml
--- a/suite/browser/tabbrowser.xml
+++ b/suite/browser/tabbrowser.xml
@@ -1838,17 +1838,17 @@
             b.webProgress.addProgressListener(filter, nsIWebProgress.NOTIFY_ALL);
             this.mTabListeners[position] = tabListener;
             this.mTabFilters[position] = filter;
 
             t.dispatchEvent(new Event("TabOpen",
               { bubbles: true, cancelable: false }));
 
             if (savedData.pos < position)
-              this.moveTabTo(position, savedData.pos);
+              this.moveTabTo(t, savedData.pos);
 
             if (this.tabs.length == 2 && this.isBrowserEmpty(this))
               this.removeCurrentTab({ disableUndo: true });
             else {
               this.selectedTab = t;
               this.mStrip.collapsed = false;
             }
             this.tabContainer._handleNewTab(t);
@@ -2376,24 +2376,22 @@
         <body>
           <![CDATA[
             document.getAnonymousElementByAttribute(this, "class",
                                                     "tab-drop-indicator-bar")
                     .collapsed = true;
             aEvent.stopPropagation();
 
             var newIndex = this.getDropIndex(aEvent);
-            var tabIndex;
             var dt = aEvent.dataTransfer;
             var draggedTab = dt.mozSourceNode;
             if (draggedTab && draggedTab.parentNode == this.tabContainer) {
-              tabIndex = this.getTabIndex(draggedTab);
-              if (newIndex > tabIndex)
+              if (newIndex > this.getTabIndex(draggedTab))
                 newIndex--;
-              this.moveTabTo(tabIndex, newIndex);
+              this.moveTabTo(draggedTab, newIndex);
               return;
             }
 
             var url;
             var linkHandler = Cc["@mozilla.org/content/dropped-link-handler;1"]
                                 .getService(Ci.nsIDroppedLinkHandler);
             try {
               // Pass true to disallow dropping javascript: or data: urls.
@@ -2429,28 +2427,26 @@
                 // We're adding a new tab, and we may want parent-tab tracking.
 
                 tab = this.loadOneTab(data.url, {
                   inBackground: bgLoad,
                   allowThirdPartyFixup: true,
                   triggeringPrincipal,
                 });
 
-                if (newIndex != this.tabs.length - 1)
-                  this.moveTabTo(this.tabs.length - 1, newIndex);
+                this.moveTabTo(tab, newIndex);
               }
               else {
                 // We're adding a new tab, but do not want parent-tab tracking.
                 tab = this.addTab(data.url, {
                   allowThirdPartyFixup: true,
                   triggeringPrincipal,
                 });
 
-                if (newIndex != this.tabs.length - 1)
-                  this.moveTabTo(this.tabs.length - 1, newIndex);
+                this.moveTabTo(tab, newIndex);
                 if (this.mCurrentTab != tab && !bgLoad)
                   this.selectedTab = tab;
               }
             });
           ]]>
         </body>
       </method>
 
@@ -2469,51 +2465,60 @@
                                                     "tab-drop-indicator-bar")
                     .collapsed = true;
             aEvent.stopPropagation();
           ]]>
         </body>
       </method>
 
       <method name="moveTabTo">
-        <parameter name="aSrcIndex"/>
-        <parameter name="aDestIndex"/>
+        <parameter name="aTab"/>
+        <parameter name="aIndex"/>
         <body>
           <![CDATA[
             // for compatibility with extensions
-            if (typeof(aSrcIndex) != "number")
-              aSrcIndex = this.getTabIndex(aSrcIndex);
-
-            if (aSrcIndex == aDestIndex)
+            if (typeof(aTab) == "number") {
+              oldPosition = aTab;
+              aTab = this.tabs[oldPosition];
+            } else {
+              oldPosition = this.getTabIndex(aTab);
+            }
+
+            if (oldPosition == aIndex)
               return;
 
-            this._browsers = null; // invalidate cache
             this.mLastRelatedIndex = 0;
 
-            this.mTabFilters.splice(aDestIndex, 0, this.mTabFilters.splice(aSrcIndex, 1)[0]);
-            this.mTabListeners.splice(aDestIndex, 0, this.mTabListeners.splice(aSrcIndex, 1)[0]);
+            this.mTabFilters.splice(aIndex, 0, this.mTabFilters.splice(oldPosition, 1)[0]);
+            this.mTabListeners.splice(aIndex, 0, this.mTabListeners.splice(oldPosition, 1)[0]);
 
             let wasFocused = (document.activeElement == this.mCurrentTab);
 
+            if (aIndex >= oldPosition)
+              ++aIndex;
             this.mCurrentTab._selected = false;
 
-            if (aDestIndex >= aSrcIndex)
-              ++aDestIndex;
-            var tab = this.tabContainer.insertBefore(this.tabs[aSrcIndex], this.tabs.item(aDestIndex));
+            // invalidate cache
+            this._browsers = null;
+
+            // Use .item() instead of [] because dragging to the end of the
+            // strip goes out of bounds: .item() returns null (so it acts like
+            // appendChild), but [] throws.
+            var tab = this.tabContainer.insertBefore(aTab, this.tabs.item(aIndex));
 
             this.mCurrentTab._selected = true;
 
             if (wasFocused)
               this.mCurrentTab.focus();
 
             this.tabContainer._handleTabSelect(false);
 
             tab.dispatchEvent(new UIEvent("TabMove",
               { bubbles: true, cancelable: false, view: window,
-                detail: aSrcIndex }));
+                detail: oldPosition }));
           ]]>
         </body>
       </method>
 
       <method name="getDropIndex">
         <parameter name="aEvent"/>
         <body>
           <![CDATA[
@@ -2574,54 +2579,52 @@
         </body>
       </method>
 
       <method name="moveTabForward">
         <body>
           <![CDATA[
             var tabPos = this.tabContainer.selectedIndex;
             if (tabPos < this.browsers.length - 1) {
-              this.moveTabTo(tabPos, tabPos + 1);
+              this.moveTabTo(this.mCurrentTab, tabPos + 1);
             }
             else if (this.arrowKeysShouldWrap)
               this.moveTabToStart();
           ]]>
         </body>
       </method>
 
       <method name="moveTabBackward">
         <body>
           <![CDATA[
             var tabPos = this.tabContainer.selectedIndex;
             if (tabPos > 0) {
-              this.moveTabTo(tabPos, tabPos - 1);
+              this.moveTabTo(this.mCurrentTab, tabPos - 1);
             }
             else if (this.arrowKeysShouldWrap)
               this.moveTabToEnd();
           ]]>
         </body>
       </method>
 
       <method name="moveTabToStart">
         <body>
           <![CDATA[
-            var tabPos = this.tabContainer.selectedIndex;
-            if (tabPos > 0) {
-              this.moveTabTo(tabPos, 0);
+            if (this.tabContainer.selectedIndex > 0) {
+              this.moveTabTo(this.mCurrentTab, 0);
             }
           ]]>
         </body>
       </method>
 
       <method name="moveTabToEnd">
         <body>
           <![CDATA[
-            var tabPos = this.tabContainer.selectedIndex;
-            if (tabPos < this.browsers.length - 1) {
-              this.moveTabTo(tabPos, this.browsers.length - 1);
+            if (this.tabContainer.selectedIndex < this.browsers.length - 1) {
+              this.moveTabTo(this.mCurrentTab, this.browsers.length - 1);
             }
           ]]>
         </body>
       </method>
 
       <method name="moveTabOver">
         <parameter name="aEvent"/>
         <body>