1582433 - Tracking protection panel - tab focus on elements lost when moving mouse

Status: NEW

(Firefox :: Menus, defect, P3)

Description

[Cristian Fogel [:cfogel]]

Attachment: trackingProtection_FocusLostOnMouseMovement.gif

Affected versions

• 69.0, 69.0.1, 70.0b7, 71.0a1 (2019-09-18), 68.0.1ESR;

Affected platforms

• Windows 10, macOS 10.12, Ubuntu 16.04;

Steps to reproduce

1. Launch Firefox, access website such as https://www.youtube.com/
2. Click on the TP/info icon;
3. Press the TAB key a couple of times;
4. Move the mouse;

Expected result

• tabbed focus remains as it is;

Actual result

• element in the dropdown is no longer highlighted/marked as tab_selected;

Regression range

• First bad: build_date: 2019-04-15 11:01:28.453000 changeset: dc2fb967b01982c985558197b923c459811705c6
• Last good: build_date: 2019-04-14 changeset: ec1f3a922d56e0a9393415872ee1d30c2f895fd7
• Pushlog: URL
• Potential regressor: 1454865, adding 1477673 as well just in case since it seems to be work done in about the same area;

Additional notes

• attached recording with the issue;
• 60.9ESR not affected;

Comment 1

[Cristian Fogel [:cfogel]]

Hi James, mind confirming if the regressor accurate for this?
Thank you!

Comment 2

[Johann Hofmann [:johannh]]

I think this is sort of the expected behavior but I'll let Jamie speak to it.

Comment 3

[Marco Zehe (:MarcoZ)]

I am not Jamie, but I don't think mouse movement should affect keyboard focus like this. If you go to about:preferences, for example, and focus something, then move the mouse, focus doesn't get lost AFAIK. These panels shouldn't behave differently. Only thing that should affect focus is if another focusable item is clicked, or if it is clicked outside the panel and focus returns to where it was before the panel closed.

Comment 4

[Johann Hofmann [:johannh]]

Yeah, I agree it's unexpected, but I vaguely remember this behavior being explicitly coded into the panel for some reason. I don't really have time to dig into it right now, though.

Comment 5

[Cristian Fogel [:cfogel]]

From an UX point of view it wouldn't be to enjoyable if you're more comfortable with keyboard shortcuts.

Comment 6

[James Teh [:Jamie]]

Technically, this was most recently regressed by bug 1477673. However, that was only because we disabled the proper keyboard navigation code in bug 1522092 (Firefox 66), which was a temporary workaround causing a bunch of other problems (bug 1539976 among others). This issue was also present between Firefox 61 and 65.

This focus loss on mouse movement behaviour was implemented way back in bug 1354144, before my time with this code. I don't really understand why. I guess it might have something to do with mouse users not liking focus rings or the confusion of having focus and hover styles in two separate places? personally, I agree it's the wrong thing to do.

Gijs, do you have any idea why we do this? I ask you since you reviewed bug 1354144, but I realise that was a long time ago. :)

Comment 7

[James Teh [:Jamie]]

For reference, the code in question is here:
https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/browser/components/customizableui/PanelMultiView.jsm#1197

Comment 8

[:Gijs (he/him)]

(In reply to James Teh [:Jamie] from comment #6)

This focus loss on mouse movement behaviour was implemented way back in bug 1354144, before my time with this code. I don't really understand why. I guess it might have something to do with mouse users not liking focus rings or the confusion of having focus and hover styles in two separate places? personally, I agree it's the wrong thing to do.

Gijs, do you have any idea why we do this? I ask you since you reviewed bug 1354144, but I realise that was a long time ago. :)

I think it's not so much mouse users "not liking" anything as it genuinely being impossible to know what enter/space will do if 2 items look identically focused/hovered (which is what happens if you hover 2 items in a panelmultiview - maybe less so in the protections panel because fewer things are "normal" <toolbarbutton class=subviewbutton> elements, all with the same hover/focus styles, but...).

In the address bar autocomplete, we have faced the same problem, IIRC the solution there is the same, though it's possible that's changed as a result of the recent address bar work.

What's the proposed solution here? Because having mouse hover do nothing, or having 2 items that both look like they're hovered/focused, is not helpful...

Comment 9

[:Gijs (he/him)]

As an aside, the comparisons with about:preferences are flawed as far as most hamburger/panelmultiview views are concerned - they look and behave like menus, and in "normal" windows or mac native menus, it is absolutely the case that mousemove inside a menu removes other "focused" items - which don't normally use "real" focus - focus remains where it is when "normal" menus are opened. It doesn't here because they are not in fact real menus, but they look and behave like them in some ways...

Comment 10

[James Teh [:Jamie]]

