fix(aria-allowed-attr): error when generic elements use aria-label an…

…d aria-labelledy (#2766) * fix(aria-allowed-attr): error when generic elements use aria-label and aria-labelledy * fix * fix * remove generic role * fixes * Apply suggestions from code review Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> * ci: fix tests Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
dequelabs · Mar 5, 2021 · 64379e1 · 64379e1
1 parent 4a17ed0
commit 64379e1
Show file tree

Hide file tree

Showing 12 changed files with 99 additions and 102 deletions.
diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md
@@ -13,7 +13,7 @@
 | Rule ID                                                                                                                          | Description                                                                                                   | Impact            | Tags                                                                                       | Issue Type                 |
 | :------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------ | :---------------- | :----------------------------------------------------------------------------------------- | :------------------------- |
 | [area-alt](https://dequeuniversity.com/rules/axe/4.1/area-alt?application=RuleDescription)                                       | Ensures &lt;area&gt; elements of image maps have alternate text                                               | Critical          | cat.text-alternatives, wcag2a, wcag111, wcag244, wcag412, section508, section508.22.a, ACT | failure, needs&nbsp;review |
-| [aria-allowed-attr](https://dequeuniversity.com/rules/axe/4.1/aria-allowed-attr?application=RuleDescription)                     | Ensures ARIA attributes are allowed for an element&apos;s role                                                | Critical          | cat.aria, wcag2a, wcag412                                                                  | failure                    |
+| [aria-allowed-attr](https://dequeuniversity.com/rules/axe/4.1/aria-allowed-attr?application=RuleDescription)                     | Ensures ARIA attributes are allowed for an element&apos;s role                                                | Critical          | cat.aria, wcag2a, wcag412                                                                  | failure, needs&nbsp;review |
 | [aria-command-name](https://dequeuniversity.com/rules/axe/4.1/aria-command-name?application=RuleDescription)                     | Ensures every ARIA button, link and menuitem has an accessible name                                           | Serious           | cat.aria, wcag2a, wcag412                                                                  | failure, needs&nbsp;review |
 | [aria-hidden-body](https://dequeuniversity.com/rules/axe/4.1/aria-hidden-body?application=RuleDescription)                       | Ensures aria-hidden=&apos;true&apos; is not present on the document body.                                     | Critical          | cat.aria, wcag2a, wcag412                                                                  | failure                    |
 | [aria-hidden-focus](https://dequeuniversity.com/rules/axe/4.1/aria-hidden-focus?application=RuleDescription)                     | Ensures aria-hidden elements do not contain focusable elements                                                | Serious           | cat.name-role-value, wcag2a, wcag412, wcag131                                              | failure, needs&nbsp;review |

diff --git a/lib/checks/aria/aria-prohibited-attr-evaluate.js b/lib/checks/aria/aria-prohibited-attr-evaluate.js
@@ -1,4 +1,5 @@
 import { getRole } from '../../commons/aria';
+import { sanitize, subtreeText } from '../../commons/text';
 import standards from '../../standards';
 
 /**
@@ -23,32 +24,40 @@ import standards from '../../standards';
  * </table>
  *
  * @memberof checks
- * @return {Boolean} True if the element does not use any prohibited ARIA attributes. False otherwise.
+ * @return {Boolean} True if the element uses any prohibited ARIA attributes. False otherwise.
  */
 function ariaProhibitedAttrEvaluate(node, options, virtualNode) {
   const prohibited = [];
-  const role = getRole(virtualNode);
-
-  if (!role) {
-    return false;
-  }
 
+  const role = getRole(virtualNode);
   const attrs = virtualNode.attrNames;
-  const prohibitedAttrs = standards.ariaRoles[role].prohibitedAttrs;
+  const prohibitedAttrs = role
+    ? standards.ariaRoles[role].prohibitedAttrs
+    : ['aria-label', 'aria-labelledby'];
 
   if (!prohibitedAttrs) {
     return false;
   }
 
   for (let i = 0; i < attrs.length; i++) {
     const attrName = attrs[i];
-    if (prohibitedAttrs.includes(attrName)) {
+    const attrValue = sanitize(virtualNode.attr(attrName));
+    if (prohibitedAttrs.includes(attrName) && attrValue !== '') {
       prohibited.push(attrName);
     }
   }
 
   if (prohibited.length) {
     this.data(prohibited);
+
+    // aria-label and aria-labelledy are not allowed on elements
+    // without roles, but if the element has text content then
+    // we will return undefined as a screen reader will read
+    // the text content
+    if (!role && sanitize(subtreeText(virtualNode)) !== '') {
+      return undefined;
+    }
+
     return true;
   }
 

diff --git a/lib/checks/aria/aria-prohibited-attr.json b/lib/checks/aria/aria-prohibited-attr.json
@@ -5,7 +5,8 @@
     "impact": "critical",
     "messages": {
       "pass": "ARIA attribute is allowed",
-      "fail": "ARIA attribute cannot be used: ${data.values}"
+      "fail": "ARIA attribute cannot be used: ${data.values}",
+      "incomplete": "ARIA attribute is not well supported on the element and the text content will be used instead: ${data.values}"
     }
   }
 }
diff --git a/test/checks/aria/aria-prohibited-attr.js b/test/checks/aria/aria-prohibited-attr.js
@@ -30,16 +30,37 @@ describe('aria-prohibited-attr', function() {
     ]);
   });
 
+  it('should return undefined if element has no role and has text content', function() {
+    var params = checkSetup(
+      '<div id="target" aria-label="foo" aria-labelledby="foo">Contents</div>'
+    );
+    assert.isUndefined(checkEvaluate.apply(checkContext, params));
+  });
+
+  it('should return true if element has no role and no text content', function() {
+    var params = checkSetup(
+      '<div id="target" aria-label="foo" aria-labelledby="foo"></div>'
+    );
+    assert.isTrue(checkEvaluate.apply(checkContext, params));
+  });
+
   it('should return false if all attributes are allowed', function() {
     var params = checkSetup(
       '<div id="target" role="button" aria-label="foo" aria-labelledby="foo">Contents</div>'
     );
     assert.isFalse(checkEvaluate.apply(checkContext, params));
   });
 
-  it('should return false if element has no role', function() {
+  it('should return false if no prohibited attributes are used', function() {
     var params = checkSetup(
-      '<div id="target" aria-label="foo" aria-labelledby="foo">Contents</div>'
+      '<div id="target" role="code" aria-selected="true">Contents</div>'
+    );
+    assert.isFalse(checkEvaluate.apply(checkContext, params));
+  });
+
+  it('should return false if prohibited attributes have no value', function() {
+    var params = checkSetup(
+      '<div id="target" role="code" aria-label="  " aria-labelledby="  ">Contents</div>'
     );
     assert.isFalse(checkEvaluate.apply(checkContext, params));
   });

diff --git a/test/commons/standards/get-aria-roles-by-type.js b/test/commons/standards/get-aria-roles-by-type.js
@@ -1,59 +1,32 @@
 describe('standards.getAriaRolesByType', function() {
   var getAriaRolesByType = axe.commons.standards.getAriaRolesByType;
 
-  before(function() {
-    axe._load({});
-  });
+  it('should return a list of role names by type', function() {
+    // first remove all role types
+    var roleNames = Object.keys(axe._audit.standards.ariaRoles);
+    var ariaRoles = {};
+    for (var i = 0; i < roleNames.length; i++) {
+      ariaRoles[roleNames[i]] = { type: 'off' };
+    }
 
-  after(function() {
-    axe.reset();
-  });
+    // then turn on a few specific roles
+    ariaRoles.article = { type: 'structure' };
+    ariaRoles.blockquote = { type: 'structure' };
+    ariaRoles.caption = { type: 'structure' };
+    ariaRoles.cell = { type: 'structure' };
+
+    axe.configure({
+      standards: {
+        ariaRoles: ariaRoles
+      }
+    });
 
-  it('should return a list of role names by type', function() {
-    // Source: https://www.w3.org/TR/wai-aria-1.1/#document_structure_roles
     var structureRoles = getAriaRolesByType('structure');
     assert.deepEqual(structureRoles, [
       'article',
       'blockquote',
       'caption',
-      'cell',
-      'code',
-      'columnheader',
-      'definition',
-      'deletion',
-      'directory',
-      'document',
-      'emphasis',
-      'feed',
-      'figure',
-      'group',
-      'heading',
-      'img',
-      'insertion',
-      'list',
-      'listitem',
-      'math',
-      'meter',
-      'none',
-      'note',
-      'paragraph',
-      'presentation',
-      'row',
-      'rowgroup',
-      'rowheader',
-      'separator',
-      'strong',
-      'subscript',
-      'superscript',
-      'table',
-      'term',
-      'text',
-      'time',
-      'toolbar',
-      'tooltip',
-      'graphics-document',
-      'graphics-object',
-      'graphics-symbol'
+      'cell'
     ]);
   });
 

diff --git a/test/commons/standards/get-aria-roles-supporting-name-from-content.js b/test/commons/standards/get-aria-roles-supporting-name-from-content.js
@@ -2,55 +2,32 @@ describe('standards.getAriaRolesSupportingNameFromContent', function() {
   var getAriaRolesSupportingNameFromContent =
     axe.commons.standards.getAriaRolesSupportingNameFromContent;
 
-  before(function() {
-    axe._load({});
-  });
+  it('should return a list of role names which are named from content', function() {
+    // first remove all namedFromContent
+    var roleNames = Object.keys(axe._audit.standards.ariaRoles);
+    var ariaRoles = {};
+    for (var i = 0; i < roleNames.length; i++) {
+      ariaRoles[roleNames[i]] = { nameFromContent: false };
+    }
 
-  after(function() {
-    axe.reset();
-  });
+    // then turn on a few specific roles
+    ariaRoles.button = { nameFromContent: true };
+    ariaRoles.cell = { nameFromContent: true };
+    ariaRoles.checkbox = { nameFromContent: true };
+    ariaRoles.columnheader = { nameFromContent: true };
+
+    axe.configure({
+      standards: {
+        ariaRoles: ariaRoles
+      }
+    });
 
-  it('should return a list of role names which are named from content', function() {
-    // Source: https://www.w3.org/TR/wai-aria-1.1/#namefromcontent
-    // Source: https://www.w3.org/TR/dpub-aria-1.0/
-    // Note: we have added roles in our spec. also note that
-    // although "tree" is listed as supporting name from content
-    // it's role definition does not list contents in the name from
-    // section (it was removed from the list in WAI ARIA 1.2)
     var contentRoles = getAriaRolesSupportingNameFromContent();
     assert.deepEqual(contentRoles, [
       'button',
       'cell',
       'checkbox',
-      'columnheader',
-      'directory',
-      'figure',
-      'gridcell',
-      'heading',
-      'link',
-      'listitem',
-      'menuitem',
-      'menuitemcheckbox',
-      'menuitemradio',
-      'option',
-      'radio',
-      'row',
-      'rowgroup',
-      'rowheader',
-      'section',
-      'sectionhead',
-      'switch',
-      'tab',
-      'table',
-      'term',
-      'text',
-      'tooltip',
-      'treeitem',
-      'doc-backlink',
-      'doc-biblioref',
-      'doc-glossref',
-      'doc-noteref',
-      'graphics-object'
+      'columnheader'
     ]);
   });
 

diff --git a/test/integration/rules/aria-allowed-attr/failures.html b/test/integration/rules/aria-allowed-attr/failures.html
@@ -20,4 +20,6 @@
 <div role="subscript" aria-labelledby="value" id="fail20">fail</div>
 <div role="superscript" aria-label="value" id="fail21">fail</div>
 <div role="superscript" aria-labelledby="value" id="fail22">fail</div>
+<div aria-label="value" id="fail23"></div>
+<div aria-labelledby="value" id="fail24"></div>
 <!-- technically presentation and none roles do not allow aria-label and aria-labelledby, but since those are global attributes the presentation role conflict will not resolve the roles to none or presentation -->
diff --git a/test/integration/rules/aria-allowed-attr/failures.json b/test/integration/rules/aria-allowed-attr/failures.json
@@ -23,6 +23,8 @@
     ["#fail19"],
     ["#fail20"],
     ["#fail21"],
-    ["#fail22"]
+    ["#fail22"],
+    ["#fail23"],
+    ["#fail24"]
   ]
 }
diff --git a/test/integration/rules/aria-allowed-attr/incomplete.html b/test/integration/rules/aria-allowed-attr/incomplete.html
@@ -0,0 +1,2 @@
+<div id="incomplete0" aria-label="foo">Foo</div>
+<div id="incomplete1" aria-labelledby="missing">Foo</div>
diff --git a/test/integration/rules/aria-allowed-attr/incomplete.json b/test/integration/rules/aria-allowed-attr/incomplete.json
@@ -0,0 +1,5 @@
+{
+  "description": "aria-allowed-attr incomplete tests",
+  "rule": "aria-allowed-attr",
+  "incomplete": [["#incomplete0"], ["#incomplete1"]]
+}
diff --git a/test/integration/rules/aria-allowed-attr/passes.html b/test/integration/rules/aria-allowed-attr/passes.html
@@ -1905,3 +1905,6 @@
     target 1
   </td>
 </table>
+
+<div id="pass79" aria-label="  ">Foo</div>
+<div id="pass80" aria-labelledby="  ">Foo</div>
diff --git a/test/integration/rules/aria-allowed-attr/passes.json b/test/integration/rules/aria-allowed-attr/passes.json
@@ -80,6 +80,8 @@
     ["#pass75"],
     ["#pass76"],
     ["#pass77"],
-    ["#pass78"]
+    ["#pass78"],
+    ["#pass79"],
+    ["#pass80"]
   ]
 }