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

NEW
Unassigned

Status

WebExtensions
Compatibility
P5
normal
11 months ago
a month ago

People

(Reporter: YUKI "Piro" Hiroshi, Unassigned)

Tracking

Trunk

Firefox Tracking Flags

(firefox57 affected)

Details

(Reporter)

Description

11 months ago
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.
(Reporter)

Comment 1

11 months ago
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;
}
--------------------------------------------------------

Updated

11 months ago
Priority: -- → P5

Updated

a month ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.