Accessibility-insights-web: False positive report on aria-required-parent with closed Shadow DOM

Created on 18 Dec 2020  路  19Comments  路  Source: microsoft/accessibility-insights-web

Describe the bug

When an element's parent is contained in a closed Shadow DOM, the aria-required-parent rule will report a failure regardless of the parent element's role.

To Reproduce

  1. Go to the CodePen repro example or this page
  2. Run Automated Checks
  3. Observe the aria-required-parent failures

CodePen repro example

https://codepen.io/joalkire/full/xxErYqx

Expected behavior

No aria-required-parent failures should be reported

Context

  • OS Name & Version: Windows 10 20H2
  • AI-Web Version & Environment: Version 2.22.0 prod
  • Browser Version: Microsoft Edge 87.0.664.66 (Official build) (64-bit)

Additional context

According to axe-core documentation, they "do not and cannot support closed Shadow DOM." There exists an axe-core issue requesting that elements using a closed shadow root be marked as Needs Review.

bug resolved

Most helpful comment

I'd expect that the majority case would be for someone in an impacted case to not be able to tell whether they are experiencing incorrect results or not (they're more likely to see false negatives than false positives). Based on that, I'd probably drop the conditional, and maybe even make the rest of the answer a little more firm. Maybe something like:

Are automated checks compatible with shadow DOMs?

Automated checks are compatible with open shadow DOMs but not with closed shadow DOMs.

Accessibility Insights will not be able to detect most types of issue for components within a closed shadow DOM. In some cases, it can also cause false issue reports; for example, if an interactive component outside a shadow DOM is labelled by an element inside a closed shadow DOM.

We recommend using open mode only.

All 19 comments

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

per conversation with Peter, the ideal approach would be that axe-core marks this as needs review. In the meantime, marking this issue as blocked and recommending we document in our docs website that closed Shadow DOM is not supported (just as axe-core doesn't support it)

@LiLoDavis could you please take a look at the documented limitations of axe-core and drafting a short FAQ to link to it?

@LiLoDavis could you please take a look at the documented limitations of axe-core and drafting a short FAQ to link to it?

@ferBonnin yes, will do.

@ferBonnin and @peterdur - How does this sound? The last sentence might be unnecessary.

Do automated checks work on Shadow DOM?

Yes. Automated checks are based on axe, which supports Shadow DOM in _open_ mode.

Because Shadow DOM in _closed_ mode prevents JavaScript from accessing an element's internal DOM, automated checks might return incorrect results.

Closed mode is sometimes implemented as a security measure, but it's easy to bypass. We recommend using open Shadow DOM.

@LiLoDavis this looks good to me, I will defer to @peterdur for the last sentence.

Apart from Shadow Dom, is there anything else that would be worth noting in our documentation?

Apart from Shadow Dom, is there anything else that would be worth noting in our documentation?

I don't have anything at this time.

@LiLoDavis @ferBonnin On the last sentence, I'm a little nervous about making security recommendations for other people's code.

+1 to Peter's comment. I think the sentiment is still a good idea though; maybe replace the phrase "a security measure" with "an encapsulation measure"?

Minor nitpick: I'd recommend using "shadow DOM" (lowercase shadow) instead of "Shadow DOM"; that seems to be more consistent with the terminology in the DOM spec and MDN (MDN is internally inconsistent, but primarily uses lowercase).

Per conversation with the team, would be good to timebox an investigation on detecting _closed shadow DOM_ to alert users that this is not supported and results might be unreliable. We will also move forward with updating the documentation.

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

Fluent UI temporarily switched fluent web components to use closed shadow DOM in October 2020, which may have led to the false positive report we received
https://github.com/microsoft/fluentui/issues/15346

+1 to Peter's comment. I think the sentiment is still a good idea though; maybe replace the phrase "a security measure" with "an encapsulation measure"?

@peterdur what do you think of this suggestion?

Minor nitpick: I'd recommend using "shadow DOM" (lowercase shadow) instead of "Shadow DOM"; that seems to be more consistent with the terminology in the DOM spec and MDN (MDN is internally inconsistent, but primarily uses lowercase).

@dbjorge I'm ok with lower case.

How about something like this for the third sentence:
Closed mode is sometimes implemented as an encapsulation measure, but if you are experiencing incorrect automated checks results, you may want to consider other approaches.

How about something like this for the third sentence:
Closed mode is sometimes implemented as an encapsulation measure, but if you are experiencing incorrect automated checks results, you may want to consider other approaches.

@peterdur this sounds good to me. @dbjorge and @ferBonnin do you agree?

I'd expect that the majority case would be for someone in an impacted case to not be able to tell whether they are experiencing incorrect results or not (they're more likely to see false negatives than false positives). Based on that, I'd probably drop the conditional, and maybe even make the rest of the answer a little more firm. Maybe something like:

Are automated checks compatible with shadow DOMs?

Automated checks are compatible with open shadow DOMs but not with closed shadow DOMs.

Accessibility Insights will not be able to detect most types of issue for components within a closed shadow DOM. In some cases, it can also cause false issue reports; for example, if an interactive component outside a shadow DOM is labelled by an element inside a closed shadow DOM.

We recommend using open mode only.

@ferBonnin and @peterdur what do you think of @dbjorge's proposed text?

I don't think we can make a strong recommendation on using open or close mode, but I agree with the more definitive messaging. How about this:

Automated checks are based on axe-core, which supports shadow DOM in _open_ mode but is not compatible with _closed_ shadow DOM.

Shadow DOM in _closed_ mode is sometimes implemented as a encapsulation measure. Because closed mode prevents JavaScript from accessing an element's internal DOM, Accessibility Insights will not be able to detect most types of issue for components within a closed shadow DOM. In some cases, it can also cause false issue reports; for example, if an interactive component outside a shadow DOM is labelled by an element inside a closed shadow DOM.

If you experience this problem, you might consider another approach to encapsulation.

Since this has been documented, marking as resolved and closing.

Was this page helpful?
0 / 5 - 0 ratings