1927910 - Drag and drop tab to create group creates multiple colour changes

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

[Amy Lee [:amylee] UX]

Attachment: tab colour.mov

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

Drag 1 tab over another tab slowly in the tab strip to trigger tab group creation

Expected behavior [What should have happened?]:

When tab group creation is triggered, a coloured highlight based on an auto-assigned tab group colour show appear over tab 1 and tab 2

Actual behavior [What actually happened?]:

As you drag tab 1 over tab 2 the selected tab group colour changes multiple times as you move the tab

See screen recording

Comment 1

[Stephen Thompson [:sthompson]]

:amylee I see how this can happen: there is a fallback for the tab group context menu where it will choose one of the colors at random if all of the colors have been used at least once. https://searchfox.org/mozilla-central/source/browser/components/tabbrowser/content/tabgroup-menu.js#196-201 I can reproduce the color cycling behavior by creating 9 tab groups and then trying to combine two ungrouped tabs. I'm not sure how you were able to trigger that behavior with no existing tab groups...

I think this is probably an unintended side effect of reusing the color picking behavior for both the tab group context menu and drag-drop. I can think of a couple of approaches:

1. When the drag-drop code enters the state where it can drop to create a tab group, grab a color and save that color on the drag-drop context. Keep using that color from the drag-drop context while the user is dragging. With this approach, if you had 10 tab groups already, then dragging a tab around would use a random color (say, red) when dragged over any standalone tabs. If the user stopped dragging without creating a new tab group and then grabbed another tab to drag, then that might use a different color (say, green).
2. Make the tab group context menu "next unused color" function deterministic instead of random. For example, if there are 9 tab groups, then creating the 10th tab group would always default to the blue color, the 11th would default to purple, etc. With this approach, if you had 10 tab groups already, then any tabs that you drag around any number of times would always use the same color until the number of tab groups changed.

Do you have a preference? Do you have a particular interest in random selection?

I think (1) is relatively simple, so I will post a patch with that.

Comment 2

[Stephen Thompson [:sthompson]]

Attachment: Bug 1927910 - limit color changes when dragging to create tab groups r=dao,dwalker,jswinarton

When all 9 tab group colors are in use, drag-drop code picks a random tab group color to highlight the tab drop target if the drop will create a tab group. The random pick is executed once per dragover event, so the color appears to change a few times a second. Kinda cool but distracting.

This patch chooses the next new tab group color on dragstart, saves that color on the drag data, and uses that color for all of the presentation + logic for creating a new tab group from a drop. The color will still be random when there are many tab groups, but the color will stay the same throughout a single drag operation.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/056f9c5d8fee
limit color changes when dragging to create tab groups r=dao,tabbrowser-reviewers

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/056f9c5d8fee