(In reply to :Gijs (he/him) from comment #8)

I think it's not so much mouse users "not liking" anything as it genuinely being impossible to know what enter/space will do if 2 items look identically focused/hovered (which is what happens if you hover 2 items in a panelmultiview

But if you move the mouse, we just remove the focus; we don't focus the hovered item. So if button 1 is focused and you then move the mouse to button 2, pressing enter/space will do nothing; it does not activate button 2.

What's the proposed solution here? Because having mouse hover do nothing, or having 2 items that both look like they're hovered/focused, is not helpful...

IMO, if we want focus and hover to look the same, they should act the same, too. That is, instead of just throwing the focus to limbo for mousemove, we should also focus the hovered item.

(In reply to :Gijs (he/him) from comment #9)

in "normal" windows or mac native menus, it is absolutely the case that mousemove inside a menu removes other "focused" items

It removes other focused items, yes, but it does at least set focus to the hovered item. I can live with that if we're modelling it after menus, as above. Focus being thrown to limbo, on the other hand, is not consistent with native menus and is broken behaviour IMO.

Comment 11

[:Gijs (he/him)]

(In reply to James Teh [:Jamie] from comment #10)

(In reply to :Gijs (he/him) from comment #8)

I think it's not so much mouse users "not liking" anything as it genuinely being impossible to know what enter/space will do if 2 items look identically focused/hovered (which is what happens if you hover 2 items in a panelmultiview

But if you move the mouse, we just remove the focus; we don't focus the hovered item. So if button 1 is focused and you then move the mouse to button 2, pressing enter/space will do nothing; it does not activate button 2.

Yeah, I agree this is suboptimal; I was going to say this works correctly in the awesomebar, but in fact it looks like the latest incarnation of the awesomebar dropdown (in 70 beta anyway) has started distinguishing hover and keyboard selection state visually (the mouse one has a kind of grey background, the keyboard one has the menu-style blue/"active" background. I have no idea what it does for the exposed-to-AT "which of these items is selected" state, because AIUI there can only be one of those...

What's the proposed solution here? Because having mouse hover do nothing, or having 2 items that both look like they're hovered/focused, is not helpful...

IMO, if we want focus and hover to look the same, they should act the same, too. That is, instead of just throwing the focus to limbo for mousemove, we should also focus the hovered item.

That's... an interesting idea. I expect there will be more fallout from doing this, unfortunately - "focus follows mouse" for hovering -> focusing windows is an acknowledged thing. Doing it at an individual control level is kinda weird... We could limit the weirdness by having it only activate if you use the keyboard to move the focus in the panel first, but even so...

This all stems from the fact that we're implementing sorta-kinda-menus with sorta-kinda-menu-like UI behaviour, and now we're coming up to the limits of what makes sense there. Markus, can you take a look? We would either need different focus/hover state for all of the controls in panels, or some other sensible way for users to understand what item will activate when they click / press enter/space, or we would need to make mouse movement cause focus tracking inside panels... none of which feel particularly appetizing to me.

Comment 12

[:Gijs (he/him)]

I don't think we'll reasonably be able to do anything here for esr68.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Nihanth, could you prioritize this bug please and find an owner if necessary please? Thanks

Comment 14

[(inactive) Nihanth Subramanya [:nhnt11]]

I don't think this is actionable without someone with a11y expertise making a call and specifying what the right behavior is. Anyway, I was made the triage owner of this component very recently and will be going through the list next week, at which time I'll assign a priority if there has been an update from Markus.

Actually, as I write this I realize that this bug might be more comfortable living in Firefox::General or whatever the right component is for PanelMultiView... I'll think about it and move it if necessary when I look next week.

Comment 15

[James Teh [:Jamie]]

Closest component I know of for PanelMultiView is Firefox: Toolbars and Customization.

Comment 16

[Markus Jaritz [:designakt] (UX)]

(In reply to :Gijs (he/him) from comment #11)

Markus, can you take a look? We would either need different focus/hover state for all of the controls in panels, or some other sensible way for users to understand what item will activate when they click / press enter/space, or we would need to make mouse movement cause focus tracking inside panels... none of which feel particularly appetizing to me.

Adding Eric to help find a solution. Having focus and hover combined doesn't see optimal.

Comment 17

[Eric Pang [:epang] UX]

Attachment: keyboard-focus-state.png

IMO both the mouse hover state and the keyboard focus state should be shown.

The problem as discussed above is the identical mouse hover/keyboard focus state. If possible, we should define a new keyboard focused state for panel items. Are we be able to follow the pattern set in the search bar? Using a blue bar and white text for keyboard focused items? Attached a screen of how this would look.

Comment 18

[:Gijs (he/him)]

I think in principle the suggestion in comment #17 would work, but we may need to do more work for other controls in these popups (like the checkboxes and links in the identity/protection popups, or the radio buttons in the forget panel, etc.) if focus styles are not yet obvious enough. I'm also not sure what we'd do for a11y - Jamie, how does the awesomebar do this right now? Does AT "selected item" state follow keyboard selection or mouse or...?

Comment 19

[James Teh [:Jamie]]

(In reply to :Gijs (he/him) from comment #18)

I'm also not sure what we'd do for a11y - Jamie, how does the awesomebar do this right now? Does AT "selected item" state follow keyboard selection or mouse or...?

In the Awesomebar, accessibility focus and selection follows keyboard selection. It doesn't set DOM focus (as is standard for autocompletes), since that would take it away from the text input. Instead, it uses aria-activedescendant.

I don't think using aria-activedescendant is a good idea for PanelMultiView, though, as some controls there need to get DOM focus (e.g. inputs, menulists) and mixing DOM focus and aria-activedescendant is going to get us into a world of pain. It also makes event handling really painful, especially as some controls listen to keyboard events themselves for activation.

Comment 20

[Andrew Overholt [:overholt]]

In the regression triage meeting today this bug came up. We were wondering if P3 meant this wasn't going to be worked on in a near future release and therefore we could update the flags accordingly?

Comment 21

[Johann Hofmann [:johannh]]

I don't think this will be solved in the near future, but maybe I'm wrong. I'm quite certain that my team is not going to work on it, maybe Gijs or Jamie/Marco have the capacity to do it, since this affects more than just the protections popup.

Comment 22

[:Gijs (he/him)]

I don't think frontend folks have cycles for this right now. Maybe Jamie/Marco/Morgan do.

Comment 23

[James Teh [:Jamie]]

We don't have cycles right now either, especially as this is going to involve a lot of fiddly visual/CSS stuff.