Fix S6825: align with React implementation

[ilia-kebets-sonarsource]

• raised by SONARHTML-212 Create rule S6825: Focusable elements should not have "aria-hidden" attribute #285 (comment)
• SonarJS implementation

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[mshima]

Looks like we have a few more false positives not related to this one: jhipster/generator-jhipster#26106 (comment).

Is there a public repository where we can fill a bug report?

[ericmorand-sonarsource]

Hi @mshima, I've been looking at some of the lines that trigger the rule, like this one, and your comment on the mentioned jhipster issue:

Doesn't make sense to require keypress on aria-hidden elements that are not focusable.

I feel that the rule should not trigger because, as you say, the element is not focusable. However, I'm also not sure how the rendered HTML page would work with assistive technology: since the close action is not reachable using the keyboard (both because the element is not focusable and because of the aria-hidden attribute), how can assistive technology users close the modal?

I want to ensure that this FP doesn't fortuitously detect a pattern that may impact impaired users negatively.

Note that the mentioned line of code should trigger S6848 (Non-interactive DOM elements should not have an interactive handler).

[mshima]

@ericmorand-sonarsource thanks for the response.

I feel that the rule should not trigger because, as you say, the element is not focusable. However, I'm also not sure how the rendered HTML page would work with assistive technology: since the close action is not reachable using the keyboard (both because the element is not focusable and because of the aria-hidden attribute), how can assistive technology users close the modal?

There is a cancel button below, so this other close 'button' is visual only.

I want to ensure that this FP doesn't fortuitously detect a pattern that may impact impaired users negatively.

Agreed.

Note that the mentioned line of code should trigger S6848 (Non-interactive DOM elements should not have an interactive handler).

That's valid.

It's a legacy code, Web:MouseEventWithoutKeyboardEquivalentCheck is a false positive in this case and doesn't justifies adding a keypress event.
S6848 is indeed an issue, that we should decide if we remove the visual only button or ignore it.