bug 1323311 make tabs.move on multiple tabs more reliable r=kmag
MozReview-Commit-ID: 9o4huF1f60g
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -511,17 +511,16 @@ this.tabs = class extends ExtensionAPI {
async removeCSS(tabId, details) {
let tab = await promiseTabWhenReady(tabId);
return tab.removeCSS(context, details);
},
async move(tabIds, moveProperties) {
- let index = moveProperties.index;
let tabsMoved = [];
if (!Array.isArray(tabIds)) {
tabIds = [tabIds];
}
let destinationWindow = null;
if (moveProperties.windowId !== null) {
destinationWindow = windowTracker.getWindow(moveProperties.windowId);
@@ -534,49 +533,60 @@ this.tabs = class extends ExtensionAPI {
/*
Indexes are maintained on a per window basis so that a call to
move([tabA, tabB], {index: 0})
-> tabA to 0, tabB to 1 if tabA and tabB are in the same window
move([tabA, tabB], {index: 0})
-> 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) {
// If the window is not specified, use the window from the tab.
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;
}
// 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 = nativeTab.pinned ? insertionPoint <= numPinned : insertionPoint >= numPinned;
if (!ok) {
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
// to the new window.
nativeTab = gBrowser.adoptTab(nativeTab, insertionPoint, false);
} else {
// 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);
}
return tabsMoved.map(nativeTab => tabManager.convert(nativeTab));
},
duplicate(tabId) {
let nativeTab = tabTracker.getTab(tabId);
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -103,16 +103,17 @@ support-files =
[browser_ext_tabs_executeScript_good.js]
[browser_ext_tabs_executeScript_bad.js]
[browser_ext_tabs_executeScript_no_create.js]
[browser_ext_tabs_executeScript_runAt.js]
[browser_ext_tabs_getCurrent.js]
[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_ext_tabs_onHighlighted.js]
[browser_ext_tabs_onUpdated.js]
[browser_ext_tabs_query.js]
[browser_ext_tabs_reload.js]
[browser_ext_tabs_reload_bypass_cache.js]
new file mode 100644
--- /dev/null
+++ b/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);
+ }
+});