Closed Bug 1702629 Opened 4 years ago Closed 4 years ago

[Windows] Disabled menuitems are not skipped when navigating menus (context, select or menubar menus) with arrow keys but have no visible active state

Categories

(Core :: Widget: Win32, defect, P2)

Firefox 89
Desktop
Windows
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox89 --- verified
firefox90 --- verified

People

(Reporter: aminomancer, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [proton-context-menus] [priority:2b] [proton-uplift])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

  • Open a context menu that has disabled menuitems. i.e. right click on a video player on youtube
  • Use arrow keys to advance/rewind "focus"

Actual results:

  • Disabled menuitems can still gain _moz-menuactive attribute, even though there is no visual feedback to show that they are focused or hovered
  • That is, the css rules that style the active menuitems explicitly say :not([disabled="true"]), or they have some overriding rule like [_moz-menuactive="true"][disabled="true"] {color: GrayText; background-color: unset;}, so you can't tell whether a disabled button is "active" (in this case meaning keyboard-focused or cursor-hovered)
  • From a sighted user's point of view, it looks like you're advancing down the list, then all of a sudden the cursor showing the selected menu/menuitem just abruptly disappears, before reappearing when you navigate to the next non-disabled element in the list.

Expected results:

  • Those rules are clearly intended to prevent visual feedback that would mislead a sighted user into thinking the element is an active affordance.
  • But the functions that advance through menuitems do not abide by the same philosophy. This could mislead a blind or visually impaired user depending on their screen reader settings, but presumably it's also visually confusing and annoying for sighted users, (at least for me) since an element that can't be activated or interacted with in any way is being pseudo-focused, so it's consuming keyboard inputs. And meanwhile, there's no visual indication that it's holding focus except for the absence of highlighting on any other menuitems in the list.
  • There's a bug from 2 decades ago making the opposite argument. I know there are differing opinions on this, since the existence of the disabled button should still be apparent to screen readers. But there are ways to make disabled buttons visible to screen readers and still let the keyboard navigation walker skip them. Look at the customizable toolbar buttons for example.
  • Looking at the most modern parts of Firefox, I can't find any examples of focus behaving like the context menus do. But I can find a ton of counterexamples, like browser-toolbarKeyNav.js, PanelMultiView.jsm, even the basic button element.
  • Also, making disabled buttons known to screen readers is not the be-all, end-all, since the buttons do not provide any information as to why they are disabled. Even with vision, the most a user can glean from their existence is the fact that they could conceivably provide the named functionality in some other, unknown circumstance.
  • I still think that's valuable information so I wouldn't want them hidden, either visually or to screen readers, but I think the priority of visibility should be lower than that of avoiding the confusion caused by permitting disabled elements to be focused, especially when there's no visual feedback. Ideally, they should be read by screen readers but not focusable at all, just like toolbar buttons with open popups. To that end, context menus could use the same kind of node filter setup that the javascript key navigation modules use.

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Generated Content, Lists, and Counters' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Layout: Generated Content, Lists, and Counters
Product: Firefox → Core

FWIW, the behavior seems to vary across platforms: on macOS and Linux, disabled items in the menu get skipped completely, whereas on Windows the disabled item can still get focus. Whether that state is visible or not seems to depend on whether Proton context menus are enabled.

Moving this to the Firefox module as it feels like more of a front-end design issue than a core platform bug.

Component: Layout: Generated Content, Lists, and Counters → Menus
Product: Core → Firefox

This is governed by SkipNavigatingDisabledMenuItem in nsLookAndFeel, and the cocoa and gtk implementations always return "yes" for this, whereas the Windows widget implementation doesn't seem to have an entry for it.

Creating the pref ui.skipNavigatingDisabledMenuItem as a numeric pref and setting it to 1 gets the "expected" results.

Windows itself is terribly conflicted about the "right" thing here - in Notepad, disabled menuitems can be arrowed to, and in MS Edge, they cannot. But if you open the filepicker in MS Edge, that still has the notepad behaviour...

Either way though, the current behaviour where the items have no visual feedback but can be navigated with the arrow keys is wrong, so we should fix something here. The trick is... the visual behaviour is only enabled on Win10 with the default theme. We can encode the same thing into Windows widget code, but can't do that with a pref. I'd still want to make sure that doing so is OK for assistive technology. I expect so, as they have their own way of navigating items anyway, but I'd want to check. Asa, can you help with that?

