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
dequelabs · Mar 24, 2023 · cce7586 · cce7586
1 parent 2842395
commit cce7586
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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(`
+      <div id="target" role="list">
+        <div role="tabpanel"></div>
+        <div role="listitem">List item 1</div>
+        <div role="tabpanel"></div>
+      </div>
+    `);
+    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(
       '<div id="target" role="list">' +