Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(new-rule): aria-braille-equivalent finds incorrect uses of aria-braille attributes #4107

Merged
merged 5 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
| :------------------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------- | :---------------- | :--------------------------------------------------------------------------------------------------------------------------------- | :------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| [area-alt](https://dequeuniversity.com/rules/axe/4.7/area-alt?application=RuleDescription) | Ensures <area> elements of image maps have alternate text | Critical | cat.text-alternatives, wcag2a, wcag244, wcag412, section508, section508.22.a, TTv5, TT6.a, EN-301-549, EN-9.2.4.4, EN-9.4.1.2, ACT | failure, needs review | [c487ae](https://act-rules.github.io/rules/c487ae) |
| [aria-allowed-attr](https://dequeuniversity.com/rules/axe/4.7/aria-allowed-attr?application=RuleDescription) | Ensures an element's role supports its ARIA attributes | Serious, Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure, needs review | [5c01ea](https://act-rules.github.io/rules/5c01ea) |
| [aria-braille-equivalent](https://dequeuniversity.com/rules/axe/4.7/aria-braille-equivalent?application=RuleDescription) | Ensure aria-braillelabel and aria-brailleroledescription have a non-braille equivalent | Serious | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure, needs review | |
| [aria-command-name](https://dequeuniversity.com/rules/axe/4.7/aria-command-name?application=RuleDescription) | Ensures every ARIA button, link and menuitem has an accessible name | Serious | cat.aria, wcag2a, wcag412, TTv5, TT6.a, EN-301-549, EN-9.4.1.2, ACT | failure, needs review | [97a4e1](https://act-rules.github.io/rules/97a4e1) |
| [aria-deprecated-role](https://dequeuniversity.com/rules/axe/4.7/aria-deprecated-role?application=RuleDescription) | Ensures elements do not use deprecated roles | Minor | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure | [674b10](https://act-rules.github.io/rules/674b10) |
| [aria-hidden-body](https://dequeuniversity.com/rules/axe/4.7/aria-hidden-body?application=RuleDescription) | Ensures aria-hidden='true' is not present on the document body. | Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure | |
Expand Down
22 changes: 22 additions & 0 deletions lib/checks/aria/braille-label-equivalent-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { sanitize, accessibleTextVirtual } from '../../commons/text';

/**
* Check that if aria-braillelabel is not empty, the element has an accessible text
* @memberof checks
* @return {Boolean}
*/
export default function brailleLabelEquivalentEvaluate(
node,
options,
virtualNode
) {
const brailleLabel = virtualNode.attr('aria-braillelabel') ?? '';
if (!brailleLabel.trim()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use sanitize from our text functions here instead (similar to aria-label and aria-labelledby checks)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return true;
}
try {
return sanitize(accessibleTextVirtual(virtualNode)) !== '';
} catch {
return undefined;
}
}
12 changes: 12 additions & 0 deletions lib/checks/aria/braille-label-equivalent.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"id": "braille-label-equivalent",
"evaluate": "braille-label-equivalent-evaluate",
"metadata": {
"impact": "serious",
"messages": {
"pass": "aria-braillelabel is used on an element with accessible text",
"fail": "aria-braillelabel is used on an element with no accessible text",
"incomplete": "Unable to compute accessible text"
}
}
}
29 changes: 29 additions & 0 deletions lib/checks/aria/braille-roledescription-equivalent-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { sanitize } from '../../commons/text';

/**
* Check that if aria-brailleroledescription is not empty,
* the element has a non-empty aria-roledescription
* @memberof checks
* @return {Boolean}
*/
export default function brailleRoleDescriptionEquivalentEvaluate(
node,
options,
virtualNode
) {
const brailleRoleDesc = virtualNode.attr('aria-brailleroledescription') ?? '';
if (sanitize(brailleRoleDesc) === '') {
return true;
}
const roleDesc = virtualNode.attr('aria-roledescription');
if (typeof roleDesc !== 'string') {
this.data({ messageKey: 'noRoleDescription' });
return false;
}

if (sanitize(roleDesc) === '') {
this.data({ messageKey: 'emptyRoleDescription' });
return false;
}
return true;
}
14 changes: 14 additions & 0 deletions lib/checks/aria/braille-roledescription-equivalent.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"id": "braille-roledescription-equivalent",
"evaluate": "braille-roledescription-equivalent-evaluate",
"metadata": {
"impact": "serious",
"messages": {
"pass": "aria-brailleroledescription is not used on an element with no accessible text",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"pass": "aria-brailleroledescription is not used on an element with no accessible text",
"pass": "aria-brailleroledescription is used on an element with aria-roledescription",

"fail": {
"noRoleDescription": "aria-brailleroledescription is used on an element with no aria-roledescription",
"emptyRoleDescription": "aria-brailleroledescription is used on an element with an empty aria-roledescription"
}
}
}
}
12 changes: 12 additions & 0 deletions lib/rules/aria-braille-equivalent.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"id": "aria-braille-equivalent",
"selector": "[aria-brailleroledescription], [aria-braillelabel]",
"tags": ["cat.aria", "wcag2a", "wcag412", "EN-301-549", "EN-9.4.1.2"],
"metadata": {
"description": "Ensure aria-braillelabel and aria-brailleroledescription have a non-braille equivalent",
"help": "aria-braille attributes must have a non-braille equivalent"
},
"all": ["braille-roledescription-equivalent", "braille-label-equivalent"],
"any": [],
"none": []
}
16 changes: 16 additions & 0 deletions locales/_template.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
"description": "Ensures role attribute has an appropriate value for the element",
"help": "ARIA role should be appropriate for the element"
},
"aria-braille-equivalent": {
"description": "Ensure aria-braillelabel and aria-brailleroledescription have a non-braille equivalent",
"help": "aria-braille attributes must have a non-braille equivalent"
},
"aria-command-name": {
"description": "Ensures every ARIA button, link and menuitem has an accessible name",
"help": "ARIA commands must have an accessible name"
Expand Down Expand Up @@ -537,6 +541,18 @@
"plural": "Invalid ARIA attribute names: ${data.values}"
}
},
"braille-label-equivalent": {
"pass": "aria-braillelabel is used on an element with accessible text",
"fail": "aria-braillelabel is used on an element with no accessible text",
"incomplete": "Unable to compute accessible text"
},
"braille-roledescription-equivalent": {
"pass": "aria-brailleroledescription is not used on an element with no accessible text",
"fail": {
"noRoleDescription": "aria-brailleroledescription is used on an element with no aria-roledescription",
"emptyRoleDescription": "aria-brailleroledescription is used on an element with an empty aria-roledescription"
}
},
"deprecatedrole": {
"pass": "ARIA role is not deprecated",
"fail": "The role used is deprecated: ${data}"
Expand Down
51 changes: 51 additions & 0 deletions test/checks/aria/braille-label-equivalent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
describe('braille-label-equivalent tests', () => {
const { checkSetup, getCheckEvaluate } = axe.testUtils;
const checkContext = axe.testUtils.MockCheckContext();
const checkEvaluate = getCheckEvaluate('braille-label-equivalent');

afterEach(() => {
checkContext.reset();
});

it('returns true without aria-braillelabel', () => {
const params = checkSetup('<img id="target" alt="" />');
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

it('returns true when aria-braillelabel is empty', () => {
const params = checkSetup(
'<img id="target" alt="" aria-braillelabel="" />'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

it('returns true when aria-braillelabel is whitespace-only', () => {
const params = checkSetup(
'<img id="target" alt="" aria-braillelabel=" \r\t\n " />'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

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" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

`);
assert.isFalse(checkEvaluate.apply(checkContext, params));
});

it('returns false when the accessible name has only whitespace', () => {
const params = checkSetup(`
<img id="target" alt=" \r\t\n " aria-braillelabel="foo" />
`);
assert.isFalse(checkEvaluate.apply(checkContext, params));
});

it('returns true when the accessible name is not empty', () => {
const params = checkSetup(`
<img id="target" alt="foo" aria-braillelabel="foo" />
`);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});
});
});
80 changes: 80 additions & 0 deletions test/checks/aria/braille-roledescription-equivalent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
describe('braille-roledescription-equivalent tests', () => {
const { checkSetup, getCheckEvaluate } = axe.testUtils;
const checkContext = axe.testUtils.MockCheckContext();
const checkEvaluate = getCheckEvaluate('braille-roledescription-equivalent');

afterEach(() => {
checkContext.reset();
});

it('returns true without aria-brailleroledescription', () => {
const params = checkSetup('<div id="target"></div>');
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

it('returns true when aria-brailleroledecription is empty', () => {
const params = checkSetup(
'<div id="target" aria-brailleroledescription=""></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

it('returns true when aria-brailleroledecription is whitespace-only', () => {
const params = checkSetup(
'<div id="target" aria-brailleroledescription=" \r\t\n "></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});

describe('when aria-brailleroledescription has text', () => {
it('returns false without aria-roledescription', () => {
const params = checkSetup(`
<div
id="target"
aria-brailleroledescription="foo"
></div>
`);
assert.isFalse(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, { messageKey: 'noRoleDescription' });
});

it('returns false when aria-roledescription is empty', () => {
const params = checkSetup(`
<div
id="target"
aria-roledescription=""
aria-brailleroledescription="foo"
></div>
`);
assert.isFalse(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
messageKey: 'emptyRoleDescription'
});
});

it('returns false when aria-roledescription has only whitespace', () => {
const params = checkSetup(`
<div
id="target"
aria-roledescription=" \r\t\n "
aria-brailleroledescription="foo"
></div>
`);
assert.isFalse(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
messageKey: 'emptyRoleDescription'
});
});

it('returns true when aria-roledescription is not empty', () => {
const params = checkSetup(`
<div
id="target"
aria-roledescription="foo"
aria-brailleroledescription="foo"
></div>
`);
assert.isTrue(checkEvaluate.apply(checkContext, params));
});
});
});
2 changes: 1 addition & 1 deletion test/integration/full/all-rules/all-rules.html
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ <h2>Ok</h2>
</tr>
</table>

<img src="img.jpg" alt="" />
<img src="img.jpg" alt="" aria-braillelabel="image" />
<video><track kind="captions" /></video>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand Down
2 changes: 1 addition & 1 deletion test/integration/full/isolated-env/isolated-env.html
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ <h2>Ok</h2>
</tr>
</table>

<img src="img.jpg" alt="" />
<img src="img.jpg" alt="" aria-braillelabel="my image" />
<video><track kind="captions" /></video>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<button id="pass1" aria-braillelabel="hello">Hello</button>
<button id="pass2" aria-braillelabel=""></button>
<button id="fail1" aria-braillelabel="hello"></button>

<aside
id="pass3"
aria-roledescription="table of contents"
aria-brailleroledescription=""
></aside>

<aside
id="pass4"
aria-roledescription="table of contents"
aria-brailleroledescription="table of contents"
></aside>

<aside
id="pass5"
aria-roledescription=""
aria-brailleroledescription=""
></aside>

<aside
id="fail2"
aria-roledescription=""
aria-brailleroledescription="table of contents"
></aside>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"description": "aria-braille-equivalent tests",
"rule": "aria-braille-equivalent",
"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"], ["#pass5"]],
"violations": [["#fail1"], ["#fail2"]]
}
74 changes: 74 additions & 0 deletions test/integration/virtual-rules/aria-braille-equivalent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
describe('aria-braille-equivalent virtual-rule', () => {
afterEach(() => {
axe.reset();
});
Comment on lines +2 to +4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this isn't necessary

Suggested change
afterEach(() => {
axe.reset();
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration isn't reverted until axe.reset() is called. It's not in testutils. Where would you think this might happen?

Copy link
Contributor

@straker straker Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


it('passes when aria-braillelabel is not empty', () => {
const results = axe.runVirtualRule('aria-braille-equivalent', {
nodeName: 'img',
attributes: {
alt: 'Hello world',
'aria-braillelabel': 'Hello world'
}
});

assert.lengthOf(results.passes, 1);
assert.lengthOf(results.violations, 0);
assert.lengthOf(results.incomplete, 0);
});

it('fails when accessible text is empty but braille label is not', () => {
const results = axe.runVirtualRule('aria-braille-equivalent', {
nodeName: 'img',
attributes: {
alt: '',
'aria-braillelabel': 'hello world'
}
});

assert.lengthOf(results.passes, 0);
assert.lengthOf(results.violations, 1);
assert.lengthOf(results.incomplete, 0);
});

it('passes when roledescription and brailleroledescription are not empty', () => {
const results = axe.runVirtualRule('aria-braille-equivalent', {
nodeName: 'div',
attributes: {
'aria-roledescription': 'Hello world',
'aria-brailleroledescription': 'Hello world'
}
});

assert.lengthOf(results.passes, 1);
assert.lengthOf(results.violations, 0);
assert.lengthOf(results.incomplete, 0);
});

it('fails when roledescription is empty but brailleroledescription is not', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const results = axe.runVirtualRule('aria-braille-equivalent', {
nodeName: 'div',
attributes: {
'aria-roledescription': '',
'aria-brailleroledescription': 'Hello world'
}
});

assert.lengthOf(results.passes, 0);
assert.lengthOf(results.violations, 1);
assert.lengthOf(results.incomplete, 0);
});

it('incompletes if the subtree fails to compute with aria-braillelabel', () => {
const results = axe.runVirtualRule('aria-braille-equivalent', {
nodeName: 'button',
attributes: {
'aria-braillelabel': 'Hello world'
}
});

assert.lengthOf(results.passes, 0);
assert.lengthOf(results.violations, 0);
assert.lengthOf(results.incomplete, 1);
});
});