axe-core icon indicating copy to clipboard operation
axe-core copied to clipboard

Modal Dialog check thinks focus leaves dialog when it doesn't

Open TastyPi opened this issue 2 years ago • 6 comments

Product: axe Extension

Expectation:

Using focus-trap to implement a dialog should return no issues with focus leaving the dialog.

Actual:

The Modal Dialog check is returning the issue "Keyboard focus is not maintained within the modal. It is possible to tab out of the modal". It does not give any insight into which element outside the dialog is able to be tabbed to. Considering I don't believe there are any elements outside the dialog that can be tabbed to (thanks to focus-trap), and I didn't see it tab to anything outside the dialog, this is either a faulty detection or it should provide more information for what element outside the dialog can be tabbed to.

Motivation:

I have no clue what element can be tabbed to outside the dialog, but presumably axe does know since it found one. It should tell me which element can be tabbed to outside the dialog.


axe-core version: 4.3.5
extension version: 4.22.1

Browser and Assistive Technology versions
Chromium: 98.0.4758.80

For Tooling issues:
- Platform:  Linux

TastyPi avatar Feb 04 '22 16:02 TastyPi

Can you provide an example of this? We won't be able to debug issues without it.

WilcoFiers avatar Feb 09 '22 16:02 WilcoFiers

My original dialog is embedded in our private repo so was awkward to share, but I've built a small example that reproduces the issue. If you go through the modal dialog test it complains that focus can leave the modal, but I couldn't reproduce that.

https://gist.github.com/TastyPi/9008631fa8fb3fa04f2593fcd24b0b73

TastyPi avatar Feb 10 '22 13:02 TastyPi

Hey @TastyPi, we've just recently worked on a fix related to this and a fix should be available in the next update of the axe DevTools extension sometime next week.

scurker avatar Mar 22 '22 16:03 scurker

@TastyPi can we close this issue?

dylanb avatar Jun 10 '22 17:06 dylanb

It seems the example in my gist works, but it still fails for my actual dialog. I'm trying to figure out what the difference between is that makes it fail, once I figure that out I'll provide another gist (or close the issue because there was an actual issue with my dialog). The extension is still not telling me which element it thinks is visible outside the dialog, if it could be updated to do that this would be a lot easier.

Edit: Actually it's a different issue to the original, it's now complaining "Screen readers can read content outside the modal dialog." rather than something is focusable. I still don't know which element it is since I add aria-hidden=true to all the other root elements like in the gist, and axe isn't telling me.

TastyPi avatar Jun 13 '22 08:06 TastyPi

OK I figured out the difference. When the dialog is open, the DOM in the gist looks like

<body>
  <main aria-hidden="true">
    ...
  </main>
  <div>
    dialog
  </div>
</body>

But my actual one looks like

<body>
  <main>
    <div aria-hidden="true">...</div>
  </main>
  <div>
    dialog
  </div>
</body>

So instead of the <main> element being hidden its only child was. I'm not sure if this should technically be accepted since there's nothing in the <main> element for the screen reader to read, but this is an easy fix for me and I think hiding the <main> element is the more correct solution.

However, axe did not make it obvious which element was "readable" outside the dialog. The issue page appears to be pointing at the button that opened the dialog, not the <main> element. This gist can be used to replicate that.

So basically the original issue is fixed, but there is another minor issue with reporting which element a screen reader can read when the dialog is open.

TastyPi avatar Jun 13 '22 08:06 TastyPi

Should axe only be failing this test if there is actual content that isn't hidden? Do wrapper elements which are either empty or contain no content actually impact screen readers?

angusd3v avatar Oct 05 '22 01:10 angusd3v

Screen reader usage will vary based on the types of elements that are outside of an active modal dialog and generally you do want aria-hidden="true" on every element that does not contain the modal because it’s not just the accessible name or text content that screen readers utilize. This could include things like landmarks, tab index, interactive elements and many other things.

scurker avatar Oct 05 '22 15:10 scurker