bug 1323311 make tabs.move on multiple tabs more reliable r=kmag

MozReview-Commit-ID: 9o4huF1f60g

--HG--
extra : rebase_source : 392718783a20d59860cf8315b1687985799f076e

browser/components/extensions/ext-tabs.js

@@ -516,7 +516,6 @@ this.tabs = class extends ExtensionAPI {
         },
 
         async move(tabIds, moveProperties) {
-          let index = moveProperties.index;
           let tabsMoved = [];
           if (!Array.isArray(tabIds)) {
             tabIds = [tabIds];
@@ -539,6 +538,7 @@ this.tabs = class extends ExtensionAPI {
                 -> tabA to 0, tabB to 0 if tabA and tabB are in different windows
           */
           let indexMap = new Map();
+          let lastInsertion = new Map();
 
           let tabs = tabIds.map(tabId => tabTracker.getTab(tabId));
           for (let nativeTab of tabs) {
@@ -546,7 +546,7 @@ this.tabs = class extends ExtensionAPI {
             let window = destinationWindow || nativeTab.ownerGlobal;
             let gBrowser = window.gBrowser;
 
-            let insertionPoint = indexMap.get(window) || index;
+            let insertionPoint = indexMap.get(window) || moveProperties.index;
             // If the index is -1 it should go to the end of the tabs.
             if (insertionPoint == -1) {
               insertionPoint = gBrowser.tabs.length;
@@ -562,7 +562,16 @@ this.tabs = class extends ExtensionAPI {
               continue;
             }
 
-            indexMap.set(window, insertionPoint + 1);
+            // If this is not the first tab to be inserted into this window and
+            // the insertion point is the same as the last insertion and
+            // the tab is further to the right than the current insertion point
+            // then you need to bump up the insertion point. See bug 1323311.
+            if (lastInsertion.has(window) &&
+                lastInsertion.get(window) === insertionPoint &&
+                nativeTab._tPos > insertionPoint) {
+              insertionPoint++;
+              indexMap.set(window, insertionPoint);
+            }
 
             if (nativeTab.ownerGlobal != window) {
               // If the window we are moving the tab in is different, then move the tab
@@ -572,6 +581,7 @@ this.tabs = class extends ExtensionAPI {
               // If the window we are moving is the same, just move the tab.
               gBrowser.moveTabTo(nativeTab, insertionPoint);
             }
+            lastInsertion.set(window, nativeTab._tPos);
             tabsMoved.push(nativeTab);
           }
 

browser/components/extensions/test/browser/browser-common.ini

@@ -108,6 +108,7 @@ support-files =
 [browser_ext_tabs_insertCSS.js]
 [browser_ext_tabs_removeCSS.js]
 [browser_ext_tabs_move.js]
+[browser_ext_tabs_move_array.js]
 [browser_ext_tabs_move_window.js]
 [browser_ext_tabs_move_window_multiple.js]
 [browser_ext_tabs_move_window_pinned.js]

browser/components/extensions/test/browser/browser_ext_tabs_move_array.js

@@ -0,0 +1,72 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(function* moveMultiple() {
+  let tabs = [];
+  for (let k of [1, 2, 3, 4]) {
+    let tab = yield BrowserTestUtils.openNewForegroundTab(window.gBrowser, `http://example.com/?${k}`);
+    tabs.push(tab);
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {permissions: ["tabs"]},
+    background: async function() {
+      function num(url) {
+        return parseInt(url.slice(-1), 10);
+      }
+
+      async function check(expected) {
+        let tabs = await browser.tabs.query({url: "http://example.com/*"});
+        let endings = tabs.map(tab => num(tab.url));
+        browser.test.assertTrue(
+          expected.every((v, i) => v === endings[i]),
+          `Tab order should be ${expected}, got ${endings}.`
+        );
+      }
+
+      async function reset() {
+        let tabs = await browser.tabs.query({url: "http://example.com/*"});
+        await browser.tabs.move(
+            tabs.sort((a, b) => (num(a.url) - num(b.url))).map(tab => tab.id),
+            {index: 0}
+        );
+      }
+
+      async function move(moveIndexes, moveTo) {
+        let tabs = await browser.tabs.query({url: "http://example.com/*"});
+        await browser.tabs.move(
+          moveIndexes.map(e => tabs[e - 1].id),
+          {index: moveTo});
+      }
+
+      let tests = [
+        // Start -> After first tab  -> After second tab
+        // [1, 2, 3, 4] -> [1, 4, 2, 3] -> [1, 4, 3, 2]
+        {"move": [4, 3], "index":  1, "result": [1, 4, 3, 2]},
+        // [1, 2, 3, 4] -> [2, 3, 1, 4] -> [3, 1, 2, 4]
+        {"move": [1, 2], "index":  2, "result": [3, 1, 2, 4]},
+        // [1, 2, 3, 4] -> [1, 2, 4, 3] -> [2, 4, 1, 3]
+        {"move": [4, 1], "index":  2, "result": [2, 4, 1, 3]},
+        // [1, 2, 3, 4] -> [2, 3, 1, 4] -> [2, 3, 1, 4]
+        {"move": [1, 4], "index":  2, "result": [2, 3, 1, 4]},
+      ];
+
+      for (let test of tests) {
+        await reset();
+        await move(test.move, test.index);
+        await check(test.result);
+      }
+
+      browser.test.notifyPass("tabs.move");
+    },
+  });
+
+  yield extension.startup();
+  yield extension.awaitFinish("tabs.move");
+  yield extension.unload();
+
+  for (let tab of tabs) {
+    yield BrowserTestUtils.removeTab(tab);
+  }
+});