fix(aria-required-childen): test visibility of grandchildren

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

@@ -34,32 +34,31 @@ export default function ariaRequiredChildrenEvaluate(
     return true;
   }
 
-  const { ownedRoles, ownedElements } = getOwnedRoles(virtualNode, required);
-  const unallowed = ownedRoles.filter(({ role }) => !required.includes(role));
+  const ownedRoles = getOwnedRoles(virtualNode, required);
+  const unallowed = ownedRoles.filter(
+    ({ role, vNode }) => vNode.props.nodeType === 1 && !required.includes(role)
+  );
 
   if (unallowed.length) {
-    this.relatedNodes(unallowed.map(({ ownedElement }) => ownedElement));
+    this.relatedNodes(unallowed.map(({ vNode }) => vNode));
     this.data({
       messageKey: 'unallowed',
       values: unallowed
-        .map(({ ownedElement, attr }) =>
-          getUnallowedSelector(ownedElement, attr)
-        )
+        .map(({ vNode, attr }) => getUnallowedSelector(vNode, attr))
         .filter((selector, index, array) => array.indexOf(selector) === index)
         .join(', ')
     });
     return false;
   }
 
-  const missing = missingRequiredChildren(virtualNode, required, ownedRoles);
-  if (!missing) {
+  if (hasRequiredChildren(required, ownedRoles)) {
     return true;
   }
 
-  this.data(missing);
+  this.data(required);
 
   // Only review empty nodes when a node is both empty and does not have an aria-owns relationship
-  if (reviewEmpty.includes(explicitRole) && !ownedElements.some(isContent)) {
+  if (reviewEmpty.includes(explicitRole) && !ownedRoles.some(isContent)) {
     return undefined;
   }
 
@@ -70,22 +69,20 @@ export default function ariaRequiredChildrenEvaluate(
  * Get all owned roles of an element
  */
 function getOwnedRoles(virtualNode, required) {
+  let vNode;
   const ownedRoles = [];
-  const ownedElements = getOwnedVirtual(virtualNode).filter(vNode => {
-    return vNode.props.nodeType !== 1 || isVisibleToScreenReaders(vNode);
-  });
-
-  for (let i = 0; i < ownedElements.length; i++) {
-    const ownedElement = ownedElements[i];
-    if (ownedElement.props.nodeType !== 1) {
+  const ownedVirtual = getOwnedVirtual(virtualNode);
+  while ((vNode = ownedVirtual.shift())) {
+    if (vNode.props.nodeType === 3) {
+      ownedRoles.push({ vNode, role: null });
+    }
+    if (vNode.props.nodeType !== 1 || !isVisibleToScreenReaders(vNode)) {
       continue;
     }
 
-    const role = getRole(ownedElement, { noPresentational: true });
-
-    const globalAriaAttr = getGlobalAriaAttr(ownedElement);
-    const hasGlobalAriaOrFocusable =
-      !!globalAriaAttr || isFocusable(ownedElement);
+    const role = getRole(vNode, { noPresentational: true });
+    const globalAriaAttr = getGlobalAriaAttr(vNode);
+    const hasGlobalAriaOrFocusable = !!globalAriaAttr || isFocusable(vNode);
 
     // if owned node has no role or is presentational, or if role
     // allows group or rowgroup, we keep parsing the descendant tree.
@@ -96,37 +93,21 @@ function getOwnedRoles(virtualNode, required) {
       (['group', 'rowgroup'].includes(role) &&
         required.some(requiredRole => requiredRole === role))
     ) {
-      ownedElements.push(...ownedElement.children);
+      ownedVirtual.push(...vNode.children);
     } else if (role || hasGlobalAriaOrFocusable) {
-      ownedRoles.push({
-        role,
-        attr: globalAriaAttr || 'tabindex',
-        ownedElement
-      });
+      const attr = globalAriaAttr || 'tabindex';
+      ownedRoles.push({ role, attr, vNode });
     }
   }
 
-  return { ownedRoles, ownedElements };
+  return ownedRoles;
 }
 
 /**
- * Get missing children roles
+ * See if any required roles are in the ownedRoles array
  */
-function missingRequiredChildren(virtualNode, required, ownedRoles) {
-  for (let i = 0; i < ownedRoles.length; i++) {
-    const { role } = ownedRoles[i];
-
-    if (required.includes(role)) {
-      required = required.filter(requiredRole => requiredRole !== role);
-      return null;
-    }
-  }
-
-  if (required.length) {
-    return required;
-  }
-
-  return null;
+function hasRequiredChildren(required, ownedRoles) {
+  return ownedRoles.some(({ role }) => role && required.includes(role));
 }
 
 /**
@@ -146,7 +127,6 @@ function getGlobalAriaAttr(vNode) {
  */
 function getUnallowedSelector(vNode, attr) {
   const { nodeName, nodeType } = vNode.props;
-
   if (nodeType === 3) {
     return `#text`;
   }
@@ -155,20 +135,19 @@ function getUnallowedSelector(vNode, attr) {
   if (role) {
     return `[role=${role}]`;
   }
-
   if (attr) {
     return nodeName + `[${attr}]`;
   }
-
   return nodeName;
 }
 
 /**
  * Check if the node has content, or is itself content
- * @param {VirtualNode} vNode
+ * @Object {Object} OwnedRole
+ * @property {VirtualNode} vNode
  * @returns {Boolean}
  */
-function isContent(vNode) {
+function isContent({ vNode }) {
   if (vNode.props.nodeType === 3) {
     return vNode.props.nodeValue.trim().length > 0;
   }

test/checks/aria/required-children.js

@@ -69,7 +69,12 @@ describe('aria-required-children', () => {
 
   it('should pass all existing required children when all required', () => {
     const params = checkSetup(
-      '<div id="target" role="menu"><li role="none"></li><li role="menuitem">Item 1</li><div role="menuitemradio">Item 2</div><div role="menuitemcheckbox">Item 3</div></div>'
+      `<div id="target" role="menu">
+        <li role="none"></li>
+        <li role="menuitem">Item 1</li>
+        <div role="menuitemradio">Item 2</div>
+        <div role="menuitemcheckbox">Item 3</div>
+      </div>`
     );
     assert.isTrue(requiredChildrenCheck.apply(checkContext, params));
   });
@@ -293,6 +298,20 @@ describe('aria-required-children', () => {
     assert.isTrue(requiredChildrenCheck.apply(checkContext, params));
   });
 
+  it('should ignore hidden children inside the group', () => {
+    const params = checkSetup(`
+      <div role="menu" id="target">
+        <ul role="group">
+          <li style="display: none">hidden</li>
+          <li aria-hidden="true">hidden</li>
+          <li style="visibility: hidden" aria-hidden="true">hidden</li>
+          <li role="menuitem">Menuitem</li>
+        </ul>
+      </div>
+    `);
+    assert.isTrue(requiredChildrenCheck.apply(checkContext, params));
+  });
+
   it('should fail when role allows group and group does not have required child', () => {
     const params = checkSetup(
       '<div role="menu" id="target"><ul role="group"><li>Menuitem</li></ul></div>'
@@ -329,19 +348,6 @@ describe('aria-required-children', () => {
   });
 
   describe('options', () => {
-    it('should return undefined instead of false when the role is in options.reviewEmpty', () => {
-      const params = checkSetup('<div role="grid" id="target"></div>', {
-        reviewEmpty: []
-      });
-      assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
-
-      // Options:
-      params[1] = {
-        reviewEmpty: ['grid']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
-
     it('should not throw when options is incorrect', () => {
       const params = checkSetup('<div role="row" id="target"></div>');
 
@@ -358,88 +364,110 @@ describe('aria-required-children', () => {
       assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
     });
 
-    it('should return undefined when the element has empty children', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target"><div></div></div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
+    describe('reviewEmpty', () => {
+      it('should return undefined instead of false when the role is in options.reviewEmpty', () => {
+        const params = checkSetup('<div role="grid" id="target"></div>', {
+          reviewEmpty: []
+        });
+        assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
+
+        // Options:
+        params[1] = {
+          reviewEmpty: ['grid']
+        };
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return false when the element has empty child with role', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target"><div role="grid"></div></div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return undefined when the element has empty children', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"><div></div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return undefined when there is a empty text node', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target"> &nbsp; <!-- empty --> \n\t </div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return false when the element has empty child with role', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"><div role="grid"></div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return false when there is a non-empty text node', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target">  hello  </div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return undefined when there is a empty text node', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"> &nbsp; <!-- empty --> \n\t </div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return undefined when the element has empty child with role=presentation', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target"><div role="presentation"></div></div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return false when there is a non-empty text node', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target">  hello  </div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return undefined when the element has empty child with role=none', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target"><div role="none"></div></div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return undefined when the element has empty child with role=presentation', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"><div role="presentation"></div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return undefined when the element has hidden children', () => {
-      const params = checkSetup(
-        `<div role="menu" id="target">
-          <div role="menuitem" hidden></div>
-          <div role="none" hidden></div>
-          <div role="list" hidden></div>
-        </div>`
-      );
-      params[1] = {
-        reviewEmpty: ['menu']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
-    });
+      it('should return false when role=none child has visible content', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"><div role="none">hello</div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
+      });
+
+      it('should return undefined when role=none child has hidden content', () => {
+        const params = checkSetup(
+          `<div role="listbox" id="target">
+            <div role="none">
+              <h1 style="display:none">hello</h1>
+              <h1 aria-hidden="true">hello</h1>
+              <h1 style="visibility:hidden" aria-hidden="true">hello</h1>
+            </div>
+          </div>`,
+          { reviewEmpty: ['listbox'] }
+        );
+
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
+
+      it('should return undefined when the element has empty child with role=none', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target"><div role="none"></div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
+
+      it('should return undefined when the element has hidden children', () => {
+        const params = checkSetup(
+          `<div role="menu" id="target">
+            <div role="menuitem" hidden></div>
+            <div role="none" hidden></div>
+            <div role="list" hidden></div>
+          </div>`,
+          { reviewEmpty: ['menu'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
 
-    it('should return undefined when the element has empty child and aria-label', () => {
-      const params = checkSetup(
-        '<div role="listbox" id="target" aria-label="listbox"><div></div></div>'
-      );
-      params[1] = {
-        reviewEmpty: ['listbox']
-      };
-      assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      it('should return undefined when the element has empty child and aria-label', () => {
+        const params = checkSetup(
+          '<div role="listbox" id="target" aria-label="listbox"><div></div></div>',
+          { reviewEmpty: ['listbox'] }
+        );
+        assert.isUndefined(requiredChildrenCheck.apply(checkContext, params));
+      });
     });
   });
 });