diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index 4a470d545d..226bd54703 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/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; } diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index 1b7165b127..fe5a15ff82 100644 --- a/test/checks/aria/required-children.js +++ b/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( - '' + `` ); 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(` + + `); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); + }); + it('should fail when role allows group and group does not have required child', () => { const params = checkSetup( '' @@ -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('
', { - 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('
'); @@ -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( - '
' - ); - 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('
', { + 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( - '
' - ); - params[1] = { - reviewEmpty: ['listbox'] - }; - assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); - }); + it('should return undefined when the element has empty children', () => { + const params = checkSetup( + '
', + { reviewEmpty: ['listbox'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return undefined when there is a empty text node', () => { - const params = checkSetup( - '
  \n\t
' - ); - 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( + '
', + { reviewEmpty: ['listbox'] } + ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return false when there is a non-empty text node', () => { - const params = checkSetup( - '
hello
' - ); - params[1] = { - reviewEmpty: ['listbox'] - }; - assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); - }); + it('should return undefined when there is a empty text node', () => { + const params = checkSetup( + '
  \n\t
', + { reviewEmpty: ['listbox'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return undefined when the element has empty child with role=presentation', () => { - const params = checkSetup( - '
' - ); - 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( + '
hello
', + { reviewEmpty: ['listbox'] } + ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return undefined when the element has empty child with role=none', () => { - const params = checkSetup( - '
' - ); - 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( + '
', + { reviewEmpty: ['listbox'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return undefined when the element has hidden children', () => { - const params = checkSetup( - `` - ); - params[1] = { - reviewEmpty: ['menu'] - }; - assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); - }); + it('should return false when role=none child has visible content', () => { + const params = checkSetup( + '
hello
', + { reviewEmpty: ['listbox'] } + ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return undefined when role=none child has hidden content', () => { + const params = checkSetup( + `
+
+

hello

+

hello

+

hello

+
+
`, + { reviewEmpty: ['listbox'] } + ); + + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return undefined when the element has empty child with role=none', () => { + const params = checkSetup( + '
', + { reviewEmpty: ['listbox'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return undefined when the element has hidden children', () => { + const params = checkSetup( + ``, + { reviewEmpty: ['menu'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); - it('should return undefined when the element has empty child and aria-label', () => { - const params = checkSetup( - '
' - ); - 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( + '
', + { reviewEmpty: ['listbox'] } + ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); }); }); }); diff --git a/test/integration/rules/aria-required-children/aria-required-children.html b/test/integration/rules/aria-required-children/aria-required-children.html index 410434d972..def03ca181 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -27,6 +27,9 @@
@@ -143,3 +146,12 @@
menu item
+ +
+
+
ignore
+ +
  • item 1
  • +
  • item 2
  • +
    +
    diff --git a/test/integration/rules/aria-required-children/aria-required-children.json b/test/integration/rules/aria-required-children/aria-required-children.json index 23f2822f0a..21f2ed2f3a 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -33,7 +33,8 @@ ["#pass15"], ["#pass16"], ["#pass17"], - ["#pass18"] + ["#pass18"], + ["#pass19"] ], "incomplete": [ ["#incomplete1"],