From cce75869be032006dc505a2af853886943973319 Mon Sep 17 00:00:00 2001
From: Steven Lambert <2433219+straker@users.noreply.github.com>
Date: Fri, 24 Mar 2023 08:47:48 -0600
Subject: [PATCH] fix(aria-required-children): list elements that are not
allowed (#3951)
* fix(aria-required-children): list elements that are not allowed
* jsdoc
* fix tests
* remove dups
---
.../aria/aria-required-children-evaluate.js | 145 ++++++++++++------
lib/checks/aria/aria-required-children.json | 2 +-
locales/_template.json | 2 +-
test/checks/aria/required-children.js | 35 ++++-
4 files changed, 129 insertions(+), 55 deletions(-)
diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js
index 205ef1c970..940c824b62 100644
--- a/lib/checks/aria/aria-required-children-evaluate.js
+++ b/lib/checks/aria/aria-required-children-evaluate.js
@@ -12,6 +12,71 @@ import {
isVisibleToScreenReaders
} from '../../commons/dom';
+/**
+ * Check that an element owns all required children for its explicit role.
+ *
+ * Required roles are taken from the `ariaRoles` standards object from the roles `requiredOwned` property.
+ *
+ * @memberof checks
+ * @param {Boolean} options.reviewEmpty List of ARIA roles that should be flagged as "Needs Review" rather than a violation if the element has no owned children.
+ * @data {String[]} List of all missing owned roles.
+ * @returns {Mixed} True if the element owns all required roles. Undefined if `options.reviewEmpty=true` and the element has no owned children. False otherwise.
+ */
+export default function ariaRequiredChildrenEvaluate(
+ node,
+ options,
+ virtualNode
+) {
+ const reviewEmpty =
+ options && Array.isArray(options.reviewEmpty) ? options.reviewEmpty : [];
+ const role = getExplicitRole(virtualNode, { dpub: true });
+ const required = requiredOwned(role);
+ if (required === null) {
+ return true;
+ }
+
+ const ownedRoles = getOwnedRoles(virtualNode, required);
+ const unallowed = ownedRoles.filter(({ role }) => !required.includes(role));
+
+ if (unallowed.length) {
+ this.relatedNodes(unallowed.map(({ ownedElement }) => ownedElement));
+ this.data({
+ messageKey: 'unallowed',
+ values: unallowed
+ .map(({ ownedElement, attr }) =>
+ getUnallowedSelector(ownedElement, attr)
+ )
+ .filter((selector, index, array) => array.indexOf(selector) === index)
+ .join(', ')
+ });
+ return false;
+ }
+
+ const missing = missingRequiredChildren(
+ virtualNode,
+ role,
+ required,
+ ownedRoles
+ );
+ if (!missing) {
+ return true;
+ }
+
+ this.data(missing);
+
+ // Only review empty nodes when a node is both empty and does not have an aria-owns relationship
+ if (
+ reviewEmpty.includes(role) &&
+ !hasContentVirtual(virtualNode, false, true) &&
+ !ownedRoles.length &&
+ (!virtualNode.hasAttr('aria-owns') || !idrefs(node, 'aria-owns').length)
+ ) {
+ return undefined;
+ }
+
+ return false;
+}
+
/**
* Get all owned roles of an element
*/
@@ -26,10 +91,9 @@ function getOwnedRoles(virtualNode, required) {
const role = getRole(ownedElement, { noPresentational: true });
- const hasGlobalAria = getGlobalAriaAttrs().some(attr =>
- ownedElement.hasAttr(attr)
- );
- const hasGlobalAriaOrFocusable = hasGlobalAria || isFocusable(ownedElement);
+ const globalAriaAttr = getGlobalAriaAttr(ownedElement);
+ const hasGlobalAriaOrFocusable =
+ !!globalAriaAttr || isFocusable(ownedElement);
// if owned node has no role or is presentational, or if role
// allows group or rowgroup, we keep parsing the descendant tree.
@@ -43,7 +107,11 @@ function getOwnedRoles(virtualNode, required) {
) {
ownedElements.push(...ownedElement.children);
} else if (role || hasGlobalAriaOrFocusable) {
- ownedRoles.push({ role, ownedElement });
+ ownedRoles.push({
+ role,
+ attr: globalAriaAttr || 'tabindex',
+ ownedElement
+ });
}
}
@@ -71,58 +139,35 @@ function missingRequiredChildren(virtualNode, role, required, ownedRoles) {
}
/**
- * Check that an element owns all required children for its explicit role.
- *
- * Required roles are taken from the `ariaRoles` standards object from the roles `requiredOwned` property.
- *
- * @memberof checks
- * @param {Boolean} options.reviewEmpty List of ARIA roles that should be flagged as "Needs Review" rather than a violation if the element has no owned children.
- * @data {String[]} List of all missing owned roles.
- * @returns {Mixed} True if the element owns all required roles. Undefined if `options.reviewEmpty=true` and the element has no owned children. False otherwise.
+ * Get the first global ARIA attribute the element has.
+ * @param {VirtualNode} vNode
+ * @return {String|null}
*/
-function ariaRequiredChildrenEvaluate(node, options, virtualNode) {
- const reviewEmpty =
- options && Array.isArray(options.reviewEmpty) ? options.reviewEmpty : [];
- const role = getExplicitRole(virtualNode, { dpub: true });
- const required = requiredOwned(role);
- if (required === null) {
- return true;
- }
+function getGlobalAriaAttr(vNode) {
+ return getGlobalAriaAttrs().find(attr => vNode.hasAttr(attr));
+}
- const ownedRoles = getOwnedRoles(virtualNode, required);
- const unallowed = ownedRoles.filter(({ role }) => !required.includes(role));
+/**
+ * Return a simple selector for an unallowed element.
+ * @param {VirtualNode} vNode
+ * @param {String} [attr] - Optional attribute which made the element unallowed
+ * @return {String}
+ */
+function getUnallowedSelector(vNode, attr) {
+ const { nodeName, nodeType } = vNode.props;
- if (unallowed.length) {
- this.relatedNodes(unallowed.map(({ ownedElement }) => ownedElement));
- this.data({
- messageKey: 'unallowed'
- });
- return false;
+ if (nodeType === 3) {
+ return `#text`;
}
- const missing = missingRequiredChildren(
- virtualNode,
- role,
- required,
- ownedRoles
- );
- if (!missing) {
- return true;
+ const role = getExplicitRole(vNode, { dpub: true });
+ if (role) {
+ return `[role=${role}]`;
}
- this.data(missing);
-
- // Only review empty nodes when a node is both empty and does not have an aria-owns relationship
- if (
- reviewEmpty.includes(role) &&
- !hasContentVirtual(virtualNode, false, true) &&
- !ownedRoles.length &&
- (!virtualNode.hasAttr('aria-owns') || !idrefs(node, 'aria-owns').length)
- ) {
- return undefined;
+ if (attr) {
+ return nodeName + `[${attr}]`;
}
- return false;
+ return nodeName;
}
-
-export default ariaRequiredChildrenEvaluate;
diff --git a/lib/checks/aria/aria-required-children.json b/lib/checks/aria/aria-required-children.json
index 38618dbc19..db1e2bf630 100644
--- a/lib/checks/aria/aria-required-children.json
+++ b/lib/checks/aria/aria-required-children.json
@@ -24,7 +24,7 @@
"fail": {
"singular": "Required ARIA child role not present: ${data.values}",
"plural": "Required ARIA children role not present: ${data.values}",
- "unallowed": "Element has children which are not allowed (see related nodes)"
+ "unallowed": "Element has children which are not allowed: ${data.values}"
},
"incomplete": {
"singular": "Expecting ARIA child role to be added: ${data.values}",
diff --git a/locales/_template.json b/locales/_template.json
index 7ac37e6d57..e41a1b4514 100644
--- a/locales/_template.json
+++ b/locales/_template.json
@@ -477,7 +477,7 @@
"fail": {
"singular": "Required ARIA child role not present: ${data.values}",
"plural": "Required ARIA children role not present: ${data.values}",
- "unallowed": "Element has children which are not allowed (see related nodes)"
+ "unallowed": "Element has children which are not allowed: ${data.values}"
},
"incomplete": {
"singular": "Expecting ARIA child role to be added: ${data.values}",
diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js
index 34c5c312c4..47191efb09 100644
--- a/test/checks/aria/required-children.js
+++ b/test/checks/aria/required-children.js
@@ -153,7 +153,10 @@ describe('aria-required-children', () => {
axe._tree,
'[role="tabpanel"]'
)[0];
- assert.deepEqual(checkContext._data, { messageKey: 'unallowed' });
+ assert.deepEqual(checkContext._data, {
+ messageKey: 'unallowed',
+ values: '[role=tabpanel]'
+ });
assert.deepEqual(checkContext._relatedNodes, [unallowed]);
});
@@ -171,7 +174,10 @@ describe('aria-required-children', () => {
axe._tree,
'[aria-live="polite"]'
)[0];
- assert.deepEqual(checkContext._data, { messageKey: 'unallowed' });
+ assert.deepEqual(checkContext._data, {
+ messageKey: 'unallowed',
+ values: 'div[aria-live]'
+ });
assert.deepEqual(checkContext._relatedNodes, [unallowed]);
});
@@ -189,10 +195,33 @@ describe('aria-required-children', () => {
axe._tree,
'[tabindex="0"]'
)[0];
- assert.deepEqual(checkContext._data, { messageKey: 'unallowed' });
+ assert.deepEqual(checkContext._data, {
+ messageKey: 'unallowed',
+ values: 'div[tabindex]'
+ });
assert.deepEqual(checkContext._relatedNodes, [unallowed]);
});
+ it('should remove duplicate unallowed selectors', () => {
+ const params = checkSetup(`
+
+ `);
+ assert.isFalse(
+ axe.testUtils
+ .getCheckEvaluate('aria-required-children')
+ .apply(checkContext, params)
+ );
+
+ assert.deepEqual(checkContext._data, {
+ messageKey: 'unallowed',
+ values: '[role=tabpanel]'
+ });
+ });
+
it('should pass when list has child with aria-hidden', () => {
const params = checkSetup(
'' +