fix(aria-required-children): allow group and rowgroup roles (#2661)

* fix(aria-required-children): allow group and rowgroup roles

* fix

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

@@ -9,17 +9,22 @@ import { hasContentVirtual, idrefs } from '../../commons/dom';
 /**
  * Get all owned roles of an element
  */
-function getOwnedRoles(virtualNode) {
+function getOwnedRoles(virtualNode, required) {
   const ownedRoles = [];
   const ownedElements = getOwnedVirtual(virtualNode);
   for (let i = 0; i < ownedElements.length; i++) {
     let ownedElement = ownedElements[i];
-    let role = getRole(ownedElement);
+    let role = getRole(ownedElement, { noPresentational: true });
 
-    // if owned node has no role or is presentational we keep
-    // parsing the descendant tree. this means intermediate roles
-    // between a required parent and child will fail the check
-    if (['presentation', 'none', null].includes(role)) {
+    // if owned node has no role or is presentational, or if role
+    // allows group or rowgroup, we keep parsing the descendant tree.
+    // this means intermediate roles between a required parent and
+    // child will fail the check
+    if (
+      !role ||
+      (['group', 'rowgroup'].includes(role) &&
+        required.some(requiredRole => requiredRole === role))
+    ) {
       ownedElements.push(...ownedElement.children);
     } else if (role) {
       ownedRoles.push(role);
@@ -99,11 +104,11 @@ function ariaRequiredChildrenEvaluate(node, options, virtualNode) {
     options && Array.isArray(options.reviewEmpty) ? options.reviewEmpty : [];
   const role = getExplicitRole(virtualNode, { dpub: true });
   const required = requiredOwned(role);
-  if (!required) {
+  if (required === null) {
     return true;
   }
 
-  const ownedRoles = getOwnedRoles(virtualNode);
+  const ownedRoles = getOwnedRoles(virtualNode, required);
   const missing = missingRequiredChildren(
     virtualNode,
     role,

lib/standards/aria-roles.js

@@ -269,7 +269,7 @@ const ariaRoles = {
   },
   list: {
     type: 'structure',
-    requiredOwned: ['listitem'],
+    requiredOwned: ['group', 'listitem'],
     allowedAttrs: ['aria-expanded'],
     superclassRole: ['section']
   },
@@ -322,7 +322,7 @@ const ariaRoles = {
   },
   menu: {
     type: 'composite',
-    requiredOwned: ['menuitemradio', 'menuitem', 'menuitemcheckbox'],
+    requiredOwned: ['group', 'menuitemradio', 'menuitem', 'menuitemcheckbox'],
     allowedAttrs: [
       'aria-activedescendant',
       'aria-expanded',
@@ -332,7 +332,7 @@ const ariaRoles = {
   },
   menubar: {
     type: 'composite',
-    requiredOwned: ['menuitemradio', 'menuitem', 'menuitemcheckbox'],
+    requiredOwned: ['group', 'menuitemradio', 'menuitem', 'menuitemcheckbox'],
     allowedAttrs: [
       'aria-activedescendant',
       'aria-expanded',
@@ -744,7 +744,7 @@ const ariaRoles = {
   },
   tree: {
     type: 'composite',
-    requiredOwned: ['treeitem'],
+    requiredOwned: ['group', 'treeitem'],
     allowedAttrs: [
       'aria-multiselectable',
       'aria-required',

test/checks/aria/required-children.js

@@ -22,7 +22,7 @@ describe('aria-required-children', function() {
         .getCheckEvaluate('aria-required-children')
         .apply(checkContext, params)
     );
-    assert.deepEqual(checkContext._data, ['listitem']);
+    assert.deepEqual(checkContext._data, ['group', 'listitem']);
   });
 
   (shadowSupported ? it : xit)(
@@ -43,7 +43,7 @@ describe('aria-required-children', function() {
           .getCheckEvaluate('aria-required-children')
           .apply(checkContext, params)
       );
-      assert.deepEqual(checkContext._data, ['listitem']);
+      assert.deepEqual(checkContext._data, ['group', 'listitem']);
     }
   );
 
@@ -309,7 +309,7 @@ describe('aria-required-children', function() {
         .apply(checkContext, params)
     );
 
-    assert.deepEqual(checkContext._data, ['listitem']);
+    assert.deepEqual(checkContext._data, ['group', 'listitem']);
   });
 
   it('should fail when list has intermediate child with role that is not a required role', function() {
@@ -322,7 +322,7 @@ describe('aria-required-children', function() {
         .apply(checkContext, params)
     );
 
-    assert.deepEqual(checkContext._data, ['listitem']);
+    assert.deepEqual(checkContext._data, ['group', 'listitem']);
   });
 
   it('should fail when nested child with role row  does not have required child role cell', function() {
@@ -492,6 +492,72 @@ describe('aria-required-children', function() {
     );
   });
 
+  it('should pass when role allows group and group has required child', function() {
+    var params = checkSetup(
+      '<div role="menu" id="target"><ul role="group"><li role="menuitem">Menuitem</li></ul></div>'
+    );
+    assert.isTrue(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
+  it('should fail when role allows group and group does not have required child', function() {
+    var params = checkSetup(
+      '<div role="menu" id="target"><ul role="group"><li>Menuitem</li></ul></div>'
+    );
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
+  it('should fail when role does not allow group', function() {
+    var params = checkSetup(
+      '<div role="listbox" id="target"><ul role="group"><li role="option">Option</li></ul></div>'
+    );
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
+  it('should pass when role allows rowgroup and rowgroup has required child', function() {
+    var params = checkSetup(
+      '<div role="table" id="target"><ul role="rowgroup"><li role="row">Row</li></ul></div>'
+    );
+    assert.isTrue(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
+  it('should fail when role allows rowgroup and rowgroup does not have required child', function() {
+    var params = checkSetup(
+      '<div role="table" id="target"><ul role="rowgroup"><li>Row</li></ul></div>'
+    );
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
+  it('should fail when role does not allow rowgroup', function() {
+    var params = checkSetup(
+      '<div role="listbox" id="target"><ul role="rowgroup"><li role="option">Option</li></ul></div>'
+    );
+    assert.isFalse(
+      axe.testUtils
+        .getCheckEvaluate('aria-required-children')
+        .apply(checkContext, params)
+    );
+  });
+
   describe('options', function() {
     it('should return undefined instead of false when the role is in options.reviewEmpty', function() {
       var params = checkSetup('<div role="grid" id="target"></div>', {

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

@@ -47,3 +47,12 @@
     </div>
   </div>
 </div>
+
+<div role="menu" id="pass17">
+  <ul role="group" id="pass18">
+    >
+    <li role="menuitem" id="pass19">>Inbox</li>
+    <li role="menuitem" id="pass20">>Archive</li>
+    <li role="menuitem" id="pass21">>Trash</li>
+  </ul>
+</div>

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

@@ -26,7 +26,12 @@
     ["#pass13"],
     ["#pass14"],
     ["#pass15"],
-    ["#pass16"]
+    ["#pass16"],
+    ["#pass17"],
+    ["#pass18"],
+    ["#pass19"],
+    ["#pass20"],
+    ["#pass21"]
   ],
   "incomplete": [
     ["#incomplete1"],