1971237 - Tab group preview hover usability improvements

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement, P2)

Description

[Stephen Thompson [:sthompson]]

1. It should be possible to hover over the tab group label and then move the mouse into the panel without the panel closing automatically
2. Look into whether the tab group hover preview panel could remain open if the mouse is not hovering over the tab group label nor the panel but is very close to either one. Microsoft Edge was mentioned as an example for this behavior. Neil Deakin designed the current XUL popup architecture and so may be a good resource.
3. There is a debounce in Tab Hover Preview – look at this for inspiration

Based on our early experiences with how this menu feels, we may want to file followup bugs for more specific improvements.

Comment 1

[Jeremy Swinarton]

While working on bug 1971233, I identified a number of usability improvements that will need to be made to this component to bring it up to the level of quality that the existing tab hover preview (THP) component is at. (A few of these are already working, but we need tests for them.)

1. Don't show previews in background windows
2. Decide on an appropriate delay before showing the panel. If the user navigates away from the panel and back quickly by accident, the delay should not be reactivated.
3. There should be an animation when the panel appears that is similar to the animation on THP.
4. It should be possible to switch between this panel and THP without animation jitters.
5. Dragging a tab group should deactivate the preview.
6. Clicking to expand the tab group should deactivate the preview.
7. Other open panels should prevent the preview panel from opening.
   8. The panel should be hidden if the URL bar receives input.
   9. If the tab strip is overflowing, and is scrolled (e.g. via middle wheel), the panel should close and the scroll should work properly.
8. The panel should be positioned in a similar location to THP. (Verify with UX)
9. It should be possible to navigate through the menu using the keyboard if the panel is active.

In Stephen's original comment, item 1 was already completed in bug 1971233. Item 3 is the same as item 2 in my list, I just wrote it out a bit more explicitly.

Once this is done I will also look into refactoring all the code that is shared between THP and the new tab group hover preview panel and factor it out into a shared class.

Comment 2

[Jeremy Swinarton]

Adding a useful comment from Dao on the first patch: in addition to factoring out code into a shared class, there may also be certain code (like tracking which other popups are open, for example) that will be running twice unless they are somehow taken out of this code and placed in some shared location, like on the tabContainer. I will look into this as part of this patch.

Comment 3

[Jeremy Swinarton]

Attachment: Bug 1971237: Refactor and improvements to tab group hover preview r?dao,dwalker,sthompson

Comment 4

[Jeremy Swinarton]

There are some slight modifications to the requirements laid out in https://bugzilla.mozilla.org/show_bug.cgi?id=1971237#c1.

Both 8 and 9 are behaviours that I originally identified as part of tab hover preview. In the tab group case, when the panel is active and interactive, it consumes all clicks and keyboard input by definition. This means it's not possible to e.g. enter things into the URL bar while this panel is active. Similarly, scrollwheel events are captured by the panel to allow it to scroll in the event of overflow, so it is not possible to scroll an overflowing tab strip via middle wheel. Therefore both 8 and 9 aren't possible to implement (and I don't think we would want to).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/234e2e63cdf7
https://hg.mozilla.org/integration/autoland/rev/65ed1ccea649
Refactor and improvements to tab group hover preview r=sthompson,tabbrowser-reviewers

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/56797b57a9b5
https://hg.mozilla.org/integration/autoland/rev/4ad07beec302
Revert "Bug 1971237: Refactor and improvements to tab group hover preview r=sthompson,tabbrowser-reviewers" for causing bc failures @ browser_tab_preview.js

Comment 7

[Sandor Molnar[:smolnar]]

Backed out for causing bc failures @ browser_tab_preview.js

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/4ad07beec3029a7a8a905b741dc90a33bc61c2e2

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | browser/components/tabbrowser/test/browser/tabs/browser_tab_preview.js | Uncaught exception in test bound tabGroupPanelClickElementSwitchesTabs

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d0f9ca001f1f
https://hg.mozilla.org/integration/autoland/rev/5b734293752e
Refactor and improvements to tab group hover preview r=sthompson,tabbrowser-reviewers

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0f7db4cd3710
https://hg.mozilla.org/integration/autoland/rev/8c6d86178688
Revert "Bug 1971237: Refactor and improvements to tab group hover preview r=sthompson,tabbrowser-reviewers" for causing mochitests failures in browser_tab_preview.js.

Comment 10

[Serban Stanca [:SerbanS]]

Reverted this because it was causing mochitests failures in browser_tab_preview.js.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/tabbrowser/test/browser/tabs/browser_tab_preview.js | Found an unexpected tab at the end of test run: about:mozilla -

Comment 11

[Jeremy Swinarton]

Hi :SerbanS, based on the push link you sent, this seems to be an intermittent failure that happens around 50% of the time. Is there a reason we can't land this and address the intermittent as a follow-up bug? I don't have access to a machine that runs MacOS 10.15 and I'm not able to reproduce this on any other OS, which means I think this will take some time to resolve. The affected functionality is also behind a feature flag that is only enabled by default on Nightly, so if this turns out to be a user-facing issue, it will have low impact.

Comment 12

[Serban Stanca [:SerbanS]]

These two instances in particular caught my attention. In both cases doesn't seem to be only a 50% chance of happening. FWIW indeed it's happening only on MacOS macOS 10.15 opt. Maybe Aryx could give us an advice here if it's safe to have this frequent failure or if you need to dig deeper.

Comment 13

[Jeremy Swinarton]

Hi Sterban and Aryx,

I've been trying to reproduce this locally for the last few days and have come up short. I discussed with the team, and given that this test only affects a component that is gated behind a feature flag and is only available in Nightly, we feel it's better to disable the test and file a follow up (bug1982893) to address this after. This will give us more time with this feature in Nightly so we can address follow-up issues as they arise through user testing.

The other issue you pointed out, that fails as: browser/components/tabbrowser/test/browser/tabs/browser_tab_preview.js | Zero delay is set immediately after leaving tab strip - false == true) — I have seen this issue pop up intermittently since we originally launched tab hover preview last year. I thought we had an intermittent on file for this already, but I can't seem to find it. In any case, in my latest try run, this seems to fail around 20% of the time on MacOS.

I just queued this patch for landing again with the test identified in bug1982893, so the only remaining intermittent should be the zero delay one. Hope this is ok.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/574337c19b1f
https://hg.mozilla.org/integration/autoland/rev/8c9714043e5a
Refactor and improvements to tab group hover preview r=sthompson,tabbrowser-reviewers

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8c9714043e5a