Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard Shortcuts modal: some shortcuts are announced incorrectly by screen readers #15655

Open
afercia opened this issue May 15, 2019 · 4 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented May 15, 2019

Noticed while testing #15631

In the Keyboard Shortcuts modal, some shortcuts have an aria-label attribute:

<kbd ... aria-label="Shift Command Comma">
	...

(Note: this is what is being used for macOS)

Other shortcuts don't have an aria-label. These aria-label attributes were originally introduced because (depending also on user settings) most screen readers don't announce some punctuation. If a keyboard shortcut uses a comma, or a backtick, etc., the shortcuts text won't be announced correctly by screen readers, defeating the purpose of providing users with a list of shortcuts.

For example, testing with Firefox and NVDA:

Screenshot (229)

The shortcuts highlighted in the screenshot above are announced, respectively, as:

  • Control plus Shift plus
  • Control plus
  • Control plus Shift plus

Possible options:

  • Never use punctuation for shortcuts: worth also reminding not all keyboard layouts have keys for the backtick. See also Allow users to customize keyboard shortcuts #3218 and several other issues and PRs where shortcuts were discussed.
  • Always provide a full "human friendly" aria-label attribute for all the shortcuts by expanding the keys names in plain text e.g. Control plus shift plus comma.
@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels May 15, 2019
@afercia
Copy link
Contributor Author

afercia commented May 15, 2019

Also on macOS, some shortcuts are announced incorrectly. For example, testing with Safari + VoiceOver, this:

Screenshot 2019-05-15 at 14 58 02

is announced as "caret option H" because, actually, ^ is the "caret" character.

@talldan
Copy link
Contributor

talldan commented May 30, 2019

Thanks for spotting this 😄

It seems like it should be possible to extend the work done on #9582 to cover these options.

I was a little surprised it doesn't already work, so it could also be a regression. I'll try to take a look at it when I have a spare moment. E2E tests might also be good if it turns out it is a regression.

@afercia
Copy link
Contributor Author

afercia commented Aug 26, 2020

Update: the "caret" character issue has been solved in #24451 / #24452.

Still to fix: the aria-labels for shortcuts that use punctuation keys (e.g.: comma, backtick).

@afercia
Copy link
Contributor Author

afercia commented Aug 26, 2020

Update 2: I just tested again the aria-labels part and they do use correct "human friendly" strings to expand the punctuation characters. See for example the screenshot below:

Screenshot 2020-08-26 at 12 53 20

However, while VoiceOver with Safari announces the aria-labels, both NVDA and JAWS on Windows don't. Instead, they read out the text content so the punctuation characters are ignored.

I'd tend to think the JAWS and NVDA behavior is the correct one. These aria-labels are used on <kbd> elements which are non-interactive elements. As also noted in the W3C "Using ARIA" guide:

https://www.w3.org/TR/using-aria/#label-support

2.10 Practical Support: aria-label, aria-labelledby and aria-describedby

Don't use aria-label or aria-labelledby on any other non-interactive content such as p, legend, li, or ul, because it is ignored.

Don't use aria-label or aria-labelledby on a span or div unless its given a role. When aria-label or aria-labelledby are on interactive roles (such as a link or button) or an img role, they override the contents of the div or span. Other roles besides Landmarks (discussed above) are ignored.

That is: aria-label is not supposed to work on non-interactive elements that don't use some ARIA role.

It's a bit unfortunate screen readers apparently don't get the nested <kbd> elements natively, as they're specifically meant to indicate keys to press.

To fix this, instead of messing with ARIA roles (I can't think of a suitable role for this case), I'd suggest to remove the aria-labels and use visually hidden text instead. The VisuallyHidden component may come in handy. Basically try this:

  • remove the aria-label attribute from the wrapping <kbd> element
  • add the shortcut strings in a visually hidden text within the wrapping <kbd> element
  • add aria-hidden="true" to the nested <kbd> element

This should be tested with Windows screen readers. Suggested combos:

  • Firefox and NVDA
  • Chrome and JAWS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants