Make WordPress Core

Opened 8 years ago

Closed 109 minutes ago

Last modified 101 minutes ago

#34668 closed defect (bug) (fixed)

Admin submenus can't be accessed via keyboard using screen readers

Reported by: abletec's profile abletec Owned by: alexstine's profile alexstine
Milestone: 6.5 Priority: normal
Severity: major Version:
Component: Toolbar Keywords: has-patch commit
Focuses: accessibility, javascript Cc:

Description

In order to *reliably* see the network admin dashboard, one has to hover over the 'My Sites' icon. Violates accessibility guidelines & makes it very difficult to access the network admin dashboard for screenreader users, & I would think many mobile users as well. I've tried example.com/wp-admin/network but this does not reliably bring up the network admin dashboard. Actually pressing the enter key while on the icon does nothing either. To reproduce, simply try accessing the network admin dashboard via keyboard. Sometimes I actually can, but that doesn't seem to happen consistently.

Attachments (4)

34668.patch (462 bytes) - added by Cheffheid 7 years ago.
Appends role="menuitem" to toolbar menu item parent ARIA attributes
34668.2.patch (2.1 KB) - added by sabernhardt 3 years ago.
adding menuitem role only to top-level links and including aria-expanded attribute
34668.3.patch (5.6 KB) - added by alexstine 2 years ago.
34668.4.patch (6.8 KB) - added by joedolson 2 weeks ago.
Refresh patch & collapse profile links

Download all attachments as: .zip

Change History (60)

This ticket was mentioned in Slack in #accessibility by cheffheid. View the logs.


8 years ago

#2 @joedolson
8 years ago

  • Focuses accessibility added

#3 @afercia
8 years ago

  • Component changed from Networks and Sites to Toolbar
  • Focuses javascript added; multisite removed

As far as I can tell, this is not specific to Network installations and happens in single installations too, for all the menu items in the admin bar and just when a screen reader is running. Putting it simply, JavaScript keyboard events don't work as many developers would expect when a screen reader is running.

Screen readers intercept key strokes (with a few exceptions) for their own needs. Browsers are simply unaware a keydown event occurred.

This kind of non-native interactions would need some serious ARIA to actually work with screen readers. I'm not even sure it can be done without an admin bar major refactoring. I'd propose to discuss this in the next accessibility team weekly chat in order to plan some action and verify available resources.

In the screenshots below:

  1. dumping the top menu item keydown event in the console, the link default action is prevented and the sub-menu opens:

https://cldup.com/nEWTcxxnht.png

  1. doing the same thing with NVDA running, no event and the link gets activated:

https://cldup.com/eca8qdjeRK.png

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


8 years ago

#5 @rianrietveld
8 years ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

#7 @afercia
7 years ago

  • Milestone changed from Future Release to 4.8

This is very noticeable with screen readers on Windows (JAWS, NVDA). Less noticeable with VoiceOver but still an issue if using Control-Option-Space to activate the menu. Moving to 4.8 to, at least, start a discussion and investigate on best options to fix it.

#8 @Cheffheid
7 years ago

It probably doesn't help here that the items with a submenu are also legitimate links. Because I think what NVDA does is pass the keydown through as a click event instead.

So, there seems to be a way to force NVDA and JAWS to pass keydown events. I've found this (old) list of the impact of the role attribute and its value on anchor tags (the first table): https://unobfuscated.blogspot.com/2013/05/event-handlers-and-screen-readers.html

As it turns out, setting a role of "menuitem" will restore the keydown event and allow the submenus to function.

I was going to ask if making these menus function the same way as the ones in the left sidebar (that is, make the submenus appear once there is focus on the parent) is acceptable, but I think this will do as a quick fix for this issue. We can always discuss a larger overhaul later.

