[Popover] role="presentation" causing accessibility issue when Popover is open

[emilyuhde]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When the Popover component is opened, the role is set to "presentation" which triggers a warning by HTML CodeSniffer that there is semantic meaning to elements under the element with role="presentation" so it's an inappropriate role to use.

"This element's role is "presentation" but contains child elements with semantic meaning."

Expected Behavior 🤔

The role used should not cause an accessibility error.

Steps to Reproduce 🕹

Steps:

1. Open up the Popover demo page on the latest MUI site https://material-ui.com/components/popover/
2. Click the "SHOW POPOVER" button to show the Popover in the first example, Simple Popover
3. With the Popover open, run the HTML CodeSniffer tool https://squizlabs.github.io/HTML_CodeSniffer/

Context 🔦

When doing accessibility testing on my component that's using Popover as its base, this error was being reported by HTML CodeSniffer. I tracked it down to it being an issue with the underlying MUI component as well.

Your Environment 🌎

Tech	Version
Material-UI	v4.3.1 in my project
React	15.8.6
Browser	Firefox, Chrome (should be all though)
TypeScript	N/A

[oliviertassinari]

@emilyuhde I think that we should ignore what CodeSniffer thinks is right or wrong (tools can be wrong). Do you have reference materials we can look at to dive into the issue? Thanks

[eps1lon]

Thank you for opening this issue and filling out the template.

"This element's role is "presentation" but contains child elements with semantic meaning."

I was always suspicious of this usage since it's largely motivated by a lint rule that forbids event handlers on presentational generic elements. The rule itself is problematic since it assumes simple event handlers that don't have conditional logic. I'm not entirely sure how to construct an accessible backdrop that doesn't cause any false alarms.

What's important here is to find out if a presentational parent with non-presentational children is actually problematic for assistive technology. While these tools are helpful (like lint rules) while authoring content they can be deceiving when consuming libraries that validate particular usage.

[oliviertassinari]

In the unknown, I would tend to have more trust in a tool that has a look at the overall DOM output (CodeSniffer) rather than for a tool that performs static analysis (eslint). But, I think that it would be great to explore the problem deeper.

[eps1lon]

I did some research and it seems like that the role is intended to remove implicit semantics. Considering the eslint rule itself recommends disabling the rule instead of using presentation I'd say we follow that recommendation.

I'll do some testing with NVDA and Firefox if this introduces some false "clickable" announcement (something I really need to get clarification from NVDA folks).

[TrevorBurnham]

Per squizlabs/HTML_CodeSniffer#274, HTML Codesniffer has been updated so that it no longer complains about role="presentation" being set on an element like this.

[eps1lon]

Per squizlabs/HTML_CodeSniffer#274, HTML Codesniffer has been updated so that it no longer complains about role="presentation" being set on an element like this.

Thanks for the update.

I'm going to close this then. @emilyuhde Thank you for reporting this even though it turned out to be a false positive. Still valuable information to know which tools our users use and how they use them.

[danielweil]

This is still an issue. I can't find elements with React Testing Library on screen when Backdrop from multiple Select is opened.