Bug 1235571: Add an `adoptTab` method to <tabbrowser> and remove duplicated code. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 30 Jan 2016 20:46:13 -0800
changeset 327795 955de39986cc7f28be4db35aaa6d48576a55d37c
parent 327358 43d3300fa0a9163592bdadb421e2fabfeb92880b
child 513761 03bff77c1865b4a7b1dffccdc994f231a09e7ce5
push id10304
push usermaglione.k@gmail.com
push dateMon, 01 Feb 2016 23:57:40 +0000
reviewersbillm
bugs1235571
milestone47.0a1
Bug 1235571: Add an `adoptTab` method to <tabbrowser> and remove duplicated code. r?billm
browser/base/content/tabbrowser.xml
browser/components/extensions/ext-tabs.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2931,16 +2931,66 @@
             if (nextTab)
               this.moveTabTo(this.mCurrentTab, nextTab._tPos);
             else if (this.arrowKeysShouldWrap)
               this.moveTabToStart();
           ]]>
         </body>
       </method>
 
+      <!-- Adopts a tab from another browser window, and inserts it at aIndex -->
+      <method name="adoptTab">
+        <parameter name="aTab"/>
+        <parameter name="aIndex"/>
+        <parameter name="aSelectTab"/>
+        <body>
+        <![CDATA[
+          // Swap the dropped tab with a new one we create and then close
+          // it in the other window (making it seem to have moved between
+          // windows).
+          let newTab = this.addTab("about:blank");
+          let newBrowser = this.getBrowserForTab(newTab);
+          let newURL = aTab.linkedBrowser.currentURI.spec;
+
+          // If we're an e10s browser window, an exception will be thrown
+          // if we attempt to drag a non-remote browser in, so we need to
+          // ensure that the remoteness of the newly created browser is
+          // appropriate for the URL of the tab being dragged in.
+          this.updateBrowserRemotenessByURL(newBrowser, newURL);
+
+          // Stop the about:blank load.
+          newBrowser.stop();
+          // Make sure it has a docshell.
+          newBrowser.docShell;
+
+          let numPinned = this._numPinnedTabs;
+          if (aIndex < numPinned || aTab.pinned && aIndex == numPinned)
+            this.pinTab(newTab);
+
+          this.moveTabTo(newTab, aIndex);
+
+          // We need to select the tab before calling swapBrowsersAndCloseOther
+          // so that window.content in chrome windows points to the right tab
+          // when pagehide/show events are fired.
+          if (aSelectTab)
+            this.selectedTab = newTab;
+
+          aTab.parentNode._finishAnimateTabMove();
+          this.swapBrowsersAndCloseOther(newTab, aTab);
+
+          // Call updateCurrentBrowser to make sure the URL bar is up to date
+          // for our new tab after we've done swapBrowsersAndCloseOther.
+          this.updateCurrentBrowser(true);
+
+          return newTab;
+        ]]>
+        </body>
+      </method>
+
+
       <method name="moveTabBackward">
         <body>
           <![CDATA[
             let previousTab = this.mCurrentTab.previousSibling;
             while (previousTab && previousTab.hidden)
               previousTab = previousTab.previousSibling;
 
             if (previousTab)
@@ -5756,52 +5806,18 @@
           // actually move the dragged tab
           if ("animDropIndex" in draggedTab._dragData) {
             let newIndex = draggedTab._dragData.animDropIndex;
             if (newIndex > draggedTab._tPos)
               newIndex--;
             this.tabbrowser.moveTabTo(draggedTab, newIndex);
           }
         } else if (draggedTab) {
-          // swap the dropped tab with a new one we create and then close
-          // it in the other window (making it seem to have moved between
-          // windows)
           let newIndex = this._getDropIndex(event, false);
-          let newTab = this.tabbrowser.addTab("about:blank");
-          let newBrowser = this.tabbrowser.getBrowserForTab(newTab);
-          let draggedBrowserURL = draggedTab.linkedBrowser.currentURI.spec;
-
-          // If we're an e10s browser window, an exception will be thrown
-          // if we attempt to drag a non-remote browser in, so we need to
-          // ensure that the remoteness of the newly created browser is
-          // appropriate for the URL of the tab being dragged in.
-          this.tabbrowser.updateBrowserRemotenessByURL(newBrowser,
-                                                       draggedBrowserURL);
-
-          // Stop the about:blank load
-          newBrowser.stop();
-          // make sure it has a docshell
-          newBrowser.docShell;
-
-          let numPinned = this.tabbrowser._numPinnedTabs;
-          if (newIndex < numPinned || draggedTab.pinned && newIndex == numPinned)
-            this.tabbrowser.pinTab(newTab);
-          this.tabbrowser.moveTabTo(newTab, newIndex);
-
-          // We need to select the tab before calling swapBrowsersAndCloseOther
-          // so that window.content in chrome windows points to the right tab
-          // when pagehide/show events are fired.
-          this.tabbrowser.selectedTab = newTab;
-
-          draggedTab.parentNode._finishAnimateTabMove();
-          this.tabbrowser.swapBrowsersAndCloseOther(newTab, draggedTab);
-
-          // Call updateCurrentBrowser to make sure the URL bar is up to date
-          // for our new tab after we've done swapBrowsersAndCloseOther.
-          this.tabbrowser.updateCurrentBrowser(true);
+          this.tabbrowser.adoptTab(draggedTab, newIndex, true);
         } else {
           // Pass true to disallow dropping javascript: or data: urls
           let url;
           try {
             url = browserDragAndDrop.drop(event, { }, true);
           } catch (ex) {}
 
           if (!url)
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -583,47 +583,41 @@ extensions.registerSchemaAPI("tabs", nul
             continue;
           }
 
           // If the window is not specified, use the window from the tab.
           let window = destinationWindow || tab.ownerDocument.defaultView;
           let windowId = WindowManager.getId(window);
           let gBrowser = window.gBrowser;
 
-          let getInsertionPoint = () => {
-            let point = indexMap.get(window) || index;
-            // If the index is -1 it should go to the end of the tabs.
-            if (point == -1) {
-              point = gBrowser.tabs.length;
-            }
-            indexMap.set(window, point + 1);
-            return point;
-          };
+          let insertionPoint = indexMap.get(window) || index;
+          // If the index is -1 it should go to the end of the tabs.
+          if (insertionPoint == -1) {
+            insertionPoint = gBrowser.tabs.length;
+          }
+
+          // We can only move pinned tabs to a point within, or just after,
+          // the current set of pinned tabs. Unpinned tabs, likewise, can only
+          // be moved to a position after the current set of pinned tabs.
+          // Attempts to move a tab to an illegal position are ignored.
+          let numPinned = gBrowser._numPinnedTabs;
+          let ok = tab.pinned ? insertionPoint <= numPinned : insertionPoint >= numPinned;
+          if (!ok) {
+            continue;
+          }
+
+          indexMap.set(window, insertionPoint + 1);
 
           if (WindowManager.getId(tab.ownerDocument.defaultView) !== windowId) {
             // If the window we are moving the tab in is different, then move the tab
             // to the new window.
-            let newTab = gBrowser.addTab("about:blank");
-            let newBrowser = gBrowser.getBrowserForTab(newTab);
-            gBrowser.updateBrowserRemotenessByURL(newBrowser, tab.linkedBrowser.currentURI.spec);
-            newBrowser.stop();
-            // This is necessary for getter side-effects.
-            void newBrowser.docShell;
-
-            if (tab.pinned) {
-              gBrowser.pinTab(newTab);
-            }
-
-            gBrowser.moveTabTo(newTab, getInsertionPoint());
-
-            tab.parentNode._finishAnimateTabMove();
-            gBrowser.swapBrowsersAndCloseOther(newTab, tab);
+            tab = gBrowser.adoptTab(tab, insertionPoint, false);
           } else {
             // If the window we are moving is the same, just move the tab.
-            gBrowser.moveTabTo(tab, getInsertionPoint());
+            gBrowser.moveTabTo(tab, insertionPoint);
           }
           tabsMoved.push(tab);
         }
 
         if (callback) {
           runSafe(context, callback, tabsMoved.map(tab => TabManager.convert(extension, tab)));
         }
       },