(Added the 2nd opinion tag, just to make sure we're on the same page for this fix)

Last edited 7 years ago by Cheffheid (previous) (diff)

@Cheffheid
7 years ago

Appends role="menuitem" to toolbar menu item parent ARIA attributes

#9 @Cheffheid
7 years ago

  • Keywords has-patch 2nd-opinion needs-testing added

#10 @afercia
7 years ago

@Cheffheid yep using role="menu" (or role="menubar") and role="menuitem" would make screen readers let browsers know of the event and so it would work. However, role="menu" and role="menuitem" are suited for menus like the ones of an application, not for navigation menus 😞

The menu role is appropriate when a list of menu items is presented in a manner similar to a menu on a desktop application.

https://www.w3.org/TR/wai-aria/roles#menu

#11 @Cheffheid
7 years ago

I have legitimately no idea what that actually means. Menu on a desktop application? Doesn't that refer to menus like the image below? In which case, since the keyword in that statement is "presented", wouldn't that mean it's appropriate? Or am I missing some other kind of menu in a desktop application that it's actually referring to?

(Sorry, lots of questions :)).

https://cdn.arstechnica.net/wp-content/uploads/2014/02/unity-integrated-menus.png

#12 @afercia
7 years ago

There's an example of "application" menu on the WAI tutorials, it's basically a menu with items that are "actions" or commands rather then navigation. Note: the example is sligthly broken :) So yes, like in the image you posted.
https://www.w3.org/WAI/tutorials/menus/examples/appmenu/

#13 @afercia
7 years ago

Seems there's some ambiguity: the new ARIA 1.1 spec (still a Candidate Recommendation at the time of writing) still refers to menus similar to the ones on desktop application but then in the authoring practices demos (still not official) the role=menu is used for site navigation:
http://w3c.github.io/aria-practices/examples/menubar/menubar-1/menubar-1.html

If this kind of usage is legit, that's exactly what would fix the toolbar (plus some aria-expanded magic).

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

#15 @afercia
7 years ago

  • Milestone changed from 4.8 to 4.8.1

We're running out of time for the 4.8 release, so: punting.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

#17 @afercia
7 years ago

  • Milestone changed from 4.8.1 to 4.9
  • Version 4.3.1 deleted

Agreed during today's bug scrub to wait a bit to see how ARIA 1.1 evolves. Moving to 4.9.

#18 @Cheffheid
7 years ago

Recommendation at the time of writing) still refers to menus similar to the ones on desktop application but then in the authoring practices demos (still not official) the role=menu is used for site navigation:

Honestly, this is what makes sense to me. Simply because of the use of the word "presented", meaning that it'd apply when it looks like it not just acts like it. But I'm also aware that my interpretation is different from what seems to be the consensus on it. :)

Guess we'll need to wait and see what happens when 1.1's status finally changes, unless someone has a brilliant idea to fix this. :)

Last edited 7 years ago by Cheffheid (previous) (diff)

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

#20 @afercia
7 years ago

  • Milestone changed from 4.9 to Future Release

As per today's bug scrub: moving to future release, waiting for ARIA 1.1.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

#22 @sabernhardt
3 years ago

This still is a problem.

If all dropdown interactions are changed in #48894, though, then the sub-menus probably should expand properly with the Enter key or spacebar as well.

Version 0, edited 3 years ago by sabernhardt (next)

This ticket was mentioned in Slack in #accessibility by johnbillion. View the logs.


3 years ago

#24 @sparklingrobots
3 years ago

Hi, all! Hope this is useful information:

There are forum reports that this is still impacting users in 5.8, as well: https://wordpress.org/support/topic/wp-multi-site-network-admin-page-not-accessible-to-screen-readers-from-wp-admin/

I took a look at #48894 -- it looks like it's been indefinitely delayed.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

#27 @sabernhardt
3 years ago

  • Keywords needs-refresh added; needs-testing removed
  • Milestone changed from Future Release to 5.9

Moving to 5.9 to investigate an option based on the menubar role.

Also, I opened ticket:53888 for an additional link to Network Admin somewhere outside the toolbar.

@sabernhardt
3 years ago

adding menuitem role only to top-level links and including aria-expanded attribute

#28 @sabernhardt
3 years ago

  • Keywords needs-testing added; needs-refresh removed
  • Owner set to sabernhardt
  • Status changed from new to accepted

Using the menu, menubar or menuitem role is not appropriate for a navigation menu. However, I made 34668.2.patch to find out if adding the menuitem role to the top-level parent links might help anyway.

