From e84363e48d7d92c0cd1d944de714db92738d227d Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 6 Jul 2020 11:01:38 -0600 Subject: [PATCH] fix(accessible-name-virtual): allow subtree text to work with virtual and serial nodes --- .../generic/has-text-content-evaluate.js | 6 ++++- lib/checks/shared/has-visible-text.json | 3 ++- lib/commons/aria/lookup-table.js | 2 +- lib/commons/aria/named-from-contents.js | 17 ++++++-------- lib/standards/aria-roles.js | 7 +++++- test/checks/shared/has-visible-text.js | 22 +++++++++++++++++++ test/commons/aria/named-from-contents.js | 7 +++--- 7 files changed, 47 insertions(+), 17 deletions(-) diff --git a/lib/checks/generic/has-text-content-evaluate.js b/lib/checks/generic/has-text-content-evaluate.js index 17b8976c9a..28792a5a48 100644 --- a/lib/checks/generic/has-text-content-evaluate.js +++ b/lib/checks/generic/has-text-content-evaluate.js @@ -1,7 +1,11 @@ import { sanitize, subtreeText } from '../../commons/text'; function hasTextContentEvaluate(node, options, virtualNode) { - return sanitize(subtreeText(virtualNode)) !== ''; + try { + return sanitize(subtreeText(virtualNode)) !== ''; + } catch (e) { + return undefined; + } } export default hasTextContentEvaluate; diff --git a/lib/checks/shared/has-visible-text.json b/lib/checks/shared/has-visible-text.json index 81c2f57a24..a6123446b9 100644 --- a/lib/checks/shared/has-visible-text.json +++ b/lib/checks/shared/has-visible-text.json @@ -5,7 +5,8 @@ "impact": "minor", "messages": { "pass": "Element has text that is visible to screen readers", - "fail": "Element does not have text that is visible to screen readers" + "fail": "Element does not have text that is visible to screen readers", + "incomplete": "Unable to determine if element has children" } } } diff --git a/lib/commons/aria/lookup-table.js b/lib/commons/aria/lookup-table.js index 126be740c2..d31e04ce6c 100644 --- a/lib/commons/aria/lookup-table.js +++ b/lib/commons/aria/lookup-table.js @@ -1906,7 +1906,7 @@ lookupTable.role = { owned: { one: ['rowgroup', 'row'] }, - nameFrom: ['author'], + nameFrom: ['author', 'contents'], context: null, implicit: ['table'], unsupported: false diff --git a/lib/commons/aria/named-from-contents.js b/lib/commons/aria/named-from-contents.js index 3d21f92035..977efc416d 100644 --- a/lib/commons/aria/named-from-contents.js +++ b/lib/commons/aria/named-from-contents.js @@ -1,5 +1,7 @@ import getRole from './get-role'; import lookupTable from './lookup-table'; +import { getNodeFromTree } from '../../core/utils'; +import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node'; /** * Check if an element is named from contents @@ -9,21 +11,16 @@ import lookupTable from './lookup-table'; * @property {Bool} strict Whether or not to follow the spects strictly * @return {Bool} */ -function namedFromContents(node, { strict } = {}) { - node = node.actualNode || node; - if (node.nodeType !== 1) { +function namedFromContents(vNode, { strict } = {}) { + vNode = vNode instanceof AbstractVirtuaNode ? vNode : getNodeFromTree(vNode); + if (vNode.props.nodeType !== 1) { return false; } - const role = getRole(node); + const role = getRole(vNode); const roleDef = lookupTable.role[role]; - if ( - (roleDef && roleDef.nameFrom.includes('contents')) || - // TODO: This is a workaround for axe-core's over-assertive implicitRole computation - // once we fix that, this extra noImplicit check can be removed. - node.nodeName.toUpperCase() === 'TABLE' - ) { + if (roleDef && roleDef.nameFrom.includes('contents')) { return true; } diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index 492124e3c7..5ffc8dad2a 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -510,7 +510,12 @@ const ariaRoles = { table: { type: 'structure', requiredOwned: ['rowgroup', 'row'], - allowedAttrs: ['aria-colcount', 'aria-rowcount', 'aria-expanded'] + allowedAttrs: ['aria-colcount', 'aria-rowcount', 'aria-expanded'], + // NOTE: although the spec says this is not named from contents, + // the accessible text acceptance tests (#139 and #140) require + // table be named from content (we even had to special case + // table in commons/aria/named-from-contents) + nameFromContent: true }, tablist: { type: 'composite', diff --git a/test/checks/shared/has-visible-text.js b/test/checks/shared/has-visible-text.js index dcb2cfe649..342f77addb 100644 --- a/test/checks/shared/has-visible-text.js +++ b/test/checks/shared/has-visible-text.js @@ -40,4 +40,26 @@ describe('has-visible-text', function() { .apply(checkContext, params) ); }); + + describe('SerialVirtualNode', function() { + it('should return false if element is not named from contents', function() { + var node = new axe.SerialVirtualNode({ + nodeName: 'article' + }); + + assert.isFalse( + axe.testUtils.getCheckEvaluate('has-visible-text')(null, {}, node) + ); + }); + + it('should return undefined if element is named from contents', function() { + var node = new axe.SerialVirtualNode({ + nodeName: 'button' + }); + + assert.isUndefined( + axe.testUtils.getCheckEvaluate('has-visible-text')(null, {}, node) + ); + }); + }); }); diff --git a/test/commons/aria/named-from-contents.js b/test/commons/aria/named-from-contents.js index e50b947fcf..a21124e1fe 100644 --- a/test/commons/aria/named-from-contents.js +++ b/test/commons/aria/named-from-contents.js @@ -22,9 +22,10 @@ describe('aria.namedFromContents', function() { }); it('works on virtual nodes', function() { - fixture.innerHTML = '
'; - flatTreeSetup(fixture); - assert.isTrue(namedFromContents({ actualNode: fixture.firstChild })); + var vNode = axe.testUtils.queryFixture( + '
' + ); + assert.isTrue(namedFromContents(vNode)); }); it('returns true when the element has an implicit role named from content', function() {