fix(required-parent): Allow *item > group > *item nesting (#2898)

* fix(required-parent): Allow *item > group > *item nesting

* chore(required-parent): Exclude menuitem from exception

* chore(required-parent): Add ownGroupRoles option

doc/check-options.md

@@ -7,6 +7,7 @@
   - [Global Options](#global-options)
   - [aria-allowed-role](#aria-allowed-role)
   - [aria-required-children](#aria-required-children)
+  - [aria-required-parent](#aria-required-parent)
   - [aria-roledescription](#aria-roledescription)
   - [color-contrast](#color-contrast)
   - [page-has-heading-one](#page-has-heading-one)
@@ -107,6 +108,29 @@ All checks allow these global options:
   </tbody>
 </table>
 
+### aria-required-parent
+
+<table>
+  <thead>
+    <tr>
+      <th>Option</th>
+      <th align="left">Default</th>
+      <th align="left">Description</th>
+    </tr>
+  </thead>
+  <tbody>
+    <tr>
+      <td>
+        <code>ownGroupRoles</code>
+      </td>
+      <td align="left">
+        <pre lang=js><code>['listitem', 'treeitem']</code></pre>
+        </td>
+      <td align="left">List of ARIA roles that when used in a group can have a grand parent with the same role. E.g. <code>list > listitem > group > listitem</code>.</td>
+    </tr>
+  </tbody>
+</table>
+
 ### aria-roledescription
 
 <table>

lib/checks/aria/aria-required-parent-evaluate.js

@@ -2,7 +2,7 @@ import { getExplicitRole, getRole, requiredContext } from '../../commons/aria';
 import { getRootNode } from '../../commons/dom';
 import { getNodeFromTree, escapeSelector } from '../../core/utils';
 
-function getMissingContext(virtualNode, reqContext, includeElement) {
+function getMissingContext(virtualNode, ownGroupRoles, reqContext, includeElement) {
   const explicitRole = getExplicitRole(virtualNode);
 
   if (!reqContext) {
@@ -20,6 +20,10 @@ function getMissingContext(virtualNode, reqContext, includeElement) {
     // if parent node has role=group and role=group is an allowed
     // context, check next parent
     if (reqContext.includes('group') && parentRole === 'group') {
+      // Allow the own role; i.e. tree > treeitem > group > treeitem
+      if (ownGroupRoles.includes(explicitRole)) {
+        reqContext.push(explicitRole);
+      }
       vNode = vNode.parent;
       continue;
     }
@@ -82,18 +86,24 @@ function getAriaOwners(element) {
  * @return {Boolean} True if the element has a parent with a required role. False otherwise.
  */
 function ariaRequiredParentEvaluate(node, options, virtualNode) {
-  var missingParents = getMissingContext(virtualNode);
+  const ownGroupRoles = (
+    options && Array.isArray(options.ownGroupRoles) 
+    ? options.ownGroupRoles 
+    : []
+  );
+  let missingParents = getMissingContext(virtualNode, ownGroupRoles);
 
   if (!missingParents) {
     return true;
   }
 
-  var owners = getAriaOwners(node);
+  const owners = getAriaOwners(node);
 
   if (owners) {
-    for (var i = 0, l = owners.length; i < l; i++) {
+    for (let i = 0, l = owners.length; i < l; i++) {
       missingParents = getMissingContext(
         getNodeFromTree(owners[i]),
+        ownGroupRoles,
         missingParents,
         true
       );

lib/checks/aria/aria-required-parent.json

@@ -1,6 +1,9 @@
 {
   "id": "aria-required-parent",
   "evaluate": "aria-required-parent-evaluate",
+  "options": {
+    "ownGroupRoles": ["listitem", "treeitem"]
+  },
   "metadata": {
     "impact": "critical",
     "messages": {

lib/standards/aria-roles.js

@@ -297,7 +297,7 @@ const ariaRoles = {
   },
   listitem: {
     type: 'structure',
-    requiredContext: ['list'],
+    requiredContext: ['list', 'group'],
     allowedAttrs: [
       'aria-level',
       'aria-posinset',

test/checks/aria/required-parent.js

@@ -21,7 +21,7 @@ describe('aria-required-parent', function() {
         .getCheckEvaluate('aria-required-parent')
         .apply(checkContext, params)
     );
-    assert.deepEqual(checkContext._data, ['list']);
+    assert.deepEqual(checkContext._data, ['list', 'group']);
   });
 
   (shadowSupported ? it : xit)(
@@ -44,7 +44,7 @@ describe('aria-required-parent', function() {
           .getCheckEvaluate('aria-required-parent')
           .apply(checkContext, params)
       );
-      assert.deepEqual(checkContext._data, ['list']);
+      assert.deepEqual(checkContext._data, ['list', 'group']);
     }
   );
 
@@ -70,7 +70,7 @@ describe('aria-required-parent', function() {
         .getCheckEvaluate('aria-required-parent')
         .apply(checkContext, params)
     );
-    assert.deepEqual(checkContext._data, ['list']);
+    assert.deepEqual(checkContext._data, ['list', 'group']);
   });
 
   it('should pass when required parent is present in an aria-owns context', function() {
@@ -152,7 +152,7 @@ describe('aria-required-parent', function() {
 
   it('should fail when intermediate node is role=group but this not an allowed context', function() {
     var params = checkSetup(
-      '<div role="list"><div role="group"><p role="listitem" id="target">Nothing here.</p></div></div>'
+      '<div role="menu"><div role="group"><p role="listitem" id="target">Nothing here.</p></div></div>'
     );
     assert.isFalse(
       axe.testUtils
@@ -161,6 +161,62 @@ describe('aria-required-parent', function() {
     );
   });
 
+  describe('group with ownGroupRoles', function () {
+    it('should pass when the role and grand parent role is in ownGroupRoles', function() {
+      var params = checkSetup(
+        '<div role="list">' +
+          '<div role="listitem">' +
+          '<div role="group">' +
+          '<div role="listitem" id="target">' +
+          '</div></div></div></div>', {
+            ownGroupRoles: ['listitem']
+          }
+      );
+
+      assert.isTrue(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-parent')
+          .apply(checkContext, params)
+      );
+    });
+
+    it('should fail when the role and grand parent role is in ownGroupRoles', function() {
+      var params = checkSetup(
+        '<div role="menu">' +
+          '<div role="menuitem">' +
+          '<div role="group">' +
+          '<div role="menuitem" id="target">' +
+          '</div></div></div></div>', {
+            ownGroupRoles: ['listitem']
+          }
+      );
+
+      assert.isFalse(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-parent')
+          .apply(checkContext, params)
+      );
+    });
+
+    it('should fail when the role is not in a group', function () {
+      var params = checkSetup(
+        '<div role="list">' +
+          '<div role="listitem">' +
+          '<div role="none">' +
+          '<div role="listitem" id="target">' +
+          '</div></div></div></div>', {
+            ownGroupRoles: ['listitem']
+          }
+      );
+
+      assert.isFalse(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-parent')
+          .apply(checkContext, params)
+      );
+    })
+  });
+
   it('should pass when intermediate node is role=none', function() {
     var params = checkSetup(
       '<div role="list"><div role="none"><p role="listitem" id="target">Nothing here.</p></div></div>'

test/integration/rules/aria-required-parent/aria-required-parent.html

@@ -38,3 +38,23 @@
     <div role="rowgroup" id="pass9">Item 1</div>
   </div>
 </div>
+
+<div role="tree">
+  <div role="treeitem" id="pass10">
+    <div role="group">
+      <div role="treeitem" id="pass11">tree item</div>
+    </div>
+  </div>
+</div>
+
+<div role="group">
+  <div role="treeitem" id="fail6">tree item</div>
+</div>
+
+<div role="menu">
+  <div role="menuitem" id="pass12">
+    <div role="group">
+      <div role="menuitem" id="fail7">menu item</div>
+    </div>
+  </div>
+</div>

test/integration/rules/aria-required-parent/aria-required-parent.json

@@ -1,7 +1,15 @@
 {
   "description": "aria-required-parent test",
   "rule": "aria-required-parent",
-  "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"], ["#fail5"]],
+  "violations": [
+    ["#fail1"],
+    ["#fail2"],
+    ["#fail3"],
+    ["#fail4"],
+    ["#fail5"],
+    ["#fail6"],
+    ["#fail7"]
+  ],
   "passes": [
     ["#pass1"],
     ["#pass2"],
@@ -11,6 +19,9 @@
     ["#pass6"],
     ["#pass7"],
     ["#pass8"],
-    ["#pass9"]
+    ["#pass9"],
+    ["#pass10"],
+    ["#pass11"],
+    ["#pass12"]
   ]
 }

test/integration/rules/preprocessor.js

@@ -20,7 +20,11 @@ var createIntegrationPreprocessor = function(logger) {
       // and add the test data to it
       var htmlpath = file.originalPath.replace(extRegex, '.html');
       var html = fs.readFileSync(htmlpath, 'utf-8');
-      var test = JSON.parse(content);
+      try {
+        var test = JSON.parse(content);
+      } catch (e) {
+        throw new Error('Unable to parse content of ' + file.originalPath)
+      }
       test.content = html;
 
       var result = template.replace('{}; /*tests*/', JSON.stringify(test));