1922268 - Clicking "New tab to right" in a context menu inadvertently adds a tab to a group, if the next tab to the right is in a group

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P2)

Description

[Jeremy Swinarton]

Attachment: Screen Recording 2024-10-02 at 10.01.21 AM.mov

Comment 2

[Stephen Thompson [:sthompson]]

This may not be a strict dupe. "Add Tab to Right" seems to use gBrowser.addAdjacentNewTab which ultimately uses gBrowser.addTab and gBrowser._insertTabAtIndex to insert a tab based on a tab index value.

https://bugzilla.mozilla.org/show_bug.cgi?id=1921830 uses gBrower.moveTabTo based on a tab index value.

Both moveTabTo and _insertTabAtIndex have their own logic for calculating how to place a tab in the DOM based on context.

Comment 3

[Stephen Thompson [:sthompson]]

With the tab browser in the state

A
B
Group
  C
  D

B has index 1 and C has index 2. Right-clicking B and clicking Add Tab to Right calls gBrowser.addAdjacentNewTab with tab B.

addAdjacentNewTab calls addTrustedTab with index 2 (B's tab._tPos == 1, the code calls addTrustedTab with tab._tPos + 1)

addTrustedTab forwards directly to addTab.

addTab calls _insertTabAtIndex with index 2 and empty values for ownerTab and openerTab.

_insertTabAtIndex grabs the tabAfter as this.tabs[index], so that picks tab C with index 2. Then it calls this.tabContainer.insertBefore(tab, tabAfter). Since tab C is nested under the tab group in the DOM, the insertBefore makes the new tab a sibling of C and therefore makes it contained within the tab group.

A
B
Group
  E
  C
  D

I think we'd want something more like

• if the tabAfter is in a group and there is no ownerTab nor openerTab, then we should do this.tabContainer.insertBefore(tab, tabAfter.group) to insert the new tab as a standalone tab before the tab group
• if the tabAfter is in a group and there is an ownerTab or openerTab, then
  • if the ownerTab/openerTab group is different from the tabAfter.group, we should assume the tab should be inserted into the ownerTab/openerTab group. Not really sure how we get to this case. It might be sufficient to insert as the last tab in the owner/opener's tab group, e.g. this.tabContainer.appendChild(ownerTab.group, tab)
  • if the ownerTab/openerTab group is the same as the tabAfter.group, this.tabContainer.insertBefore(tab, tabAfter); should work fine
• if there is no tabAfter, it's inserting to the end of the tab bar
  • if there is no ownerTab/openerTab, the new tab should stand alone, so this.tabContainer.insertBefore(tab, tabAfter); will work fine
  • if there is an ownerTab/openerTab
    • if ownerTab/openerTab has a group, presumably we're inserting the tab to the end of the group, so we'd do this.tabContainer.appendChild(ownerTab.group, tab)
    • if ownerTab/openerTab doesn't have a group, I think this.tabContainer.insertBefore(tab, tabAfter); still works fine

Comment 4

[Jeremy Swinarton]

Ok, that makes sense. I'll reopen this. Thank you for the comprehensive write-up, this is very clear!

Comment 6

[Stephen Thompson [:sthompson]]

Attachment: Bug 1922268: inserting tabs behaves with adjacent tab groups r=dao,dwalker,jswinarton

Fixes a number of edge cases where tabs created/inserted in the tab strip were incorrectly being placed inside of an adjacent tab group or incorrectly being placed outside of the intended tab group

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/818b70032c34
inserting tabs behaves with adjacent tab groups r=dao,tabbrowser-reviewers

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/818b70032c34

Comment 9

[Monica Chiorean]

On Win10x64 using Firefox 135.0a1 and 134.0b3 I installed 'Simple Tab Groups' add-on and If I open a new tab to the right after a group tab this opens a new tab in the group. Is this expected? Thank you.
A
tab group B
new tab C in group B

Comment 10

[Stephen Thompson [:sthompson]]

This bug affects Firefox's built-in tab group feature, which is still under development. This feature does not require any add-on.

In 134 and 135, you need to enable the browser.tabs.groups.enabled preference in about:config.

If you right-click on a tab inside of a tab group, then choosing "New Tab to Right" should open the new tab inside of the tab group. If you right-click on a standalone tab, then choosing "New Tab to Right" should open the new tab as a standalone tab.

Comment 11

[Monica Chiorean]

I was able to reproduce the issue on Win10x64 using Firefox 134.0a1(20241028212656) after enabling the pref. Thank you.
Verified as fixed on Win10x64/Ubuntu 24.04 using Firefox builds 134.0b5(20241204091850) and 135.0a1(20241205213207).