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

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

Closed
1 task done
magentaRE opened this issue Oct 6, 2023 · 13 comments
Closed
1 task done
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) area: cdk/a11y area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@magentaRE
Copy link

Is this a regression?

  • 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 magentaRE added the needs triage This issue needs to be triaged by the team label Oct 6, 2023
@zarend
Copy link
Contributor

zarend commented Oct 6, 2023

Hello,

Do you have a WCAG criteria to support this?

-Zach

@magentaRE
Copy link
Author

@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.

@zarend
Copy link
Contributor

zarend commented Oct 6, 2023

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

@magentaRE
Copy link
Author

@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.

@zarend zarend added area: material/sidenav and removed needs triage This issue needs to be triaged by the team labels Oct 9, 2023
@zarend
Copy link
Contributor

zarend commented Oct 9, 2023

Hello,

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

-Zach

@zarend zarend changed the title bug(sidenav/drawer/focustrap): tabindex creates invisible elements bug(sidenav/drawer/focustrap): NVDA announces "blank" when interacting with focus trap Oct 9, 2023
@zarend
Copy link
Contributor

zarend commented Oct 9, 2023

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 zarend added Accessibility This issue is related to accessibility (a11y) area: cdk/a11y P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Oct 9, 2023
@magentaRE
Copy link
Author

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

@zarend
Copy link
Contributor

zarend commented Dec 6, 2023

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

@magentaRE
Copy link
Author

magentaRE commented Dec 7, 2023

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.

@zarend
Copy link
Contributor

zarend commented Dec 7, 2023

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

@josthun
Copy link

josthun commented Apr 1, 2024

Hello,

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

@crisbeto
Copy link
Member

crisbeto commented Aug 8, 2024

Closing as a duplicate of #29545 which was recently fixed.

@crisbeto crisbeto closed this as completed Aug 8, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: cdk/a11y area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

4 participants