With NVDA and Firefox, the patch toggles those links when pressing the Enter key on its own. If people want to follow the My Sites link instead of toggling, though, they can use either Shift + Enter or Ctrl + Enter to open it in a new tab or window. The mouse/pointer interaction would remain the same as it has been.

We may also want to consider opening submenus on focus like in the side admin menu (comment:8).

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

#30 @alexstine
2 years ago

@sabernhardt Here's a modification. I think your patch is going in the right direction. I tried to make it a little better. I know this should not be an ARIA menu, but I ended up making it one. The biggest changes are I removed the aria-expanded false from JavaScript and allowed PHP to handle that directly. The other change was I allowed all items to have menuitem. Finally, I made an adjustment so that when sub menus open, they have the menu role and will announce the title of the menu via aria-label.

This patch is far from perfect, but maybe if the code is half way decent, it can be a keeper.

Can you work on the JavaScript some more? For some reason, if you click on About WordPress, the sub menu will only open if the screen reader is in focus mode first. Can you bind to the link and throw event.preventDefault(); unless Ctrl+Enter or Shift+Enter keys are used?

Thanks!

@alexstine
2 years ago

#31 @sabernhardt
2 years ago

@alexstine Thanks for testing and for the patch!

The goal with using the menuitem role was to put screen readers in focus mode automatically for these links, so it would need to work consistently for all of them. The problem might relate to how "About WordPress" is the first link after clicking the "Skip to Toolbar" link. If I tab forward and back, and then press Enter, the menu expands.

Because this is only a partial fix, I tried to avoid changing the behavior when using a pointing device. But perhaps disabling the "About WordPress" top-level link is fair.

If we add ARIA labels for submenus, I think the user account actions menu could have something better than the "Howdy, [name]" text. (Editing the labels might be more of an enhancement.)

34668.3.patch also removes the special log-out link for keyboard users. The link is probably unnecessary with the rest of these changes, though eliminating it would require people to search for the one within the submenu.

And if we remove the negative tabindex from the spare profile link, I would rather consider combining links so the user's display name and "Edit Profile" text are together (ticket:43633).

#32 @alexstine
2 years ago

@sabernhardt Agree with those observations completely.

Honestly, if this was to be refactored correctly, I would not allow top level submenus to have links. They would be rendered as buttons with the top level link being the first item in the menu. That way there is no problem with having to handle keyboard events differently with screen readers. I think this will greatly improve the UX, your latest idea, but this whole class needs to be refactored in the future. I'd like to do it now but I just don't think there's enough support in current class implementation to do what I want it to do. I could re-code the core links, but it's the custom admin menus that wouldn't have proper support. The only true way to achieve this would be to replicate the functionality in the class. That way, you would get this.

About WordPress button
Begin submenu:
About WordPress link
Other links...

Hope this makes sense.

#33 @sabernhardt
2 years ago

OK, disabling the click event on the "About WordPress" top-level link would not work. It may be fine with a mouse, but apparently that would make the entire About menu unusable with a touchscreen.

Maybe there is an event that can fire immediately after skipping to the toolbar.

#34 @alexstine
2 years ago

@sabernhardt Would it be possible to open the submenu on focus of the top level link? This way the submenu will open and the top level link will still be clickable. The submenu should still close on Escape press or once the container has focus loss. Will be harder to implement, that's why I held out on this suggestion.

Thanks.

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


2 years ago

#36 @sabernhardt
2 years ago

  • Milestone changed from 5.9 to Future Release

As @alexstine said on Slack:

More time might be good to re-consider options.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


11 months ago

#39 @joedolson
4 months ago

  • Milestone changed from Future Release to 6.5

I'd like to add this to the 6.5 release cycle. This is a fairly significant accessibility issue, and needs to get solved.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


4 months ago

#41 @alexstine
4 months ago

  • Owner changed from sabernhardt to alexstine

Adding to my queue to work on again.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


4 months ago

#43 @joedolson
3 months ago

  • Severity changed from normal to major
  • Summary changed from Network admin can't be accessed via keyboard to Admin submenus can't be accessed via keyboard using screen readers

Updating title and severity. The title of this issue has masked the fact that the issue impacts *all* adminbar submenus.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


