2029534 - tabs.group() to a new window prepends a split view instead of appending it

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect, P2)

Description

[Rob Wu [:robwu]]

In bug 2029099, I wrote a unit test that a split view can join a tab group in another window. The unit test however failed and demonstrates that the adoption of a split view results in the split view unexpectedly appearing at the left of the tab group, instead of at the right.

This is because internally, group.addTabs is called, which ought append tabs to a tab group.
In the regular case, tabIndex is used to select the last tab, but in the split view case, somehow elementIndex is used instead by addTabs (source). Changing it to tabIndex causes the unit test to pass.

I'm splitting the fix from bug 2029099, because the implementation changes in the other bug are 100% extension code, whereas this bug is caused by the implementation of tabbrowser. This logic was added relatively recently in bug 2006386.

Comment 1

[Rob Wu [:robwu]]

Btw, changing elementIndex to tabIndex does not cause the test from bug 2029099 to fail, because the test there only checks whether the split view was moved, not where exactly it moved to: https://searchfox.org/firefox-main/rev/0989a082704f0bda8d370ccd57402645d834757e/browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_tabContextMenu.js#567-572

Perhaps the exact location is not significant for the UI, but in the extension API we want consistent behavior, so I'm going to propose a patch that changes elementIndex to tabIndex.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 2029534 - Consistently use tabIndex in group.addTab

The regular (non split view) case already uses tabIndex, which
results in the desired effect of appending tabs to a group.

In the split view case, the use of elementIndex instead of tabIndex
caused the split view to be inserted before ab existing group instead of
appended. Changing this to tabIndex fixes this inconsistency.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/548f3cfcc212
https://hg.mozilla.org/integration/autoland/rev/ebdbf7eb75e6
Consistently use tabIndex in group.addTab r=tabbrowser-reviewers,nsharpley

Comment 4

[Silaghi Andreea]

https://hg.mozilla.org/mozilla-central/rev/ebdbf7eb75e6