2017148 - tabs.move() to a new window does not preserve split view

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect, P1)

Description

[Rob Wu [:robwu]]

The intention is to preserve split views when any of their tabs are moved around (bug 2016868).
While that works for tabs.move within windows, it does not for attempts to move tabs across windows: even when I bulk-move two tabs part of a split view, the split view disappears.

This difference in behavior is because of the way that ext-tabs.js implements tab movement: the tabs.move() implementation moves tabs sequentially. For movements within a window, the internal moveTabTo method preserves split views (due to the changes from bug 2004383). For movements across windows, we call adoptTab.

While moveTabTo tries to handle tabs, tab groups and tab splits all at once, the adoptTab method only handles tabs. Tab groups can be handled by adoptTabGroup, which is used by the tabGroups.move extension API. We have to do something similar for split views, e.g. by calling adoptSplitView method directly from ext-tabs.js.

P.S. browser.windows.create() with a tabId of a tab in a split view causes the split view as a whole to be adopted as desired. There is no unit test for that scenario, but I manually verified it to work.

Comment 1

[Rob Wu [:robwu]]

P.S. browser.windows.create() with a tabId of a tab in a split view causes the split view as a whole to be adopted as desired. There is no unit test for that scenario, but I manually verified it to work.

Not sure how exactly I tested before, but when I wrote a unit test it failed. I'll patch that as well.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 2017148 - tabs.move() keep split view when adopting tabs from other window

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 2017148 - windows.create() adopt split view instead of tab

• Change ext-windows.js, when it encounters a tab in a split view,
  pass its split view instead of the tab.

• Fix bug in tabbrowser adoptSplitView: Make sure that elementIndex is
  accepted even if 0; previously elementIndex 0 was ignored,
  resulting in the index to not be respected (e.g. when passed to
  adoptSplitView in browser-init.js.

• Fix bug in tabbrowser adoptSplitView: the selectTab option had jsdoc
  (likely copied from adoptTabGroup, which has similar logic), but the
  option was not implemented, causing the wrong tab to be selected when
  a split view was adopted in a new window.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4c5d7a64cea3
https://hg.mozilla.org/integration/autoland/rev/ffee0ee71a9f
tabs.move() keep split view when adopting tabs from other window r=rpl,tabbrowser-reviewers,sclements
https://github.com/mozilla-firefox/firefox/commit/c9076fb946fd
https://hg.mozilla.org/integration/autoland/rev/1a98b959fc9a
windows.create() adopt split view instead of tab r=rpl,tabbrowser-reviewers,sclements

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ffee0ee71a9f
https://hg.mozilla.org/mozilla-central/rev/1a98b959fc9a