feat(aria-allowed-attr): report violation for non-global ARIA attrs o…

…n element without role (#3342) * feat(aria-allowed-attr): report violation for non-global ARIA attrs on element without role * Update lib/checks/aria/aria-allowed-attr.json Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> * no role or focusable * ignore button/button_idl.html error * if * Update lib/checks/aria/aria-allowed-attr.json Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
dequelabs · Jan 13, 2022 · fb5d990 · fb5d990
1 parent 53a6684
commit fb5d990
Show file tree

Hide file tree

Showing 9 changed files with 96 additions and 37 deletions.
diff --git a/lib/checks/aria/aria-allowed-attr-evaluate.js b/lib/checks/aria/aria-allowed-attr-evaluate.js
@@ -1,5 +1,6 @@
-import { uniqueArray, closest } from '../../core/utils';
+import { uniqueArray, closest, isHtmlElement } from '../../core/utils';
 import { getRole, allowedAttr, validateAttr } from '../../commons/aria';
+import { isFocusable } from '../../commons/dom';
 import cache from '../../core/base/cache';
 
 /**
@@ -69,7 +70,7 @@ function ariaAllowedAttrEvaluate(node, options, virtualNode) {
   ariaAttr.forEach(attr => {
     preChecks[attr] = validateRowAttrs;
   });
-  if (role && allowed) {
+  if (allowed) {
     for (let i = 0; i < attrs.length; i++) {
       const attrName = attrs[i];
       if (validateAttr(attrName) && preChecks[attrName]?.()) {
@@ -82,6 +83,11 @@ function ariaAllowedAttrEvaluate(node, options, virtualNode) {
 
   if (invalid.length) {
     this.data(invalid);
+
+    if (!isHtmlElement(virtualNode) && !role && !isFocusable(virtualNode)) {
+      return undefined;
+    }
+
     return false;
   }
 

diff --git a/lib/checks/aria/aria-allowed-attr.json b/lib/checks/aria/aria-allowed-attr.json
@@ -16,7 +16,8 @@
       "fail": {
         "singular": "ARIA attribute is not allowed: ${data.values}",
         "plural": "ARIA attributes are not allowed: ${data.values}"
-      }
+      },
+      "incomplete": "Check that there is no problem if the ARIA attribute is ignored on this element: ${data.values}"
     }
   }
 }
diff --git a/test/aria-practices/apg.spec.js b/test/aria-practices/apg.spec.js
@@ -6,11 +6,13 @@ const { getWebdriver, connectToChromeDriver } = require('./run-server');
 const { assert } = require('chai');
 const globby = require('globby');
 
-describe('aria-practices', function () {
+describe('aria-practices', function() {
   // Use path.resolve rather than require.resolve because APG has no package.json
   const apgPath = path.resolve(__dirname, '../../node_modules/aria-practices/');
-  const filePaths = globby.sync(`${apgPath}/examples/**/*.html`)
-  const testFiles = filePaths.map(fileName => fileName.split('/aria-practices/examples/')[1])
+  const filePaths = globby.sync(`${apgPath}/examples/**/*.html`);
+  const testFiles = filePaths.map(
+    fileName => fileName.split('/aria-practices/examples/')[1]
+  );
   const port = 9515;
   const addr = `http://localhost:9876/node_modules/aria-practices/`;
   let driver, axeSource;
@@ -36,22 +38,24 @@ describe('aria-practices', function () {
       'color-contrast',
       'heading-order', // w3c/aria-practices#2119
       'list', // w3c/aria-practices#2118
-      'scrollable-region-focusable', // w3c/aria-practices#2114
+      'scrollable-region-focusable' // w3c/aria-practices#2114
     ],
     'feed/feedDisplay.html': ['page-has-heading-one'], // w3c/aria-practices#2120
     // "page within a page" type thing going on
     'menubar/menubar-navigation.html': [
       'aria-allowed-role',
       'landmark-banner-is-top-level',
-      'landmark-contentinfo-is-top-level',
+      'landmark-contentinfo-is-top-level'
     ],
     // "page within a page" type thing going on
     'treeview/treeview-navigation.html': [
       'aria-allowed-role',
       'landmark-banner-is-top-level',
       'landmark-contentinfo-is-top-level'
-    ]
-  }
+    ],
+    //https://github.com/w3c/aria-practices/issues/2199
+    'button/button_idl.html': ['aria-allowed-attr']
+  };
 
   // Not an actual content file
   const skippedPages = [
@@ -60,19 +64,24 @@ describe('aria-practices', function () {
     'toolbar/help.html' // Embedded into another page
   ];
 
-  testFiles.filter(filePath => !skippedPages.includes(filePath)).forEach(filePath => {
-    it(`finds no issue in "${filePath}"`, async () => {
-      await driver.get(`${addr}/examples/${filePath}`);
-
-      const builder = new AxeBuilder(driver, axeSource);
-      builder.disableRules([
-        ...disabledRules['*'],
-        ...(disabledRules[filePath] || []),
-      ]);
-
-      const { violations } = await builder.analyze();
-      const issues = violations.map(({ id, nodes }) => ({ id, issues: nodes.length }))
-      assert.lengthOf(issues, 0);
+  testFiles
+    .filter(filePath => !skippedPages.includes(filePath))
+    .forEach(filePath => {
+      it(`finds no issue in "${filePath}"`, async () => {
+        await driver.get(`${addr}/examples/${filePath}`);
+
+        const builder = new AxeBuilder(driver, axeSource);
+        builder.disableRules([
+          ...disabledRules['*'],
+          ...(disabledRules[filePath] || [])
+        ]);
+
+        const { violations } = await builder.analyze();
+        const issues = violations.map(({ id, nodes }) => ({
+          id,
+          issues: nodes.length
+        }));
+        assert.lengthOf(issues, 0);
+      });
     });
-  });
 });
diff --git a/test/checks/aria/allowed-attr.js b/test/checks/aria/allowed-attr.js
@@ -59,7 +59,7 @@ describe('aria-allowed-attr', function() {
     assert.isNull(checkContext._data);
   });
 
-  it.skip('should return false for non-global attributes if there is no role', function() {
+  it('should return false for non-global attributes if there is no role', function() {
     var vNode = queryFixture(
       '<div id="target" tabindex="1" aria-selected="true" aria-owns="foo"></div>'
     );
@@ -72,21 +72,22 @@ describe('aria-allowed-attr', function() {
     assert.deepEqual(checkContext._data, ['aria-selected="true"']);
   });
 
-  it('should return true for non-global attributes if there is no role', function() {
+  it('should not report on invalid attributes', function() {
     var vNode = queryFixture(
-      '<div id="target" tabindex="1" aria-selected="true" aria-owns="foo"></div>'
+      '<div role="dialog" id="target" tabindex="1" aria-cats="true"></div>'
     );
 
     assert.isTrue(
       axe.testUtils
         .getCheckEvaluate('aria-allowed-attr')
         .call(checkContext, null, null, vNode)
     );
+    assert.isNull(checkContext._data);
   });
 
-  it('should not report on invalid attributes', function() {
+  it('should not report on allowed attributes', function() {
     var vNode = queryFixture(
-      '<div role="dialog" id="target" tabindex="1" aria-cats="true"></div>'
+      '<div role="radio" id="target" tabindex="1" aria-required="true" aria-checked="true"></div>'
     );
 
     assert.isTrue(
@@ -97,18 +98,45 @@ describe('aria-allowed-attr', function() {
     assert.isNull(checkContext._data);
   });
 
-  it('should not report on allowed attributes', function() {
+  it('should return undefined for custom element that has no role and is not focusable', function() {
     var vNode = queryFixture(
-      '<div role="radio" id="target" tabindex="1" aria-required="true" aria-checked="true"></div>'
+      '<my-custom-element id="target" aria-expanded="true"></my-custom-element>'
     );
 
-    assert.isTrue(
+    assert.isUndefined(
       axe.testUtils
         .getCheckEvaluate('aria-allowed-attr')
         .call(checkContext, null, null, vNode)
     );
-    assert.isNull(checkContext._data);
+    assert.isNotNull(checkContext._data);
   });
+
+  it("should return false for custom element that has a role which doesn't allow the attribute", function() {
+    var vNode = queryFixture(
+      '<my-custom-element role="insertion" id="target" aria-expanded="true"></my-custom-element>'
+    );
+
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-allowed-attr')
+        .call(checkContext, null, null, vNode)
+    );
+    assert.isNotNull(checkContext._data);
+  });
+
+  it('should return false for custom element that is focusable', function() {
+    var vNode = queryFixture(
+      '<my-custom-element tabindex="1" id="target" aria-expanded="true"></my-custom-element>'
+    );
+
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-allowed-attr')
+        .call(checkContext, null, null, vNode)
+    );
+    assert.isNotNull(checkContext._data);
+  });
+
   describe('invalid aria-attributes when used on role=row as a descendant of a table or a grid', function() {
     [
       'aria-posinset="1"',

diff --git a/test/integration/rules/aria-allowed-attr/failures.html b/test/integration/rules/aria-allowed-attr/failures.html
@@ -28,12 +28,12 @@
 <kbd aria-label="value" id="fail27"></kbd>
 <abbr aria-label="value" id="fail28"></abbr>
 <custom-elm aria-label="value" id="fail29"></custom-elm>
-<!-- <audio
+<audio
   src="/test/assets/moon-speech.mp3"
   controls
   aria-orientation="horizontal"
   id="fail30"
-></audio> -->
+></audio>
 <div role="mark" aria-label="value" id="fail31">fail</div>
 <div role="mark" aria-labelledby="value" id="fail32">fail</div>
 <div role="suggestion" aria-label="value" id="fail33">fail</div>

diff --git a/test/integration/rules/aria-allowed-attr/failures.json b/test/integration/rules/aria-allowed-attr/failures.json
@@ -31,6 +31,7 @@
     ["#fail27"],
     ["#fail28"],
     ["#fail29"],
+    ["#fail30"],
     ["#fail31"],
     ["#fail32"],
     ["#fail33"],

diff --git a/test/integration/rules/aria-allowed-attr/incomplete.html b/test/integration/rules/aria-allowed-attr/incomplete.html
@@ -1,2 +1,3 @@
 <div id="incomplete0" aria-label="foo">Foo</div>
 <div id="incomplete1" aria-labelledby="missing">Foo</div>
+<my-custom-elm id="incomplete2" aria-expanded="true">Foo</my-custom-elm>
diff --git a/test/integration/rules/aria-allowed-attr/incomplete.json b/test/integration/rules/aria-allowed-attr/incomplete.json
@@ -1,5 +1,5 @@
 {
   "description": "aria-allowed-attr incomplete tests",
   "rule": "aria-allowed-attr",
-  "incomplete": [["#incomplete0"], ["#incomplete1"]]
+  "incomplete": [["#incomplete0"], ["#incomplete1"], ["#incomplete2"]]
 }
diff --git a/test/integration/virtual-rules/aria-allowed-attr.js b/test/integration/virtual-rules/aria-allowed-attr.js
@@ -55,7 +55,7 @@ describe('aria-allowed-attr virtual-rule', function() {
     assert.lengthOf(results.incomplete, 0);
   });
 
-  it.skip('should fail for non-global attributes and element with no role', function() {
+  it('should fail for non-global attributes and element with no role', function() {
     var results = axe.runVirtualRule('aria-allowed-attr', {
       nodeName: 'div',
       attributes: {
@@ -119,4 +119,17 @@ describe('aria-allowed-attr virtual-rule', function() {
     assert.lengthOf(results.violations, 1);
     assert.lengthOf(results.incomplete, 0);
   });
+
+  it('should incomplete for non-global attributes and custom element', function() {
+    var results = axe.runVirtualRule('aria-allowed-attr', {
+      nodeName: 'custom-elm1',
+      attributes: {
+        'aria-checked': true
+      }
+    });
+
+    assert.lengthOf(results.passes, 0);
+    assert.lengthOf(results.violations, 0);
+    assert.lengthOf(results.incomplete, 1);
+  });
 });