fix: Allow aria idref(s) to be empty

dequelabs · Jul 13, 2018 · 1498696 · 1498696
1 parent d792970
commit 1498696
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 22 deletions.
diff --git a/lib/commons/aria/attributes.js b/lib/commons/aria/attributes.js
@@ -55,7 +55,7 @@ aria.validateAttr = function(att) {
  * @param  {String} attr The name of the attribute
  * @return {Boolean}
  */
-aria.validateAttrValue = function(node, attr) {
+aria.validateAttrValue = function validateAttrValue(node, attr) {
 	/*eslint complexity: ["error",17]*/
 	'use strict';
 	var matches,
@@ -74,27 +74,27 @@ aria.validateAttrValue = function(node, attr) {
 		case 'nmtoken':
 			return (
 				typeof value === 'string' &&
-				attrInfo.values.indexOf(value.toLowerCase()) !== -1
+				attrInfo.values.includes(value.toLowerCase())
 			);
 
 		case 'nmtokens':
 			list = axe.utils.tokenList(value);
 			// Check if any value isn't in the list of values
 			return list.reduce(function(result, token) {
-				return result && attrInfo.values.indexOf(token) !== -1;
+				return result && attrInfo.values.includes(token);
 				// Initial state, fail if the list is empty
 			}, list.length !== 0);
 
 		case 'idref':
+			// idref is allowed to be empty
+			if (value.trim().length === 0) {
+				return true;
+			}
 			return !!(value && doc.getElementById(value));
 
 		case 'idrefs':
-			// exempt attributes that allow empty strings
-			if (
-				attrInfo.values &&
-				attrInfo.values.indexOf('') !== -1 &&
-				value.trim().length === 0
-			) {
+			// idrefs are allowed to be empty
+			if (value.trim().length === 0) {
 				return true;
 			}
 			list = axe.utils.tokenList(value);

diff --git a/lib/commons/aria/index.js b/lib/commons/aria/index.js
@@ -87,8 +87,7 @@ lookupTable.attributes = {
 		type: 'string'
 	},
 	'aria-labelledby': {
-		type: 'idrefs',
-		values: ['']
+		type: 'idrefs'
 	},
 	'aria-level': {
 		type: 'int'

diff --git a/test/checks/aria/valid-attr-value.js b/test/checks/aria/valid-attr-value.js
@@ -100,17 +100,32 @@ describe('aria-valid-attr-value', function() {
 		axe.commons.aria.validateAttrValue = orig;
 	});
 
-	it('should allow empty strings rather than idrefs for specific attributes', function() {
+	it('should allow empty strings rather than idref', function() {
+		fixtureSetup(
+			'<button aria-controls="">Button</button>' +
+				'<div aria-activedescendant=""></div>'
+		);
+		var passing1 = fixture.querySelector('button');
+		var passing2 = fixture.querySelector('div');
+		assert.isTrue(
+			checks['aria-valid-attr-value'].evaluate.call(checkContext, passing1)
+		);
+		assert.isTrue(
+			checks['aria-valid-attr-value'].evaluate.call(checkContext, passing2)
+		);
+	});
+
+	it('should allow empty strings rather than idrefs', function() {
 		fixtureSetup(
 			'<button aria-labelledby="">Button</button>' + '<div aria-owns=""></div>'
 		);
-		var passing = fixture.querySelector('button');
-		var failing = fixture.querySelector('div');
+		var passing1 = fixture.querySelector('button');
+		var passing2 = fixture.querySelector('div');
 		assert.isTrue(
-			checks['aria-valid-attr-value'].evaluate.call(checkContext, passing)
+			checks['aria-valid-attr-value'].evaluate.call(checkContext, passing1)
 		);
-		assert.isFalse(
-			checks['aria-valid-attr-value'].evaluate.call(checkContext, failing)
+		assert.isTrue(
+			checks['aria-valid-attr-value'].evaluate.call(checkContext, passing2)
 		);
 	});
 

diff --git a/test/integration/rules/aria-valid-attr-value/aria-valid-attr-value.html b/test/integration/rules/aria-valid-attr-value/aria-valid-attr-value.html
@@ -1,8 +1,7 @@
 
 <h2>Violations</h2>
 <div>
-	<div aria-activedescendant="Idonotexist" id="violation0">hi</div>
-	<div aria-activedescendant="" id="violation1">hi</div>
+	<div aria-activedescendant="Idonotexist" id="violation1">hi</div>
 	<div aria-activedescendant="ref noexist ref2" id="violation2">hi</div>
 
 	<div aria-atomic="blah" id="violation3">hi</div>
@@ -250,6 +249,7 @@ <h2>Possible False Positives</h2>
 	<div aria-errormessage="pass68" id="pass169">hi</div>
 	<div aria-errormessage="ref" aria-describedby="ref" id="pass170">hi</div>
 
+	<div aria-activedescendant="" id="pass171">hi</div>
 
 	<div id="ref">Hi</div>
 	<div id="ref2">Hi2</div>

diff --git a/test/integration/rules/aria-valid-attr-value/aria-valid-attr-value.json b/test/integration/rules/aria-valid-attr-value/aria-valid-attr-value.json
@@ -2,7 +2,7 @@
 	"description": "aria-valid-attr-value tests",
 	"rule": "aria-valid-attr-value",
 	"violations": [
-		["#violation0"], ["#violation1"], ["#violation2"], ["#violation3"], ["#violation4"], ["#violation5"],
+		["#violation1"], ["#violation2"], ["#violation3"], ["#violation4"], ["#violation5"],
 		["#violation6"], ["#violation7"], ["#violation8"], ["#violation9"], ["#violation10"], ["#violation11"],
 		["#violation12"], ["#violation13"], ["#violation14"], ["#violation15"], ["#violation16"], ["#violation17"],
 		["#violation18"], ["#violation19"], ["#violation20"], ["#violation21"], ["#violation22"], ["#violation23"],
@@ -31,7 +31,7 @@
 		["#pass140"], ["#pass141"], ["#pass142"], ["#pass143"], ["#pass144"], ["#pass145"], ["#pass146"],
 		["#pass147"], ["#pass148"], ["#pass149"], ["#pass150"], ["#pass151"], ["#pass152"], ["#pass153"],
 		["#pass154"], ["#pass155"], ["#pass156"], ["#pass157"], ["#pass158"], ["#pass159"], ["#pass160"],
-    ["#pass161"], ["#pass162"], ["#pass163"], ["#pass164"], ["#pass165"], ["#pass166"], ["#pass167"],
-    ["#pass168"], ["#pass169"], ["#pass170"]
+		["#pass161"], ["#pass162"], ["#pass163"], ["#pass164"], ["#pass165"], ["#pass166"], ["#pass167"],
+		["#pass168"], ["#pass169"], ["#pass170"], ["#pass171"]
 	]
 }