Severity: -- → S2
Status: UNCONFIRMED → NEW
Component: Menus → Widget: Win32
Ever confirmed: true
Flags: needinfo?(asa)
OS: Unspecified → Windows
Product: Firefox → Core
Hardware: Unspecified → Desktop
Summary: Disabled menuitems are not skipped when navigating context/select menus with arrow keys → [Windows] Disabled menuitems are not skipped when navigating menus (context, select or menubar menus) with arrow keys but have no visible active state
Whiteboard: [proton-context-menus]
Priority: -- → P2
Whiteboard: [proton-context-menus] → [proton-context-menus] [priority:2b]

This seems to have come from bug 1692376 which was trying to mimic Windows Explorer behaviour. Note though that Explorer has the same bug -- right click on the desktop and 'Paste' option can be selected with the keyboard but has no visible state to indicate this. I would instead suggest that going without the change in bug 1692376 is better behaviour.

(In reply to Neil Deakin from comment #4)

Note though that Explorer has the same bug -- right click on the desktop and 'Paste' option can be selected with the keyboard but has no visible state to indicate this.

🤔

I would instead suggest that going without the change in bug 1692376 is better behaviour.

If we wanted to change this behaviour we'd need to align that with the UX team's expectations. There's no spec for disabled item hover states right now, and I think the current implementation was discussed with them...

I'd still like to understand if skipping disabled items in the arrow key navigation on Windows presents an a11y problem, as that seems like the more straightforward solution.

Yeah I think the crux of this issue is gonna be whether skipping disabled items prevents them from being read by screen readers. I can't speak for other screen readers or operating systems, but I use NVDA and its browse mode picks up menuitems whether they're disabled or focusable or not, just as long as they're not "hidden"

Gijs - We did not create a hover state spec for disabled items because one should not get a visible feedback from disabled item. I think whether we allow screen readers to read disabled menu items is a call for Asa and the accessibility team.

(In reply to aminomancer from comment #0)

  • But the functions that advance through menuitems do not abide by the same philosophy. This could mislead a blind or visually impaired user depending on their screen reader settings,

Screen readers read "unavailable" or similar when a disabled menu item gets focus; e.g. "Undo, unavailable".

I know there are differing opinions on this, since the existence of the disabled button should still be apparent to screen readers. But there are ways to make disabled buttons visible to screen readers and still let the keyboard navigation walker skip them.

That's somewhat controversial. They might be exposed to screen readers so that a screen reader user can see them with the review cursor, but at least for menus, screen reader users tend to rely solely on arrow key navigation. So, most screen reader users probably wouldn't be aware of the presence of disabled items if they weren't focused. That said, this is also true for other disabled controls (buttons, form fields, etc.), which explicitly don't get focus. Windows is inconsistent in this respect.

  • Looking at the most modern parts of Firefox, I can't find any examples of focus behaving like the context menus do.

That's true, though my understanding is that our "menus" are supposed to mimic native menus, and native Win32 menus do focus disabled items.

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

Screen readers read "unavailable" or similar when a disabled menu item gets focus; e.g. "Undo, unavailable".

That's the default state for NVDA, and presumably others I haven't tried, but I did say depending on the settings. Not everybody uses announce, especially on non-form inputs. And for NVDA or macOS users at least, there's an option to make the announcement when opening a new page or menu, not just when focusing. So it isn't necessary for it to consume focus for screen readers to detect it, assuming the user actually wants it to be read. Maybe a survey of screen reader users, or a shield study, would be useful here.

That's somewhat controversial. They might be exposed to screen readers so that a screen reader user can see them with the review cursor, but at least for menus, screen reader users tend to rely solely on arrow key navigation. So, most screen reader users probably wouldn't be aware of the presence of disabled items if they weren't focused. That said, this is also true for other disabled controls (buttons, form fields, etc.), which explicitly don't get focus. Windows is inconsistent in this respect.

I don't know if there's any data on what settings most screen reader users have, but the most popular screen readers can read unfocusable controls if the user chooses. I would expect that most screen reader users don't actually want to waste time reading disabled controls, which is why the option is disabled by default. So even if we assume that most screen reader users wouldn't notice disabled menu items without them holding focus, why is that a problem?

It doesn't look like there's any obvious consistency with respect to disabling versus hiding menu items anyway. There's logic mediating that of course, but from the user's point of view it may as well be arbitrary. Some are hidden, others are disabled. The hidden ones are not visible to sighted users of course, nor are they read by screen readers. The implication of disabling is that the control has no function in this specific context, (e.g. clicking on a specific video player on youtube.com) but in other contexts like it (e.g. video players in general) the control could have a function. But, just as an example, I don't see what good it is to have the "save video as" menu item read out on streams. Its visual presence isn't a problem, but when you're using a screen reader and they're being read out in sequence, it's just a nuisance.

I get the general principle though, I even referenced it in my original post several times. I know that some people think it's important to have information available about disabled affordances, as a general rule. But when we get down to brass tacks and specific menus, I can't see any specific examples that are actually useful. And if it's such a controversial issue, then maybe it's the kind of thing that should be user-configured. Lock the current behavior behind one of the many a11y prefs for example. There are already a ton of them, for behaviors that are far more obscure and minor than this one, so what's one more?

That's true, though my understanding is that our "menus" are supposed to mimic native menus, and native Win32 menus do focus disabled items.

Yeah, that didn't occur to me when I posted this bug, but it doesn't really change my mind. I get what you're saying, but why mimic native menus and nothing else? Over the last year all sorts of "natively" styled controls have been removed. Firefox looks nothing like windows, it has a grand total of zero UWP-style controls, and even the basic xul elements are supposed to have custom firefox styles. I can only speak for myself, but when I use firefox I don't have any expectation that it's going to behave exactly like microsoft's ancient programs, nor do I want it to.

If that's the most important consideration then I guess that's a reasonable benchmark, but there should at least be a preference to turn it off. Personally I don't place much value on mimicking microsoft's decades-old mistakes, and there are already countless ways that firefox deviates from native behavior. In particular, the whole rest of the UI not mimicking the windows focus behavior. From my point of view, the inconsistency between firefox controls is more important than having a potential inconsistency between firefox and windows. At least be internally consistent, right? And why should Windows users be punished when things behave reasonably and efficiently on Unix?

(In reply to aminomancer from comment #9)

That's the default state for NVDA, and presumably others I haven't tried, but I did say depending on the settings. Not everybody uses announce, especially on non-form inputs. And for NVDA or macOS users at least, there's an option to make the announcement when opening a new page or menu, not just when focusing.

I don't follow this. Your original point was that focusing disabled items "could mislead a blind or visually impaired user depending on their screen reader settings". I can't think of any setting on any screen reader that would prevent it from informing the user of the disabled/unavailable state, for focus or otherwise.

I don't know if there's any data on what settings most screen reader users have, but the most popular screen readers can read unfocusable controls if the user chooses.

I don't have data, but (based on my own experience as a screen reader user and also my observations as a screen reader developer for 10+ years) I'm reasonably sure that most screen reader users are not going to use their screen reader's review cursor functionality for menus. It's very different for something like a web page, where the placement of content is more complex and varied, but for menus, most are going to use arrow key navigation.

Anyway, this is starting to meander. From an a11y perspective, most non-menu things (and that includes things like the Firefox Hamburger menu) can't get focused when disabled. So, there's a reasonably strong argument to the effect that menus allowing that is inconsistent UX. The only reason we kept that behaviour is consistency with native Win32 menus. Therefore, the only risk here from the a11y perspective is that we're changing years-old behaviour (which does have some impact on users; muscle memory, etc.) and making it inconsistent with native. But I guess you could argue we're doing the same to the visual UX with Proton and we're clearly willing to accept that risk.

I'm not sure about jaws, but NVDA and macOS have say all/overview announce modes that only show names and roles. I don't think there's any reason that the review cursor wouldn't show "unavailable" but not everyone uses the review cursor since it can be overwhelming. If I remember correctly, on macOS the overview is enabled by default. I haven't used it myself in years, just prior experience, so correct me if I'm wrong about that. I also forgot to mention that the review cursor gets harder to use the more controls there are, so having useless controls read is discouraging.

I think you might have misunderstood me, I'm not under the impression that anyone would use primarily the mouse to navigate context menus. At least in NVDA the review cursor follows the mouse cursor, tab focus, and arrow keys. It's all one thing, just different ways of moving the highlighter. If tab/arrow key focus skips disabled items, then they won't be read at all unless the user hovers them with the mouse or uses say all/overview. That's what I'm hoping for, disabled items shouldn't be read as if they were interactible just like they shouldn't be focused.

I completely agree about changing very old behavior, especially behavior that impacts users who will have the hardest time finding and changing preferences, so it's definitely not a minor thing. I don't know what the policy is for handling things like that. It seems like the least harm would be done by leaving the current state as the default behavior, and just adding a preference to negate it. But I'm just not sure if users actually want disabled items to be visible in the first place. If they were just hidden altogether would anyone even notice, let alone complain? There are already hundreds of menus in the content area context menu that are disabled in a given context and we don't think anything of it since we don't notice that they're there in the first place without looking at the source code lol.

I think Edge does the best thing for keyboard users with its hamburger and context menus (of the various Microsoft Windows 10 experiences I've tested.) They don't have a mouse hover highlight for disabled menu items but they do have a keyboard highlight for disabled menu items. We may not have the time or be willing to take the risk to differentiate between mouse and keyboard based on where we are in the development cycle but I do think adding the disabled highlight (for both mouse and keyboard) to get closer to the Edge behavior would be good for sighted keyboard users while having no impact on screen reader users.

Flags: needinfo?(asa)

(In reply to Asa Dotzler [:asa] from comment #12)

I think Edge does the best thing for keyboard users with its hamburger and context menus (of the various Microsoft Windows 10 experiences I've tested.)

I just realized that Edge canary, which I've been testing on, is different from Edge release. I think the Edge canary behavior is better.

I played around with Edge Canary and found the behavior is different if NVDA is running or not (may require a restart of Edge Canary).

If NVDA is running before Edge Canary is started, keypresses in the menu will show disabled menuitems as active. This works with either Alt+F to open the application menu or mouse clicks on the application menu button, then keypresses.

If NVDA is not running before Edge Canary is started, keypresses in the menu will skip disabled menuitems.

UX recommendation: If we want to have a disabled hover state, we recommend matching the pattern:
• disabled menu item 40% opacity of active menu item
• disabled hover state = 40% opacity of active hover state

Note: After talking with Asa, it sounds like ideally, in the future we could work to make the disabled items skipped for most folks and only active for screen reader folks... which would improve the experience for all.

I like that idea of a different behavior for screen reader users, wasn't aware that was even feasible.

See Also: → 1710948
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

(In reply to aminomancer from comment #16)

I like that idea of a different behavior for screen reader users, wasn't aware that was even feasible.

My understanding (from off-bug discussions) is that this would be a different behaviour for keyboard users (vs mouse users). I would not support differentiating behaviour for screen reader users specifically.

Oh, no, sorry, I missed comment 14. I don't think screen reader sniffing and changing behaviour here is a good idea. If we're going to drop disabled menu items for keyboard users, I think we should drop it for everyone and be willing to deal with the potential user griping this causes. As per comment 10, there's a strong argument in support of this; we just have to accept that we're deliberately inconsistent with native Win32.

Keyboard navigation is what this entire bug is about. I don't see any problem with the mouse behavior as it currently exists. The problem with keyboard behavior is that menuitems are selected in sequence when using a keyboard. When using a mouse, it doesn't matter whether disabled menuitems can be made active or not, since menuitems are selected purely by mouse coordinates either way.
I was referring to this comment:

I played around with Edge Canary and found the behavior is different if NVDA is running or not (may require a restart of Edge Canary).

If NVDA is running before Edge Canary is started, keypresses in the menu will show disabled menuitems as active. This works with either Alt+F to open the application menu or mouse clicks on the application menu button, then keypresses.

If NVDA is not running before Edge Canary is started, keypresses in the menu will skip disabled menuitems.

(In reply to aminomancer from comment #20)

I was referring to this comment:

I played around with Edge Canary and found the behavior is different if NVDA is running or not (may require a restart of Edge Canary).

Yeah, that's the comment I meant I'd missed. I still don't think it's a good idea. Altering behaviour based on the presence of a screen reader is almost never a good idea.

That said, I'm confused by comment 12:

I think Edge does the best thing for keyboard users with its hamburger and context menus (of the various Microsoft Windows 10 experiences I've tested.) They don't have a mouse hover highlight for disabled menu items but they do have a keyboard highlight for disabled menu items.

This is what we're implementing for 89, right?

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

(In reply to aminomancer from comment #20)

I was referring to this comment:

I played around with Edge Canary and found the behavior is different if NVDA is running or not (may require a restart of Edge Canary).

Yeah, that's the comment I meant I'd missed. I still don't think it's a good idea. Altering behaviour based on the presence of a screen reader is almost never a good idea.

I would like to take this discussion to bug 1710948 which I filed about this idea; we can morph that as appropriate.

That said, I'm confused by comment 12:

I think Edge does the best thing for keyboard users with its hamburger and context menus (of the various Microsoft Windows 10 experiences I've tested.) They don't have a mouse hover highlight for disabled menu items but they do have a keyboard highlight for disabled menu items.

This is what we're implementing for 89, right?

No. We don't have a straightforward way of distinguishing mouse and keyboard highlights in CSS right now - everything uses _moz-menuactive, which is set from C++ code. That attribute is also used by various other bits of C++ code (including code in accessible/ (!), though that's debug-only). 89 is already in beta, the last non-RC beta is next week, tomorrow is a day off for everyone, and doing a rewrite of how context menu highlighting works would be incredibly risky. We also wouldn't want to affect the old-style menus, which are still in use on Windows versions older than Windows 10, and for non-default Win10 themes, so we'd need to mirror the conditions under which we use the new CSS into the C++ code that deals with the highlighting... it's not doable right now.

The patch I've attached to this bug just re-introduces visual highlight styles for disabled items, for both mouse and keyboard users. This is what we did prior to MR1/proton, and it addresses the original concern in this bug (of invisible highlights with the keyboard), and we can then consider doing something more elaborate in bug 1710948.

(FWIW, I don't know whether even this patch will get uplifted to 89 anymore at this point - it's taken quite a while to get to a decision and as noted above, we're quickly running out of time for 89.)

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

(FWIW, I don't know whether even this patch will get uplifted to 89 anymore at this point - it's taken quite a while to get to a decision and as noted above, we're quickly running out of time for 89.)

Shilpa, I've just had r+ from Jared and requested landing of the patch on autoland. To try to avoid delays for uplift requests here, could you clarify how PM feels about uplifting this (small, low-risk, CSS only change, which makes the new menus work properly for keyboard users instead of making it impossible to see what menuitem is selected) to 89, assuming it lands without issues and QA signs off on it?

Flags: needinfo?(smohanty)

Hi Gijs, thanks for the context, let's proceed ahead with the uplift.

Flags: needinfo?(smohanty) → needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0bd649c1f0f4 give disabled menuitems in the Windows custom-style context menu a hover background colour, r=jaws

Comment on attachment 9221702 [details]
Bug 1702629 - give disabled menuitems in the Windows custom-style context menu a hover background colour, r?jaws

Beta/Release Uplift Approval Request

  • User impact if declined: Context menu keyboard navigation does not work in a usable way, because the user cannot see which entry is selected/focused
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is low-to-medium (but I can't pick that...). This adds a different CSS background colour to disabled items that are selected/focused with the keyboard, and nothing else. It's a relatively small, CSS-only patch. So that's all low risk.

The reason I'm a little more nervous is around web content select dropdowns, which share these styles and where we've had to make repeated adjustments to cope with web content styles (bug 1699399, bug 1703810, one or two others). I believe these new styles are lower-specificity than the web content select dropdowns, and actually the same specificity as existing rules with similar effects (but transparent background instead of opaque) so in principle there shouldn't be any fallout - but the proof, as they say, is always in the pudding...

  • String changes made/needed: None
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #9221702 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9221702 [details]
Bug 1702629 - give disabled menuitems in the Windows custom-style context menu a hover background colour, r?jaws

Approved for 89.0b13.

Attachment #9221702 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
QA Whiteboard: [qa-triaged]
Whiteboard: [proton-context-menus] [priority:2b] → [proton-context-menus] [priority:2b] [proton-uplift]

Verified that using latest Nightly 90.0a1 and Firefox 89.0b13 there is now a hover effect for the disabled items inside context menus on Windows 10.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1712261
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: