1394477 - Wrongly ordered tabs moved by browser.tabs.move() across windows

Status: RESOLVED DUPLICATE of bug 1809364

(WebExtensions :: Compatibility, enhancement, P5)

Description

[YUKI "Piro" Hiroshi]

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/move
The API reference says that the first argument accepts an array of ids, then the first tab is placed at the position specified via "index" and others are placed next to the first tab in given order.

However, other tabs are wrongly ordered if they are moved across windows.

Testcase (you need to run it on any addon with "tabs" permission):
--------------------------------------------------------
async function testMoveMultipleTabsInOneWindow() {
  var window = await browser.windows.create();
  await browser.tabs.create({
    windowId: window.id,
    url: `about:blank?extra`
  });
  var indexes = [1, 2, 3, 4 ,5];
  var tabs = await Promise.all(indexes.map(index =>
               browser.tabs.create({
                 windowId: window.id,
                 url: `about:blank?${index}`
             })));
  var movedTabs = await browser.tabs.move(tabs.map(tab => tab.id), {
                         index: 1,
                         windowId: window.id
                       });
  await browser.windows.remove(window.id);

  var expected = indexes.map((index) => `tab${index}:${index}`).join('\n');
  var actual = movedTabs.map((tab, index) =>
                 `tab${index + 1}:${tab.index}`).join('\n');
  if (actual == expected)
    console.log('testMoveMultipleTabsInOneWindow SUCCESS');
  else
    console.log('testMoveMultipleTabsInOneWindow FAIL, ', actual);
}

async function testMoveMultipleTabsAcrossWindows() {
  var window1 = await browser.windows.create();
  var window2 = await browser.windows.create();
  var indexes = [1, 2, 3, 4 ,5];
  var tabs = await Promise.all(indexes.map(index =>
               browser.tabs.create({
                 windowId: window1.id,
                 url: `about:blank?${index}`
             })));
  var movedTabs = await browser.tabs.move(tabs.map(tab => tab.id), {
                         index: 1,
                         windowId: window2.id
                       });
  await browser.windows.remove(window1.id);
  await browser.windows.remove(window2.id);

  var expected = indexes.map((index) => `tab${index}:${index}`).join('\n');
  var actual = movedTabs.map((tab, index)
                 => `tab${index + 1}:${tab.index}`).join('\n');
  if (actual == expected)
    console.log('testMoveMultipleTabsAcrossWindows SUCCESS');
  else
    console.log('testMoveMultipleTabsAcrossWindows FAIL, ', actual);
}

testMoveMultipleTabsInOneWindow();
testMoveMultipleTabsAcrossWindows();
--------------------------------------------------------


Testcase for Google Chrome:
--------------------------------------------------------
function testMoveMultipleTabsInOneWindow() {
  chrome.windows.create(window => {
    chrome.tabs.create({
      windowId: window.id,
      url: `about:blank?extra`
    }, extraTab => {
      var indexes = [1, 2, 3, 4 ,5];
      Promise.all(indexes.map(index =>
        new Promise((resolve, reject) => {
          chrome.tabs.create({
            windowId: window.id,
            url: `about:blank?${index}`
          }, tab => resolve(tab));
        }))).then(tabs => {
        chrome.tabs.move(tabs.map(tab => tab.id), {
          index: 1,
          windowId: window.id
        }, movedTabs => {
          chrome.windows.remove(window.id);
          var expected = indexes.map((index)
                           => `tab${index}:${index}`).join('\n');
          var actual = movedTabs.map((tab, index)
                         => `tab${index + 1}:${tab.index}`).join('\n');
          if (actual == expected)
            console.log('testMoveMultipleTabsInOneWindow SUCCESS');
          else
            console.log('testMoveMultipleTabsInOneWindow FAIL, ', actual);
        });
      });
    });
  });
}

function testMoveMultipleTabsAcrossWindows() {
  chrome.windows.create(window1 => {
    chrome.windows.create(window2 => {
      var indexes = [1, 2, 3, 4 ,5];
      Promise.all(indexes.map(index =>
        new Promise((resolve, reject) => {
          chrome.tabs.create({
            windowId: window1.id,
            url: `about:blank?${index}`
          }, tab => resolve(tab));
        }))).then(tabs => {
        chrome.tabs.move(tabs.map(tab => tab.id), {
          index: 1,
          windowId: window2.id
        }, movedTabs => {
          chrome.windows.remove(window1.id);
          chrome.windows.remove(window2.id);
          var expected = indexes.map((index)
                           => `tab${index}:${index}`).join('\n');
          var actual = movedTabs.map((tab, index)
                         => `tab${index + 1}:${tab.index}`).join('\n');
          if (actual == expected)
            console.log('testMoveMultipleTabsAcrossWindows SUCCESS');
          else
            console.log('testMoveMultipleTabsAcrossWindows FAIL, ', actual);
        });
      });
    });
  });
}

testMoveMultipleTabsInOneWindow();
testMoveMultipleTabsAcrossWindows();
--------------------------------------------------------

Chrome reports "SUCCESS" for both cases but Firefox reprots "FAIL" for the across-window case.

Workaround to move tabs across windows in correct order:
--------------------------------------------------------
async function safeMoveTabsAcrossWindows(aTabIds, aMoveOptions) {
  return await Promise.all(aTabIds.map(async (aTabId, aIndex) => {
    var movedTab = await browser.tabs.move(aTabId, clone(aMoveOptions, {
      index: aMoveOptions.index + aIndex
    }));
    if (Array.isArray(movedTab))
      movedTab = movedTab[0];
    return movedTab;
  }));
}
--------------------------------------------------------
This works as expected.

Comment 1

[YUKI "Piro" Hiroshi]

Sorry, the workaround depends another function "clone". It is:
--------------------------------------------------------
function clone(aOriginalObject, aExtraProperties) {
  var cloned = {};
  for (let key of Object.keys(aOriginalObject)) {
    cloned[key] = aOriginalObject[key];
  }
  if (aExtraProperties) {
    for (let key of Object.keys(aExtraProperties)) {
      cloned[key] = aExtraProperties[key];
    }
  }
  return cloned;
}
--------------------------------------------------------

Comment 2

[Gregory Pappas [:gregp]]

Confirmed that the test cases in comment 0 return SUCCESS with the patch from bug 1809364.