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.

Sign up for GitHub

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

Jump to bottom

fix(aria-valid-attr-value): report incomplete when idrefs are not required #4204

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
+31 −42
Draft

fix(aria-valid-attr-value): report incomplete when idrefs are not required #4204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion doc/rule-descriptions.md
View file
Open in desktop
Expand Up @@ -32,7 +32,7 @@
| [aria-roles](https://dequeuniversity.com/rules/axe/4.8/aria-roles?application=RuleDescription) | Ensures all elements with a role attribute use a valid value | Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure | [674b10](https://act-rules.github.io/rules/674b10) |
| [aria-toggle-field-name](https://dequeuniversity.com/rules/axe/4.8/aria-toggle-field-name?application=RuleDescription) | Ensures every ARIA toggle field has an accessible name | Serious | cat.aria, wcag2a, wcag412, TTv5, TT5.c, EN-301-549, EN-9.4.1.2, ACT | failure, needs review | [e086e5](https://act-rules.github.io/rules/e086e5) |
| [aria-tooltip-name](https://dequeuniversity.com/rules/axe/4.8/aria-tooltip-name?application=RuleDescription) | Ensures every ARIA tooltip node has an accessible name | Serious | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure, needs review | |
| [aria-valid-attr-value](https://dequeuniversity.com/rules/axe/4.8/aria-valid-attr-value?application=RuleDescription) | Ensures all ARIA attributes have valid values | Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure, needs review | [6a7281](https://act-rules.github.io/rules/6a7281) |
| [aria-valid-attr-value](https://dequeuniversity.com/rules/axe/4.8/aria-valid-attr-value?application=RuleDescription) | Ensures all ARIA attributes have valid values | Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure, needs review | [6a7281](https://act-rules.github.io/rules/6a7281), [in6db8](https://act-rules.github.io/rules/in6db8) |
| [aria-valid-attr](https://dequeuniversity.com/rules/axe/4.8/aria-valid-attr?application=RuleDescription) | Ensures attributes that begin with aria- are valid ARIA attributes | Critical | cat.aria, wcag2a, wcag412, EN-301-549, EN-9.4.1.2 | failure | [5f99a7](https://act-rules.github.io/rules/5f99a7) |
| [blink](https://dequeuniversity.com/rules/axe/4.8/blink?application=RuleDescription) | Ensures <blink> elements are not used | Serious | cat.time-and-media, wcag2a, wcag222, section508, section508.22.j, TTv5, TT2.b, EN-301-549, EN-9.2.2.2 | failure | |
| [button-name](https://dequeuniversity.com/rules/axe/4.8/button-name?application=RuleDescription) | Ensures buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a, TTv5, TT6.a, EN-301-549, EN-9.4.1.2, ACT | failure, needs review | [97a4e1](https://act-rules.github.io/rules/97a4e1), [m6b1q3](https://act-rules.github.io/rules/m6b1q3) |
Expand Down
49 changes: 19 additions & 30 deletions lib/checks/aria/aria-valid-attr-value-evaluate.js
View file
Open in desktop
@@ -1,4 +1,8 @@
import { validateAttrValue } from '../../commons/aria';
import {
validateAttrValue,
getExplicitRole,
requiredAttr as getRequiredAttrs
} from '../../commons/aria';
import standards from '../../standards';

/**
Expand Down Expand Up @@ -34,6 +38,9 @@ export default function ariaValidAttrValueEvaluate(node, options, virtualNode) {
const aria = /^aria-/;
const skipAttrs = ['aria-errormessage'];

const role = getExplicitRole(virtualNode);
const requiredAttrs = getRequiredAttrs(role);

const preChecks = {
// aria-controls should only check if element exists if the element
// doesn't have aria-expanded=false or aria-selected=false (tabs)
Expand All @@ -59,34 +66,6 @@ export default function ariaValidAttrValueEvaluate(node, options, virtualNode) {
// @see https://github.com/dequelabs/axe-core/issues/1524
'aria-owns': () => {
return virtualNode.attr('aria-expanded') !== 'false';
},
// aria-describedby should not mark missing element as violation but
// instead as needs review
// @see https://github.com/dequelabs/axe-core/issues/1151
'aria-describedby': validValue => {
if (!validValue) {
needsReview = `aria-describedby="${virtualNode.attr(
'aria-describedby'
)}"`;
// TODO: es-modules_tree
messageKey =
axe._tree && axe._tree[0]._hasShadowRoot ? 'noIdShadow' : 'noId';
}

return;
},
// aria-labelledby should not mark missing element as violation but
// instead as needs review
// @see https://github.com/dequelabs/axe-core/issues/2621
'aria-labelledby': validValue => {
if (!validValue) {
needsReview = `aria-labelledby="${virtualNode.attr(
'aria-labelledby'
)}"`;
// TODO: es-modules_tree
messageKey =
axe._tree && axe._tree[0]._hasShadowRoot ? 'noIdShadow' : 'noId';
}
}
};

Expand Down Expand Up @@ -114,7 +93,17 @@ export default function ariaValidAttrValueEvaluate(node, options, virtualNode) {
(preChecks[attrName] ? preChecks[attrName](validValue) : true) &&
!validValue
) {
if (attrValue === '' && !isStringType(attrName)) {
const attrType = standards.ariaAttrs[attrName]?.type;
// Only required IDREFs can fail this check
if (
['idref', 'idrefs'].includes(attrType) &&
!requiredAttrs.includes(attrName)
) {
needsReview = `aria-labelledby="${attrValue}"`;
// TODO: es-modules_tree
messageKey =
axe._tree && axe._tree[0]._hasShadowRoot ? 'noIdShadow' : 'noId';
Comment on lines +96 to +105
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure yet. Perhaps IDREF checking should go into a separate check?

} else if (attrValue === '' && !isStringType(attrName)) {
needsReview = attrName;
messageKey = 'empty';
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/aria-valid-attr-value.json
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
"impact": "critical",
"matches": "aria-has-attr-matches",
"tags": ["cat.aria", "wcag2a", "wcag412", "EN-301-549", "EN-9.4.1.2"],
"actIds": ["6a7281"],
"actIds": ["6a7281", "in6db8"],
"metadata": {
"description": "Ensures all ARIA attributes have valid values",
"help": "ARIA attributes must conform to valid values"
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -174,7 +174,7 @@
"typedarray": "^0.0.7",
"typescript": "^5.2.2",
"uglify-js": "^3.17.4",
"wcag-act-rules": "github:w3c/wcag-act-rules#2341a1b",
"wcag-act-rules": "github:w3c/wcag-act-rules#f03fb85",
"weakmap-polyfill": "^2.0.4"
},
"lint-staged": {
Expand Down
5 changes: 5 additions & 0 deletions test/act-rules/aria-required-id-references-in6db8.spec.js
View file
Open in desktop
@@ -0,0 +1,5 @@
require('./act-runner.js')({
id: 'in6db8',
title: 'ARIA required ID references exist',
axeRules: ['aria-valid-attr-value']
});
5 changes: 0 additions & 5 deletions test/act-rules/id-value-unique-3ea0c8.spec.js
View file
Open in desktop

This file was deleted.