fix(accessible-name-virtual): allow subtree text to work with virtual…

… and serial nodes (#2346)
dequelabs · Jul 7, 2020 · 67d2dca · 67d2dca
1 parent 8ad41af
commit 67d2dca
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -22,9 +22,10 @@ describe('aria.namedFromContents', function() {
 	});
 
 	it('works on virtual nodes', function() {
-		fixture.innerHTML = '<div role="foo"></div>';
-		flatTreeSetup(fixture);
-		assert.isTrue(namedFromContents({ actualNode: fixture.firstChild }));
+		var vNode = axe.testUtils.queryFixture(
+			'<div id="target" role="foo"></div>'
+		);
+		assert.isTrue(namedFromContents(vNode));
 	});
 
 	it('returns true when the element has an implicit role named from content', function() {