tabGroups.move() requires index different to Google Chrome
Categories
(WebExtensions :: Compatibility, defect, P1)
Tracking
(firefox139+ fixed, firefox140 fixed)
People
(Reporter: yuki, Assigned: robwu)
References
(Blocks 1 open bug)
Details
(Whiteboard: [addons-jira])
Attachments
(4 files)
The method tabGroups.move() accepts index via the object given as the second argument, but we need to specify an index different from Chromium.
Steps to reproduce
Load the attached testcase as a temporary extension.
It prepares four tabs with two groups like:
- group0
- about:blank#0
- about:blank#1
- group1
- about:blank#2
- about:blank#3
and tries to move the group0 after group1 with the code: chrome.tabGroups.move(group0Id, { index: 2 }).
Expected result
On Chrome this works as expected and rearrange groups as:
- group1
- about:blank#2
- about:blank#3
- group0
- about:blank#0
- about:blank#1
Actual result
On Nightly 140.0a1 nothing changed.
We need to run chrome.tabGroups.move(group0Id, { index: 4 }) to rearrange groups like on Chrome.
Environment
I've confirmed this incompatibility with:
- Nightly 140.0a1 Build ID: 20250430092515
- Google Chrome 135.0.7049.115
| Reporter | ||
Comment 1•1 year ago
|
||
| Reporter | ||
Comment 2•1 year ago
|
||
| Reporter | ||
Comment 3•1 year ago
|
||
I've tried more with two groups on Chrome:
- group0
- tab0
- tab1
- group1
- tab2
- tab3
- tab4
To move the group0 next to the group1, I had to run chrome.tabGroups.move(group0, { index: 3 }) on Chrome. index:2 reports an error Cannot move the group to an index that is in the middle of another group. Thus the index parameter looks to mean the final index of the first tab of the moved group.
On the other hand, Firefox requires chrome.tabGroups.move(group0, { index: 5 }) on this case. The required index looks odd, because I just have to run chrome.tabs.move(tab0, { index: 4 }) when I hope to move the tab0 after the tab4. I can't understand why I have to specify index: 5 for tab groups...
| Reporter | ||
Comment 4•1 year ago
|
||
One more: Firefox does not report error when unavailable index is given to tabGroups.move(). This is also an incompatibility but I think it is not critical.
| Reporter | ||
Comment 5•1 year ago
|
||
I've understand that Chrome compatibility does not have highest priority on Firefox's WebExtensions API as described at the bug 1965057, and indeed some differences of APIs like tabGroups.onCreated/onRemoved/onMoved between Firefox and Chrome looks reasonable for extension authors, but I think the difference described here looks unreasonable. Until this bug is fixed, we extension authors look to need to do to move tab groups safely:
- Call
tabGoups.get()to get the label and color of the group. - Call
tabs.ungroup()for the group member tabs. - Move member tabs to the destination location.
- Call
tabs.group()for the member tabs.
| Assignee | ||
Comment 6•1 year ago
|
||
Chrome treats index as the desired index after moving., after the group is moved. This requires extensions to know where the tabs within the group came from.
Firefox treats index as the current position before which the tab group should be inserted. The idea behind this is that if an extension queries a tab, and wants to put a group before that tab, then they can just specify its tab.index as the index:
index 0 - before group0
group0
tab0
index 1 - inside group0
tab1
index 2 - after group0 / before group1
group1
tab2
index 3 - inside group1
tab3
index 4 - inside group1
tab4
index 5 - after group1 (no tab at index 5, but need value for "after last tab", is -1 enough?)
Given this, it is clear why Firefox does not change anything when you request a move to index 2.
Chrome's interpretation is undocumented but the source shines some light on it: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/extensions/api/tab_groups/tab_groups_api.cc;l=340-361;drc=c4d16e4480885ba044064093bc1b83e15136b9fa
- The intended outcome according to the source comment is "When moving to the right, adjust the target index for the size of the group, since the group itself may occupy several indices to the right."
- The actual implementation: takes the given
indexif before the tab group, otherwise adds the number of tabs in the group to the index. The resulting index is validated: error if past end of the tab strip or if position is in another group. - Given tabs
[0g, 1g, 2, 3, 4, 5], withtabGroups.move(g, { index: 3}):- Firefox's result is
[2, 0g, 1g, 3, 4, 5](moved to tab formerly at index 3). Chrome's output is[2, 3, 4, 0g, 1g, 5](moved to place that is now index 3).
- Firefox's result is
- The actual implementation: takes the given
- In Firefox and Chrome, moving a tab group to a different window has the same meaning:
indexis the current index in the new window (and simultaneously also the index of the group after moving). Interestingly, Chrome does not raise an error when the index is out of range in this case, unlike the same-window case, where it does.
(In reply to YUKI "Piro" Hiroshi from comment #5)
I've understand that Chrome compatibility does not have highest priority on Firefox's WebExtensions API as described at the bug 1965057
We do consider compatibility seriously. We don't do bug-for-bug compatibility though. I think that you meant to link to bug 1962475; Chrome's extension API currently follows their internal implementation closely, but they are willing to consider alternatives as expressed in https://issues.chromium.org/issues/414993460#comment8
| Assignee | ||
Comment 7•1 year ago
|
||
Given the above explanation (comment 6), which behavior seems more correct to you?
| Assignee | ||
Comment 8•1 year ago
|
||
For comparison, the tabs.move() API also allows bulk-move of tabs. In Firefox, the given index specifies the insertion point for the first tab of the list of tabs, and all other tabs are put after it. With this logic, it is possible for the tabs to end up at a lower position than given by index, if the other tabs to move shift.
Chrome's tabs.move() bulk-moves tabs with similar semantics for the first tab. The following tabs are placed after it (but in a buggy order: https://issues.chromium.org/issues/336984026).
| Assignee | ||
Comment 9•1 year ago
|
||
I'm going to work on a patch to change the semantics of "index" in tabGroups.move to match Chrome's: index should be the desired position after moving tabs.
This decision is based on the following:
- We have not shipped
tabGroups.moveto release yet, it is currently only on Beta (139). Now would be the best time to make breaking changes. - Chrome's
tabGroupsAPI has been around for a few years, and shipping different behavior makes it more difficult to maintain a cross-browser compatible extension. - This report (and https://issues.chromium.org/issues/336984026) and a quick internal poll shows that developers may expect
indexto be the desired position, rather than the initial position (before move).
Updated•1 year ago
|
| Assignee | ||
Comment 10•1 year ago
|
||
[Tracking Requested - why for this release]: tabGroups extension API was added in Firefox 139 (currently beta). This report shows a significant incompatibility in tabGroups.move (added in bug 1961660) with Chrome, which we plan to address in the next few days.
Updated•1 year ago
|
| Assignee | ||
Comment 11•1 year ago
|
||
To ensure that a group ends up at the position specified by index,
this patch adjusts the index when the tab group moves to the right.
Before this patch, the tab group appeared at a too low (left) index
because the original logic did not account for tabs shifting after a
repositioning to the right.
This also introduces stricter validation for moving tabs near pinned
tabs or other tab groups. Previously, the tabbrowser internals adjusted
the index as needed to fit adjacent to pinned tabs or groups. Now, the
extension API throws an error.
The new behavior matches developer expectations and Chrome's behavior:
https://bugzilla.mozilla.org/show_bug.cgi?id=1963825#c9
| Reporter | ||
Comment 12•1 year ago
|
||
Sorry for my delay, I've read the description of current API design on Firefox and I've understood why I had to specify the index 5 on the case.
Hmm, the chaos looks to came from the design decision on Chrome keeping compatibility of tabs API - tab groups have no index information and only tabs have, but moving of a tab group requires the destination position. Anyway, it is good news that the API will become compatible to existing implementation on Chrome. Thank you for reconsideration!
| Assignee | ||
Comment 13•1 year ago
|
||
To ensure that a group ends up at the position specified by index,
this patch adjusts the index when the tab group moves to the right.
Before this patch, the tab group appeared at a too low (left) index
because the original logic did not account for tabs shifting after a
repositioning to the right.
This also introduces stricter validation for moving tabs near pinned
tabs or other tab groups. Previously, the tabbrowser internals adjusted
the index as needed to fit adjacent to pinned tabs or groups. Now, the
extension API throws an error.
The new behavior matches developer expectations and Chrome's behavior:
https://bugzilla.mozilla.org/show_bug.cgi?id=1963825#c9
Original Revision: https://phabricator.services.mozilla.com/D249493
Updated•1 year ago
|
Comment 14•1 year ago
|
||
firefox-beta Uplift Approval Request
- User impact if declined: tabGroups extension API was added in Firefox 139 (currently beta). This report shows a significant incompatibility in tabGroups.move (added in bug 1961660) with Chrome, which this patch resolves.
- Code covered by automated testing: yes
- Fix verified in Nightly: no
- Needs manual QE test: no
- Steps to reproduce for manual QE testing: Not needed; API change covered by automated unit tests.
- Risk associated with taking this patch: Low
- Explanation of risk level: Change to extension API only (which landed in 139), not used anywhere else. Covered by unit tests
- String changes made/needed: No
- Is Android affected?: no
Comment 15•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Comment 16•1 year ago
|
||
| uplift | ||
Comment 17•1 year ago
|
||
| bugherder | ||
Description
•