fix(aria-allowed-attr): no inconsistent aria-checked on HTML checkbox…

…es (#3895) * feat(aria-allowed-attr): no inconsistent aria-checked on HTML checkboxes * Add integration tests * Add non-table row test * Cleanup / docs work * Handle checkbox indeterminate state * Fix virtual rule test * Apply suggestions from code review Co-authored-by: Steven Lambert <[email protected]> * Fix broken test --------- Co-authored-by: Steven Lambert <[email protected]>
dequelabs · May 2, 2023 · 704043e · 704043e
1 parent 0f405c6
commit 704043e
Show file tree

Hide file tree

Showing 18 changed files with 498 additions and 173 deletions.
diff --git a/doc/check-options.md b/doc/check-options.md
@@ -207,6 +207,10 @@ All checks allow these global options:
 
 ### aria-allowed-attr
 
+Previously supported properties `validTreeRowAttrs` is no longer available. `invalidTableRowAttrs` from [aria-conditional-attr](#aria-conditional-attr) instead.
+
+### aria-conditional-attr
+
 <table>
   <thead>
     <tr>
@@ -218,7 +222,7 @@ All checks allow these global options:
   <tbody>
     <tr>
       <td>
-        <code>validTreeRowAttrs</code>
+        <code>invalidTableRowAttrs</code>
       </td>
       <td align="left">
         <pre lang=js><code>[

diff --git a/lib/checks/aria/aria-allowed-attr-evaluate.js b/lib/checks/aria/aria-allowed-attr-evaluate.js
@@ -1,7 +1,6 @@
-import { uniqueArray, closest, isHtmlElement } from '../../core/utils';
+import { uniqueArray, isHtmlElement } from '../../core/utils';
 import { getRole, allowedAttr, validateAttr } from '../../commons/aria';
 import { isFocusable } from '../../commons/dom';
-import cache from '../../core/base/cache';
 
 /**
  * Check if each ARIA attribute on an element is allowed for its semantic role.
@@ -30,62 +29,31 @@ import cache from '../../core/base/cache';
 export default function ariaAllowedAttrEvaluate(node, options, virtualNode) {
   const invalid = [];
   const role = getRole(virtualNode);
-  const attrs = virtualNode.attrNames;
   let allowed = allowedAttr(role);
+
   // @deprecated: allowed attr options to pass more attrs.
   // configure the standards spec instead
   if (Array.isArray(options[role])) {
     allowed = uniqueArray(options[role].concat(allowed));
   }
 
-  const tableMap = cache.get('aria-allowed-attr-table', () => new WeakMap());
-
-  function validateRowAttrs() {
-    // check if the parent exists otherwise a TypeError will occur (virtual-nodes specifically)
-    if (virtualNode.parent && role === 'row') {
-      const table = closest(
-        virtualNode,
-        'table, [role="treegrid"], [role="table"], [role="grid"]'
-      );
-
-      let tableRole = tableMap.get(table);
-      if (table && !tableRole) {
-        tableRole = getRole(table);
-        tableMap.set(table, tableRole);
-      }
-      if (['table', 'grid'].includes(tableRole) && role === 'row') {
-        return true;
-      }
-    }
-  }
-  // Allows options to be mapped to object e.g. {'aria-level' : validateRowAttrs}
-  const ariaAttr = Array.isArray(options.validTreeRowAttrs)
-    ? options.validTreeRowAttrs
-    : [];
-  const preChecks = {};
-  ariaAttr.forEach(attr => {
-    preChecks[attr] = validateRowAttrs;
-  });
-  if (allowed) {
-    for (let i = 0; i < attrs.length; i++) {
-      const attrName = attrs[i];
-      if (validateAttr(attrName) && preChecks[attrName]?.()) {
-        invalid.push(attrName + '="' + virtualNode.attr(attrName) + '"');
-      } else if (validateAttr(attrName) && !allowed.includes(attrName)) {
-        invalid.push(attrName + '="' + virtualNode.attr(attrName) + '"');
-      }
+  // Unknown ARIA attributes are tested in aria-valid-attr
+  for (const attrName of virtualNode.attrNames) {
+    if (validateAttr(attrName) && !allowed.includes(attrName)) {
+      invalid.push(attrName);
     }
   }
 
-  if (invalid.length) {
-    this.data(invalid);
+  if (!invalid.length) {
+    return true;
+  }
 
-    if (!isHtmlElement(virtualNode) && !role && !isFocusable(virtualNode)) {
-      return undefined;
-    }
+  this.data(
+    invalid.map(attrName => attrName + '="' + virtualNode.attr(attrName) + '"')
+  );
 
-    return false;
+  if (!role && !isHtmlElement(virtualNode) && !isFocusable(virtualNode)) {
+    return undefined;
   }
-
-  return true;
+  return false;
 }
diff --git a/lib/checks/aria/aria-conditional-attr-evaluate.js b/lib/checks/aria/aria-conditional-attr-evaluate.js
@@ -0,0 +1,20 @@
+import getRole from '../../commons/aria/get-role';
+import ariaConditionalCheckboxAttr from './aria-conditional-checkbox-attr-evaluate';
+import ariaConditionalRowAttr from './aria-conditional-row-attr-evaluate';
+
+const conditionalRoleMap = {
+  row: ariaConditionalRowAttr,
+  checkbox: ariaConditionalCheckboxAttr
+};
+
+export default function ariaConditionalAttrEvaluate(
+  node,
+  options,
+  virtualNode
+) {
+  const role = getRole(virtualNode);
+  if (!conditionalRoleMap[role]) {
+    return true;
+  }
+  return conditionalRoleMap[role].call(this, node, options, virtualNode);
+}
diff --git a/lib/checks/aria/aria-conditional-attr.json b/lib/checks/aria/aria-conditional-attr.json
@@ -0,0 +1,23 @@
+{
+  "id": "aria-conditional-attr",
+  "evaluate": "aria-conditional-attr-evaluate",
+  "options": {
+    "invalidTableRowAttrs": [
+      "aria-posinset",
+      "aria-setsize",
+      "aria-expanded",
+      "aria-level"
+    ]
+  },
+  "metadata": {
+    "impact": "serious",
+    "messages": {
+      "pass": "ARIA attribute is allowed",
+      "fail": {
+        "checkbox": "Remove aria-checked, or set it to \"${data.checkState}\" to match the real checkbox state",
+        "rowSingular": "This attribute is supported with treegrid rows, but not ${data.ownerRole}: ${data.invalidAttrs}",
+        "rowPlural": "These attributes are supported with treegrid rows, but not ${data.ownerRole}: ${data.invalidAttrs}"
+      }
+    }
+  }
+}
diff --git a/lib/checks/aria/aria-conditional-checkbox-attr-evaluate.js b/lib/checks/aria/aria-conditional-checkbox-attr-evaluate.js
@@ -0,0 +1,39 @@
+export default function ariaConditionalCheckboxAttr(
+  node,
+  options,
+  virtualNode
+) {
+  const { nodeName, type } = virtualNode.props;
+  const ariaChecked = normalizeAriaChecked(virtualNode.attr('aria-checked'));
+  if (nodeName !== 'input' || type !== 'checkbox' || !ariaChecked) {
+    return true;
+  }
+
+  const checkState = getCheckState(virtualNode);
+  if (ariaChecked === checkState) {
+    return true;
+  }
+  this.data({
+    messageKey: 'checkbox',
+    checkState
+  });
+  return false;
+}
+
+function getCheckState(vNode) {
+  if (vNode.props.indeterminate) {
+    return 'mixed';
+  }
+  return vNode.props.checked ? 'true' : 'false';
+}
+
+function normalizeAriaChecked(ariaCheckedVal) {
+  if (!ariaCheckedVal) {
+    return '';
+  }
+  ariaCheckedVal = ariaCheckedVal.toLowerCase();
+  if (['mixed', 'true'].includes(ariaCheckedVal)) {
+    return ariaCheckedVal;
+  }
+  return 'false';
+}
diff --git a/lib/checks/aria/aria-conditional-row-attr-evaluate.js b/lib/checks/aria/aria-conditional-row-attr-evaluate.js
@@ -0,0 +1,36 @@
+import getRole from '../../commons/aria/get-role';
+import { closest } from '../../core/utils';
+
+export default function ariaConditionalRowAttr(
+  node,
+  { invalidTableRowAttrs } = {},
+  virtualNode
+) {
+  const invalidAttrs =
+    invalidTableRowAttrs?.filter?.(invalidAttr => {
+      return virtualNode.hasAttr(invalidAttr);
+    }) ?? [];
+  if (invalidAttrs.length === 0) {
+    return true;
+  }
+
+  const owner = getRowOwner(virtualNode);
+  const ownerRole = owner && getRole(owner);
+  if (!ownerRole || ownerRole === 'treegrid') {
+    return true;
+  }
+
+  const messageKey = `row${invalidAttrs.length > 1 ? 'Plural' : 'Singular'}`;
+  this.data({ messageKey, invalidAttrs, ownerRole });
+  return false;
+}
+
+function getRowOwner(virtualNode) {
+  // check if the parent exists otherwise a TypeError will occur (virtual-nodes specifically)
+  if (!virtualNode.parent) {
+    return;
+  }
+  const rowOwnerQuery =
+    'table:not([role]), [role~="treegrid"], [role~="table"], [role~="grid"]';
+  return closest(virtualNode, rowOwnerQuery);
+}
diff --git a/lib/core/base/virtual-node/virtual-node.js b/lib/core/base/virtual-node/virtual-node.js
@@ -56,8 +56,17 @@ class VirtualNode extends AbstractVirtualNode {
   // add to the prototype so memory is shared across all virtual nodes
   get props() {
     if (!this._cache.hasOwnProperty('props')) {
-      const { nodeType, nodeName, id, multiple, nodeValue, value, selected } =
-        this.actualNode;
+      const {
+        nodeType,
+        nodeName,
+        id,
+        multiple,
+        nodeValue,
+        value,
+        selected,
+        checked,
+        indeterminate
+      } = this.actualNode;
 
       this._cache.props = {
         nodeType,
@@ -67,7 +76,9 @@ class VirtualNode extends AbstractVirtualNode {
         multiple,
         nodeValue,
         value,
-        selected
+        selected,
+        checked,
+        indeterminate
       };
     }
 

diff --git a/lib/rules/aria-allowed-attr.json b/lib/rules/aria-allowed-attr.json
@@ -7,7 +7,7 @@
     "description": "Ensures ARIA attributes are allowed for an element's role",
     "help": "Elements must only use allowed ARIA attributes"
   },
-  "all": [],
-  "any": ["aria-allowed-attr"],
+  "all": ["aria-allowed-attr", "aria-conditional-attr"],
+  "any": [],
   "none": ["aria-unsupported-attr", "aria-prohibited-attr"]
 }
diff --git a/locales/_template.json b/locales/_template.json
@@ -429,6 +429,14 @@
       "pass": "Element has an aria-busy attribute",
       "fail": "Element uses aria-busy=\"true\" while showing a loader"
     },
+    "aria-conditional-attr": {
+      "pass": "ARIA attribute is allowed",
+      "fail": {
+        "checkbox": "Remove aria-checked, or set it to \"${data.checkState}\" to match the real checkbox state",
+        "rowSingular": "This attribute is supported with treegrid rows, but not ${data.ownerRole}: ${data.invalidAttrs}",
+        "rowPlural": "These attributes are supported with treegrid rows, but not ${data.ownerRole}: ${data.invalidAttrs}"
+      }
+    },
     "aria-errormessage": {
       "pass": "aria-errormessage exists and references elements visible to screen readers that use a supported aria-errormessage technique",
       "fail": {