2 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


4 weeks ago

@joedolson
2 weeks ago

Refresh patch & collapse profile links

#46 @joedolson
2 weeks ago

I've refreshed the patch and collapsed the duplicate profile links into a single item.

While using the menu role isn't ideal, I think that we should set aside semantic purity for the sake of having the adminbar submenus functional, which this patch does accomplish.

The behavior on the About WordPress menu item is kind of weird - it does work, but not on arrival from the skip link, and the reason isn't clear to me.

But I think that this is something we should get in place sooner rather than later, regardless. While this fix isn't perfect, it is *much* better than what's in place right now - which is completely broken.

#47 @afercia
2 weeks ago

I'd agree the most pragmatic solution is to use a role=menu and I wouldn't mind too much whether the 'pureness' of the technical approach. However, I'd think that at that point all the menu items should have a role="menuitem"?

The behavior on the About WordPress menu item is kind of weird - it does work, but not on arrival from the skip link, and the reason isn't clear to me.

Testing on Windows with NVDA? Maybe in that scenario the screen reader doesn't automatically switch to 'focus mode'? I think it can be forced via NVDA key + Space bar.

#48 @joedolson
2 weeks ago

However, I'd think that at that point all the menu items should have a role="menuitem"?

I'm not seeing the case in this patch where menu items don't get the menuitem role, and all items I can see in the adminbar have the role. Can you clarify? I could be missing something, for sure.

Maybe in that scenario the screen reader doesn't automatically switch to 'focus mode'?

Yes, the NVDA key + Space bar allows it to work; I'm just not sure why this is happening. It may be because focus is being placed on the menu rather than on the menuitem, so it doesn't trigger. But I'm reluctant to change the skip link focus target.

I don't think that this is a blocking issue, since it's pretty trivial to get around - and it's significantly less of a problem than the current state of things.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 days ago

#50 @alexstine
7 hours ago

Let's commit this for now as it is an improvement. Just noting, a complete solution should be found in the future.

Thanks.

#51 @joedolson
7 hours ago

  • Keywords commit added; 2nd-opinion needs-testing removed

Testing:

Before patch:

  • With screen reader: Tab through admin bar items with submenus. Submenus do not expand and cannot be reached. Enter takes user to target link instead.
  • Without screen reader: Tab through admin bar items with submenus. Submenus expand on enter and user can access submenu items.

After patch:

  • With screen reader: Tab through admin bar items with submenus. Submenus expand on enter user can access submenu items. Screen reader user can access top level link using shift+enter or ctrl+enter.
  • Without screen reader: Tab through admin bar items with submenus. Submenus expand on enter and user can access submenu items.

Verified in NVDA/Firefox, NVDA/Chrome, NVDA/Edge. Jaws/Firefox, Jaws/Chrome, Jaws/Edge.

We acknowledge that this is not an ideal fix, but it does change the problem from what it is now: a significant accessibility barrier to any submenu item in the navigation, to a functional though less than perfect interface.

#52 @joedolson
7 hours ago

Other included changes: remove screen-reader-only log out, as it is now possible for screen readers to reach the standard log out; collapse profile links in submenu into one link to remove duplicate.

#54 @joedolson
109 minutes ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 57708:

Toolbar: Accessibility: Keyboard navigation for screen readers.

Change the admin toolbar to have role="menu" and support opening for screen readers. Remove screen reader only log out link and collapse duplicate profile links into one link. This is an imperfect solution to a complex problem in the adminbar, but the lack of screen reader access to submenus is a major accessibility problem, and this fix provides access, even if the mechanism is imperfect.

Screen reader log out added in [21452].

Props abletec, Cheffheid, sabernhardt, alexstine, joedolson, afercia, sparklingrobots, danieltj, swissspidy, netweb, dionysous.
Fixes #34668, #43633.

#56 @joedolson
101 minutes ago

In 57709:

Code Standards: Fix alignment in tests.

Fix variable alignment in changed tests. Follow-up to [r57708]. Because I edited the wrong local copy when I fixed that.

Props joedolson.
See #34668.

Note: See TracTickets for help on using tickets.