feat(new-rule): aria-braille-equivalent finds incorrect uses of aria-braille attributes #4107
Conversation
…braille attributes
| virtualNode | ||
| ) { | ||
| const brailleLabel = virtualNode.attr('aria-braillelabel') ?? ''; | ||
| if (!brailleLabel.trim()) { |
There was a problem hiding this comment.
Should we use sanitize from our text functions here instead (similar to aria-label and aria-labelledby checks)?
There was a problem hiding this comment.
We probably shouldn't use it for aria-labelledby either. What sanitize does that trim doesn't do is replace duplicate whitespace characters in between words with single space characters. We don't need that here.
Suppose it doesn't matter much though. And if we do ever need to do this in a way that's different from trim it helps to have them in place.
| "metadata": { | ||
| "impact": "serious", | ||
| "messages": { | ||
| "pass": "aria-braillelabel is not used on an element with no accessible text", |
There was a problem hiding this comment.
Just simplifying the double negative.
| "pass": "aria-braillelabel is not used on an element with no accessible text", | |
| "pass": "aria-braillelabel is used on an element with accessible text", |
| virtualNode | ||
| ) { | ||
| const brailleRoleDesc = virtualNode.attr('aria-brailleroledescription') ?? ''; | ||
| if (!brailleRoleDesc.trim()) { |
| return false; | ||
| } | ||
|
|
||
| if (roleDesc.trim() === '') { |
There was a problem hiding this comment.
Should keep consistent use of if you're using !brailleRoleDesc.trim() or brailleRoleDesc.trim() === '')
| "metadata": { | ||
| "impact": "serious", | ||
| "messages": { | ||
| "pass": "aria-brailleroledescription is not used on an element with no accessible text", |
There was a problem hiding this comment.
| "pass": "aria-brailleroledescription is not used on an element with no accessible text", | |
| "pass": "aria-brailleroledescription is used on an element with aria-roledescription", |
| describe('when aria-braillelabel has text', () => { | ||
| it('returns false when the accessible name is empty', () => { | ||
| const params = checkSetup(` | ||
| <img id="target" alt="" aria-braillelabel="foo" /> |
There was a problem hiding this comment.
This brings up an interesting question. This element is essentially role=presentation and thus won't be seen by the accessibility tree and thus I assume braille machines. Do we want to fail for these cases or do something similar to button-name rule and add the presentational-role check?
There was a problem hiding this comment.
I think this should fail. There's no reason someone would put aria-braillelabel on an element like this. We might change how that works if this attribute ever goes on the prohibited attrs list, which is what I asked the ARIA WG about. I think that would be more appropriate. Until then I think this is reasonable.
| afterEach(() => { | ||
| axe.reset(); | ||
| }); |
There was a problem hiding this comment.
I believe this isn't necessary
| afterEach(() => { | |
| axe.reset(); | |
| }); |
There was a problem hiding this comment.
Configuration isn't reverted until axe.reset() is called. It's not in testutils. Where would you think this might happen?
There was a problem hiding this comment.
The audit is reverted back to the original one in the beforeEach. Don't remember why we didn't use axe.reset there, maybe we should (in a different pr)? Either way, it's not strictly something that needs to hold up this pr.
| assert.lengthOf(results.incomplete, 0); | ||
| }); | ||
|
|
||
| it('fails when roledescription is empty but brailleroledescription is not', () => { |
There was a problem hiding this comment.
I believe accessibleTextVirtual can throw in subtree text if the element doesn't have a child (See https://github.com/dequelabs/axe-core/blob/develop/test/integration/virtual-rules/button-name.js#L116-L125). If that's the case, we should add a test for incomplete.
This rule effectively checks two things:
Closes: #3955