ionic-framework icon indicating copy to clipboard operation
ionic-framework copied to clipboard

bug: adding aria labels to host and input element inside shadow root causes issues with nvda

Open mwcampbell opened this issue 3 years ago • 8 comments

Ionic version:

[ ] 4.x [x] 5.x

I'm submitting a ...

[x] bug report [ ] feature request

Current behavior: The ion-toggle component sets ARIA attributes, such as role and aria-checked, on both the checkbox input element and its container. This means there's more than one switch in the accessibility tree, and only the inner switch can actually receive focus or otherwise handle programmatic interaction from assistive technologies.

Expected behavior: Only the checkbox input element should have these ARIA attributes. The container should have no ARIA attributes, like most HTML container elements.

Steps to reproduce: This issue can be seen on the online documentation page for the ion-toggle component.

mwcampbell avatar Apr 20 '21 00:04 mwcampbell

Thanks for the issue. Can you please clarify what issue this is causing in your application?

Adding these attributes on the host is required for VoiceOver to work properly: https://github.com/ionic-team/ionic-framework/blob/master/.github/COMPONENT-GUIDE.md#example-components-2

Additionally, since ion-toggle is a shadow DOM component, the inner switch should be in its own isolated DOM tree and should not interfere with the host attributes.

liamdebeasi avatar Apr 20 '21 13:04 liamdebeasi

I haven't yet decided to use Ionic in my own application, because I was reviewing the code for possible accessibility issues first. (Flawless accessibility is a must for any third-party front-end components that I use.) But I verified that the accessibility tree for the ion-toggle documentation page, as rendered by Chromium-based Edge on Windows, includes nodes for both the host element and the child input element. Both of these accessibility nodes have the role for a toggle button, but only the child one is marked as focusable. Meanwhile, the NVDA screen reader only presents the parent (host) element in its browse mode. So there isn't a single, canonical accessible element. This ambiguity is bound to cause problems. For example, if I navigate to the "Blueberry" toggle with NVDA's find command, NVDA puts its cursor on the parent element, and the child doesn't get the keyboard focus. Then if I press Tab, the focus doesn't move to the next toggle as expected, but somewhere else entirely.

mwcampbell avatar Apr 20 '21 14:04 mwcampbell

Could you provide a code sample of the issue with steps to reproduce? Our testing with NVDA did not present these kinds of issues so I am interested in learning more about how we can improve ion-toggle.

liamdebeasi avatar Apr 20 '21 14:04 liamdebeasi

Rather than write my own code sample, I'll give steps for reproducing the problem using the ion-toggle documentation page.

  1. Start NVDA and a Chromium-based browser (tested with the latest stable versions of Edge and Chrome).
  2. Go to https://ionicframework.com/docs/api/toggle
  3. Press x to jump to the first checkbox (NVDA treats toggles as checkboxes)
  4. Assuming you're using the NVDA desktop keyboard layout, press Insert+NumPad 5 (with NumLock off) to report the current navigator object.
  5. Press Tab.

These steps expose two problems:

  1. The current navigator object should be the BLueberry toggle. Instead, it's the document, which is what has the keyboard focus.
  2. Tab should move to the next toggle. Instead, it jumps to the "Open" link after all of the toggles.

I've verified that neither of these problems happen with bare HTML checkboxes.

mwcampbell avatar Apr 20 '21 15:04 mwcampbell

Thanks for the clear instructions. I was able to reproduce the behavior. I definitely agree that this is undesirable behavior, though the process to fix this is not very clear cut.

What NVDA Does

When you press "X", NVDA grabs the first checkbox/toggle. In this case, it sees <ion-toggle role="switch"> and grabs that. When you press "Tab", Chrome moves focus to the first focusable element, which is the <input type="checkbox"> inside of your ion-toggle. To someone using NVDA, they hear "Blueberry" twice, implying that there are two toggles named "Blueberry".

If we remove aria-label and role from the host, this issue would go away. This leads to the next question: Why do we have it on both the host and the inner input element in the first place? The answer is due to how different browser engines handle focusing with the keyboard. In particular, there is a big difference between Chromium (Chrome, Edge, etc) and WebKit (Safari) that makes this necessary.

Chromium

Chromium considers checkboxes, radios, buttons, and text inputs all focusable via keyboard input. So when you press "Tab" on a page containing an ion-toggle, Chromium will focus the <input type="checkbox"> inside the shadow root of your ion-toggle. This will cause the screen reader to read the aria-label on this input.

WebKit

WebKit considers text inputs focusable, but does not consider checkboxes, radios, and buttons focusable via keyboard input. So when you press "Tab" on a page containing an ion-toggle, nothing will happen because the <input type="checkbox"> does not get focused automatically. When you use VoiceOver in Safari on iOS and you tap the ion-toggle, VoiceOver is going to read the aria-label on the host <ion-toggle> since that is what you just selected. Because the inner <input type="checkbox"> is never focused, the label on that element is never read.

Potential Solution

Unfortunately, we cannot just remove the host aria-label and role attributes altogether as that would break accessibility in Safari on macOS and iOS. One idea I had was to detect if an app is running in a WebKit environment and add these attributes to the host but refrain from adding them if an app is not running in a WebKit environment.

I need to test this idea out of course, but this could be a potential path forward. Let me know if you have any questions. Thanks!

liamdebeasi avatar Apr 21 '21 16:04 liamdebeasi

I have an alternative hypothesis about why the toggle isn't accessible with VoiceOver on iOS when you don't put the ARIA attributes on the host element. When you tap on the toggle component, VoiceOver does a hit test to determine which accessible object it should announce. If the host element is a div or span without any ARIA attributes, then that hit test probably lands on one of the child elements. And I wouldn't be surprised if the hit test ends up on the visible label rather than the hidden input element. So, what happens if you remove the ARIA attributes from the host, and add the aria-hidden flag to all children other than the input element? I don't have an iPhone handy, and I haven't yet set up an Ionic dev environment, so I can't easily test this myself.

Thanks a lot for taking the time to look at this. So many developers seem to just ignore accessibility problems. It's encouraging to know that the Ionic team is really trying to get accessibility right. (Yes, accessibility matters a lot to me, not only as a developer but as a blind person.)

mwcampbell avatar Apr 21 '21 17:04 mwcampbell

Ah that's an interesting idea. I am hoping to have some time in the coming days to look at this, so I will test that idea out. I also reached out to some contacts on the WebKit team at Apple to see if they could clarify what is going on under the hood.

Thanks for opening this issue. We really appreciate any community feedback on how we can improve the accessibility of our components!

liamdebeasi avatar Apr 22 '21 17:04 liamdebeasi

Hi there,

We are proposing some changes to form components that seek to resolve this issue. We would love your feedback on these proposed changes! You can read more about the changes and leave comments here: https://github.com/ionic-team/ionic-framework/discussions/25660

liamdebeasi avatar Aug 02 '22 21:08 liamdebeasi

Thanks for the report. In Ionic 7.0 we will be introducing a new ion-toggle syntax that resolves this issue. Details on the syntax change can be found here: https://github.com/ionic-team/ionic-framework/discussions/25660

The work for this was completed in https://github.com/ionic-team/ionic-framework/pull/26357, so I am going to close this issue. We will have a public beta period in the future for developers to test and provide feedback on Ionic 7. Please let me know if there are any questions.

liamdebeasi avatar Nov 29 '22 17:11 liamdebeasi

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

ionitron-bot[bot] avatar Dec 29 '22 18:12 ionitron-bot[bot]