fix(aria-required-attr): allow aria-valuenow to pass on elements with…

… value (#1579)

* fix(aria-required-attr): allow aria-valuenow to pass on elements with value

* allow empty string values to pass

* add new form functions to identify elements with value

* reset file

* fix IE11

* move hasAttribute check outside isAriaRange

lib/checks/aria/required-attr.js

@@ -1,19 +1,44 @@
 options = options || {};
 
-var missing = [];
+const missing = [];
+const {
+	isNativeTextbox,
+	isNativeSelect,
+	isAriaTextbox,
+	isAriaListbox,
+	isAriaCombobox,
+	isAriaRange
+} = axe.commons.forms;
+
+// aria-valuenow should fail if element does not have a value property
+// @see https://github.com/dequelabs/axe-core/issues/1501
+const preChecks = {
+	'aria-valuenow': function() {
+		return !(
+			isNativeTextbox(node) ||
+			isNativeSelect(node) ||
+			isAriaTextbox(node) ||
+			isAriaListbox(node) ||
+			isAriaCombobox(node) ||
+			(isAriaRange(node) && node.hasAttribute('aria-valuenow'))
+		);
+	}
+};
 
 if (node.hasAttributes()) {
-	var attr,
-		role = node.getAttribute('role'),
-		required = axe.commons.aria.requiredAttr(role);
+	const role = node.getAttribute('role');
+	let required = axe.commons.aria.requiredAttr(role);
 
 	if (Array.isArray(options[role])) {
 		required = axe.utils.uniqueArray(options[role], required);
 	}
 	if (role && required) {
-		for (var i = 0, l = required.length; i < l; i++) {
-			attr = required[i];
-			if (!node.getAttribute(attr)) {
+		for (let i = 0, l = required.length; i < l; i++) {
+			const attr = required[i];
+			if (
+				!node.getAttribute(attr) &&
+				(preChecks[attr] ? preChecks[attr]() : true)
+			) {
 				missing.push(attr);
 			}
 		}

lib/commons/forms/index.js

@@ -0,0 +1,8 @@
+/**
+ * Namespace for forms-related utilities.
+ * @namespace commons.forms
+ * @memberof axe
+ */
+
+var forms = {};
+commons.forms = forms;

lib/commons/forms/is-aria-combobox.js

@@ -0,0 +1,13 @@
+/* global forms */
+
+/**
+ * Determines if an element is an aria combobox element
+ * @method isAriaCombobox
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if aria combobox
+ * @returns {Bool}
+ */
+forms.isAriaCombobox = function(node) {
+	const role = axe.commons.aria.getRole(node, { noImplicit: true });
+	return role === 'combobox';
+};

lib/commons/forms/is-aria-listbox.js

@@ -0,0 +1,13 @@
+/* global forms */
+
+/**
+ * Determines if an element is an aria listbox element
+ * @method isAriaListbox
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if aria listbox
+ * @returns {Bool}
+ */
+forms.isAriaListbox = function(node) {
+	const role = axe.commons.aria.getRole(node, { noImplicit: true });
+	return role === 'listbox';
+};

lib/commons/forms/is-aria-range.js

@@ -0,0 +1,14 @@
+/* global forms */
+const rangeRoles = ['progressbar', 'scrollbar', 'slider', 'spinbutton'];
+
+/**
+ * Determines if an element is an aria range element
+ * @method isAriaRange
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if aria range
+ * @returns {Bool}
+ */
+forms.isAriaRange = function(node) {
+	const role = axe.commons.aria.getRole(node, { noImplicit: true });
+	return rangeRoles.includes(role);
+};

lib/commons/forms/is-aria-textbox.js

@@ -0,0 +1,13 @@
+/* global forms */
+
+/**
+ * Determines if an element is an aria textbox element
+ * @method isAriaTextbox
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if aria textbox
+ * @returns {Bool}
+ */
+forms.isAriaTextbox = function(node) {
+	const role = axe.commons.aria.getRole(node, { noImplicit: true });
+	return role === 'textbox';
+};

lib/commons/forms/is-native-select.js

@@ -0,0 +1,13 @@
+/* global forms */
+
+/**
+ * Determines if an element is a native select element
+ * @method isNativeSelect
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if select
+ * @returns {Bool}
+ */
+forms.isNativeSelect = function(node) {
+	const nodeName = node.nodeName.toUpperCase();
+	return nodeName === 'SELECT';
+};

lib/commons/forms/is-native-textbox.js

@@ -0,0 +1,28 @@
+/* global forms */
+const nonTextInputTypes = [
+	'button',
+	'checkbox',
+	'color',
+	'file',
+	'hidden',
+	'image',
+	'password',
+	'radio',
+	'reset',
+	'submit'
+];
+
+/**
+ * Determines if an element is a native textbox element
+ * @method isNativeTextbox
+ * @memberof axe.commons.forms
+ * @param {Element} node Node to determine if textbox
+ * @returns {Bool}
+ */
+forms.isNativeTextbox = function(node) {
+	const nodeName = node.nodeName.toUpperCase();
+	return (
+		nodeName === 'TEXTAREA' ||
+		(nodeName === 'INPUT' && !nonTextInputTypes.includes(node.type))
+	);
+};

lib/commons/text/form-control-value.js

@@ -1,7 +1,13 @@
 /* global text, aria, dom */
-const selectRoles = ['combobox', 'listbox'];
-const rangeRoles = ['progressbar', 'scrollbar', 'slider', 'spinbutton'];
-const controlValueRoles = ['textbox', ...selectRoles, ...rangeRoles];
+const controlValueRoles = [
+	'textbox',
+	'progressbar',
+	'scrollbar',
+	'slider',
+	'spinbutton',
+	'combobox',
+	'listbox'
+];
 
 text.formControlValueMethods = {
 	nativeTextboxValue,
@@ -62,24 +68,7 @@ text.formControlValue = function formControlValue(virtualNode, context = {}) {
  */
 function nativeTextboxValue(node) {
 	node = node.actualNode || node;
-	const nonTextInputTypes = [
-		'button',
-		'checkbox',
-		'file',
-		'hidden',
-		'image',
-		'password',
-		'radio',
-		'reset',
-		'submit',
-		'color'
-	];
-	const nodeName = node.nodeName.toUpperCase();
-
-	if (
-		nodeName === 'TEXTAREA' ||
-		(nodeName === 'INPUT' && !nonTextInputTypes.includes(node.type))
-	) {
+	if (axe.commons.forms.isNativeTextbox(node)) {
 		return node.value || '';
 	}
 	return '';
@@ -93,7 +82,7 @@ function nativeTextboxValue(node) {
  */
 function nativeSelectValue(node) {
 	node = node.actualNode || node;
-	if (node.nodeName.toUpperCase() !== 'SELECT') {
+	if (!axe.commons.forms.isNativeSelect(node)) {
 		return '';
 	}
 	return (
@@ -112,8 +101,7 @@ function nativeSelectValue(node) {
  */
 function ariaTextboxValue(virtualNode) {
 	const { actualNode } = virtualNode;
-	const role = aria.getRole(actualNode);
-	if (role !== 'textbox') {
+	if (!axe.commons.forms.isAriaTextbox(actualNode)) {
 		return '';
 	}
 	if (!dom.isHiddenWithCSS(actualNode)) {
@@ -135,8 +123,7 @@ function ariaTextboxValue(virtualNode) {
  */
 function ariaListboxValue(virtualNode, context) {
 	const { actualNode } = virtualNode;
-	const role = aria.getRole(actualNode);
-	if (role !== 'listbox') {
+	if (!axe.commons.forms.isAriaListbox(actualNode)) {
 		return '';
 	}
 
@@ -169,11 +156,10 @@ function ariaListboxValue(virtualNode, context) {
  */
 function ariaComboboxValue(virtualNode, context) {
 	const { actualNode } = virtualNode;
-	const role = aria.getRole(actualNode, { noImplicit: true });
 	let listbox;
 
 	// For combobox, find the first owned listbox:
-	if (!role === 'combobox') {
+	if (!axe.commons.forms.isAriaCombobox(actualNode)) {
 		return '';
 	}
 	listbox = aria
@@ -193,8 +179,10 @@ function ariaComboboxValue(virtualNode, context) {
  */
 function ariaRangeValue(node) {
 	node = node.actualNode || node;
-	const role = aria.getRole(node);
-	if (!rangeRoles.includes(role) || !node.hasAttribute('aria-valuenow')) {
+	if (
+		!axe.commons.forms.isAriaRange(node) ||
+		!node.hasAttribute('aria-valuenow')
+	) {
 		return '';
 	}
 	// Validate the number, if not, return 0.

test/checks/aria/required-attr.js

@@ -2,43 +2,47 @@ describe('aria-required-attr', function() {
 	'use strict';
 
 	var fixture = document.getElementById('fixture');
+	var queryFixture = axe.testUtils.queryFixture;
 	var checkContext = axe.testUtils.MockCheckContext();
+	var options = undefined;
 
 	afterEach(function() {
 		fixture.innerHTML = '';
 		checkContext.reset();
 	});
 
 	it('should detect missing attributes', function() {
-		var node = document.createElement('div');
-		node.setAttribute('role', 'slider');
-		node.id = 'test';
-		node.tabIndex = 1;
-		fixture.appendChild(node);
+		var vNode = queryFixture('<div id="target" role="slider" tabindex="1">');
 
 		assert.isFalse(
-			checks['aria-required-attr'].evaluate.call(checkContext, node)
+			checks['aria-required-attr'].evaluate.call(
+				checkContext,
+				vNode.actualNode,
+				options,
+				vNode
+			)
 		);
 		assert.deepEqual(checkContext._data, ['aria-valuenow']);
 	});
 
 	it('should return true if there is no role', function() {
-		var node = document.createElement('div');
-		fixture.appendChild(node);
+		var vNode = queryFixture('<div id="target">');
 
 		assert.isTrue(
-			checks['aria-required-attr'].evaluate.call(checkContext, node)
+			checks['aria-required-attr'].evaluate.call(
+				checkContext,
+				vNode.actualNode,
+				options,
+				vNode
+			)
 		);
 		assert.isNull(checkContext._data);
 	});
 
 	it('should determine attribute validity by calling axe.commons.aria.requiredAttr', function() {
-		var node = document.createElement('div');
-		node.id = 'test';
-		node.tabIndex = 1;
-		node.setAttribute('role', 'cats');
-		node.setAttribute('aria-cats', 'maybe');
-		fixture.appendChild(node);
+		var vNode = queryFixture(
+			'<div id="target" role="cats" tabindex="1" aria-cats="maybe">'
+		);
 
 		var orig = axe.commons.aria.requiredAttr;
 		var called = 0;
@@ -48,14 +52,32 @@ describe('aria-required-attr', function() {
 			return ['aria-cats', 'aria-bats'];
 		};
 		assert.isFalse(
-			checks['aria-required-attr'].evaluate.call(checkContext, node)
+			checks['aria-required-attr'].evaluate.call(
+				checkContext,
+				vNode.actualNode,
+				options,
+				vNode
+			)
 		);
 		assert.deepEqual(checkContext._data, ['aria-bats']);
 		assert.equal(called, 1);
 
 		axe.commons.aria.requiredAttr = orig;
 	});
 
+	it('should pass aria-valuenow if element has value property', function() {
+		var vNode = queryFixture('<input id="target" type="range" role="slider">');
+
+		assert.isTrue(
+			checks['aria-required-attr'].evaluate.call(
+				checkContext,
+				vNode.actualNode,
+				options,
+				vNode
+			)
+		);
+	});
+
 	describe('options', function() {
 		it('should require provided attribute names for a role', function() {
 			axe.commons.aria.lookupTable.role.mccheddarton = {
@@ -67,16 +89,16 @@ describe('aria-required-attr', function() {
 				nameFrom: ['author'],
 				context: null
 			};
-			fixture.innerHTML = '<div role="mccheddarton" id="target"></div>';
-			var target = fixture.children[0];
+			var vNode = queryFixture('<div role="mccheddarton" id="target"></div>');
 			var options = {
 				mccheddarton: ['aria-snuggles']
 			};
 			assert.isFalse(
 				checks['aria-required-attr'].evaluate.call(
 					checkContext,
-					target,
-					options
+					vNode.actualNode,
+					options,
+					vNode
 				)
 			);
 			assert.deepEqual(checkContext._data, ['aria-snuggles', 'aria-valuemax']);

test/commons/forms/is-aria-combobox.js

@@ -0,0 +1,21 @@
+describe('forms.isAriaCombobox', function() {
+	'use strict';
+	var isAriaCombobox = axe.commons.forms.isAriaCombobox;
+
+	it('returns true for an element with role=combobox', function() {
+		var node = document.createElement('div');
+		node.setAttribute('role', 'combobox');
+		assert.isTrue(isAriaCombobox(node));
+	});
+
+	it('returns false for elements without role', function() {
+		var node = document.createElement('div');
+		assert.isFalse(isAriaCombobox(node));
+	});
+
+	it('returns false for elements with incorrect role', function() {
+		var node = document.createElement('div');
+		node.setAttribute('role', 'main');
+		assert.isFalse(isAriaCombobox(node));
+	});
+});