1932489 - Tab group label should shift over when dragging first tab out of group

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

[Amy Lee [:amylee] UX]

Attachment: Drag tab out of group.mov

Steps to reproduce [can add screenshots or video recordings for reference]:

1. Create a tab group with 2 or more tabs and have an ungroup tab to the left of the group
2. Drag the first tab in the group (next to label) to the left and out of the group

Expected behavior [What should have happened?]:
The tab group shifts over to the right

Actual behavior [What actually happened?]:
Tab group doesn't move so the ungrouped tab on the left shifts over and overlaps with the tab group label (see video)

Comment 1

[tabmix.onemen]

Removing the first tab from a group shouldn't move that tab to the end of the group instead it should attach the group label to the next tab.

ungroupTab(tab) {
      if (!tab.group) {
        return;
      }

      this._handleTabMove(tab, () =>
        gBrowser.tabContainer.insertBefore(tab, tab.group.nextElementSibling)
      );
    }

add check here if the tab is the first in the group and not the last tab or single tab use gBrowser.tabContainer.insertBefore(tab, tab.group.previousElementSibling.nextElementSibling)

Comment 2

[Stephen Thompson [:sthompson]]

Bug 1926382 fixed most of the visual/interface issues with tab group labels during drag drop using a simple code change. The tradeoff was that the tab group label and the first tab in that tab group are being animated together, which doesn't allow for the first tab in the tab group to be dragged well.

I haven't been able to think of a clever shortcut to making tabs and tab group labels work together well in all cases.

Comment 3

[Stephen Thompson [:sthompson]]

(In reply to tabmix.onemen from comment #1)

Removing the first tab from a group shouldn't move that tab to the end of the group instead it should attach the group label to the next tab.

ungroupTab(tab) {
      if (!tab.group) {
        return;
      }

      this._handleTabMove(tab, () =>
        gBrowser.tabContainer.insertBefore(tab, tab.group.nextElementSibling)
      );
    }

add check here if the tab is the first in the group and not the last tab or single tab use gBrowser.tabContainer.insertBefore(tab, tab.group.previousElementSibling.nextElementSibling)

I think you are describing a different interaction. This bug is about dragging the first tab out of a tab group. It sounds like you are describing the behavior for the "Remove from Group" context menu option when right-clicking on a tab inside of a tab group.

I filed bug 1935307 to capture your recommendation. Thanks!

Comment 4

[Stephen Thompson [:sthompson]]

Attachment: Bug 1932489: drag/drop with `nodesFromRect` and a new "tab strip position" `elementIndex` r=dao,dwalker,jswinarton

Comment 5

[Stephen Thompson [:sthompson]]

Attachment: Bug 1932489: Drag/drop based on "element index" instead of tab index. r=jswinarton

Comment 6

[Jeremy Swinarton]

Attachment: WIP: Bug 1932489: First attempt to remove jitter from the tab group drag/drop patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8fda1ba5cd3
Drag/drop based on "element index" instead of tab index. r=tabbrowser-reviewers,jswinarton

Comment 10

[Sandor Molnar[:smolnar]]

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

Comment 11

[Rares Doghi, Desktop QA]

Verified as fixed in our latest Nightly build 136.0a1 (2025-01-30).

Comment 12

[tabmix.onemen]

The comment above greatestOverlap indicate
If the elements have different sizes, then this returns the larger overlap percentage.

but there is a 3rd case that you did not cover

*   p2            p1      p1+s1    p2+s2
*    |             |        |        |
*    ---------------------------------
*    =================================
*                   s2
*                  ==========
*                      s1
*                  ==========
*                   overlap

When element 1 is completely inside element 2 set overlapSize = s1
and the function greatestOverlap should return s1 in this case

Comment 13

[tabmix.onemen]

Dragging tabs is a mess in RTL browser

Comment 14

[Dão Gottwald [:dao]]

(In reply to tabmix.onemen from comment #13)

Dragging tabs is a mess in RTL browser

I've filed bug 1945440, will attach a patch shortly. Please file new bugs if you find more issues.

Comment 15

[Rares Doghi, Desktop QA]

Updating the Status flag for this issue since this was already verified, unless there is a reason why this was changed back to Fixed in which case I apologize for updating it again.