components icon indicating copy to clipboard operation
components copied to clipboard

bug(sidenav/drawer/focustrap): NVDA announces "blank" when interacting with focus trap

Open magentaRE opened this issue 8 months ago • 10 comments

Is this a regression?

  • [X] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

legacy non MDC-based components

Description

mat-sidenav generates two <div> elements that have tabindex set to 0, even if the sidenav is closed. This happens in mode="over" of sidenav:

<div class="cdk-visually-hidden cdk-focus-trap-anchor" aria-hidden="true" tabindex="0"></div>

Although they have aria-hidden="true", they can be tabbed into which creates major accessibility issue for keyboard users. In previous versions of sidenav, the tabindex="0" was only added after the sidenav was opened and it was removed when user closed the sidenav. User can tab onto those 2 divs (with the sidenav closed). That creates 2 invisible elements which can confuse user while navigating through the website.

This issue is basically same as #27623, but in that issue, the concern was that Axe core found 2 issues, which is different to this issue. This issue is concerned about tabbing onto "invisible" elements. IMO, this is a regression bug and adding tabindex="0" should be done only when the sidenav is opened. Also, will #27629 fix this issue as well?

Reproduction

StackBlitz link: https://stackblitz.com/edit/rwjkdz-juypse?file=src%2Fexample%2Fsidenav-autosize-example.html Steps to reproduce:

  1. Open any mat-sidenav example with the mode="over" in the new MDC components.
  2. Use keyboard to navigate through the page.
  3. User tabs onto 2 invisible elements created by the sidenav (with sidenav being closed)

Expected Behavior

There are no elements that can be tabbed onto created by sidenav when the sidenav is closed.

Actual Behavior

There are 2 <div> elements (used for creating focus trap) encapsulating sidenav that can be tabbed onto created by sidenav (with the sidenav closed).

Environment

  • Angular: 16.2.7
  • CDK/Material: 16.2.7
  • Browser(s): any
  • Operating System (e.g. Windows, macOS, Ubuntu): any

magentaRE avatar Oct 06 '23 10:10 magentaRE

Hello,

Do you have a WCAG criteria to support this?

-Zach

zarend avatar Oct 06 '23 16:10 zarend

@zarend I found this:

https://dequeuniversity.com/rules/axe/html/4.4/aria-hidden-focus

Focusable content through tabindex.

<p tabindex="0" aria-hidden="true">Some text</p>

That "Why it Matters" sections really explains the issue about putting tabindex to already aria-hidden element.

magentaRE avatar Oct 06 '23 22:10 magentaRE

Hi @magentaRE ,

Thanks for bringing up this concern.

AXE can be a helpful tool for quickly identifying things that can be a problem for accessibility.

In many conditions, I would consider it a mistake to put aria-hidden="true" on an interactive element. That's because I would expect the intent of interactive elements is for users to be able to focus them.

We test our components with supported screen readers. We've had issues in the past with cases where focus trapping could be escaped.

The focus trap anchor is a bit of an exception. the user is never intended to actually focus and interact with the anchor. We can think of it as a bumper to prevent users from escaping the focus trap. ConfigureableFocusTrap was deliberately made this way to improve focus trapping.

<div class="cdk-visually-hidden cdk-focus-trap-anchor" aria-hidden="true" tabindex="0"></div>

In general, our policy on AXE violations is that we strive to understand why AXE is creating a violation, but it doesn't necessary mean there is a bug. AXE produces false-positives from time to time, and we do not treat the results of AXE as a specification.

It's safe to ignore this violation in your application if it's on the .cdk-focus-trap-anchor element.

Again, we test our components on supported screen readers. We don't have any information here to believe that the focus trapping is not working or has a real accessibility problem here.

Is this resolution acceptable?

Best regards,

Zach

zarend avatar Oct 06 '23 22:10 zarend

@zarend Thanks for the quick reply. The real problem is, that even if the sidenav is closed, you still bump into those 2 focus trap anchors.

NVDA screen reader reads them as "blank". Previous implementation of sidenav correctly behaved - when it was closed, no tabindex vs. when it was opened, tabindex="0". Why was that behavior of the sidenav removed?

In my app, you would bump into those 2 focus trap anchors and only then you would jump to the main content of the application. Previously, you jumped from sidenav directly onto the content when operating the website via a keyboard. It's really confusing for visually impaired users - they get "blank" announced twice, which doesn't make a whole lot of sense.

magentaRE avatar Oct 07 '23 16:10 magentaRE

Hello,

That you for reporting this. NVDA announcing the cdk-focus-trap-anchor as "blank" looks like suspicious behavior to me.

-Zach

zarend avatar Oct 09 '23 17:10 zarend

I changed the title to mention that NVDA is announcing "blank" so that this issue can get more attention. I have not tried to reproduce myself, and I am working if this will work better if MatDrawer switches from FocusTrap to ConfigurableFocusTrap (#27629).

-Zach

zarend avatar Oct 09 '23 17:10 zarend

Any updates on this? This is still an issue in 17.0.1.

magentaRE avatar Nov 23 '23 08:11 magentaRE

Hello,

I believe this is how ConfigurableFocusTrap was designed. It's designed to have anchors on each end of the "trapped" DOM, which prevent focus from moving outside of the anchors.

This issue is available to work on if anyone is interested in researching the focus trapping to understand what can be done to address this.

Best Regards,

Zach

zarend avatar Dec 06 '23 23:12 zarend

Hi Zach, I know that sidenav was designed to have those 2 anchors and I have nothing against that. I just thinks that those anchors are not behaving correctly.

In my app, I fixed it as it was in older Angular Material's versions - When the sidenav is closed, those 2 anchors have the tabindex attribute removed. When it's opened, tabindex is set back to 0. This fix makes sure the sidenav behaves correctly in its opened state and also makes sure that NVDA doesn't read those "blank" elements in its closed state.

magentaRE avatar Dec 07 '23 09:12 magentaRE

Hello,

Thanks for investigating this. Feel free to send us a PR if the fix made in your app can be applied to this library. That way other developers would not need to implement work-around.

-Zach

zarend avatar Dec 07 '23 17:12 zarend

Hello,

I have run into the same issue on version 17.2.1. Is there any update on when/if this may be fixed?

josthun avatar Apr 01 '24 14:04 josthun