feat(checks): deprecate role-none and role-presentation for presentat…

…ional-role (#2503) * feat(checks): deprecate role-none and role-presentation for presentational-role * fix locales * Update lib/checks/shared/presentational-role.json Co-authored-by: Wilco Fiers <[email protected]> * Update lib/checks/shared/presentational-role.json Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Wilco Fiers <[email protected]>
dequelabs · Sep 8, 2020 · cef54a0 · cef54a0
1 parent 63262bb
commit cef54a0
Show file tree

Hide file tree

Showing 34 changed files with 392 additions and 81 deletions.
diff --git a/build/configure.js b/build/configure.js
@@ -87,6 +87,7 @@ function buildRules(grunt, options, commons, callback) {
 		var tags = options.tags ? options.tags.split(/\s*,\s*/) : [];
 		var rules = result.rules;
 		var checks = result.checks;
+		parseChecks(checks);
 
 		// Translate checks
 		if (locale && locale.checks) {

diff --git a/lib/checks/shared/presentational-role-evaluate.js b/lib/checks/shared/presentational-role-evaluate.js
@@ -0,0 +1,42 @@
+import { getExplicitRole, getRole } from '../../commons/aria';
+import { getGlobalAriaAttrs } from '../../commons/standards';
+import { isFocusable } from '../../commons/dom';
+
+function presentationalRoleEvaluate(node, options, virtualNode) {
+	const role = getRole(virtualNode);
+	const explicitRole = getExplicitRole(virtualNode);
+
+	if (['presentation', 'none'].includes(role)) {
+		this.data({ role });
+		return true;
+	}
+
+	// if the user didn't intended to make this presentational we fail
+	if (!['presentation', 'none'].includes(explicitRole)) {
+		return false;
+	}
+
+	// user intended to make this presentational so inform them of
+	// problems caused by role conflict resolution
+	const hasGlobalAria = getGlobalAriaAttrs().some(attr =>
+		virtualNode.hasAttr(attr)
+	);
+	const focusable = isFocusable(virtualNode);
+	let messageKey;
+
+	if (hasGlobalAria && !focusable) {
+		messageKey = 'globalAria';
+	} else if (!hasGlobalAria && focusable) {
+		messageKey = 'focusable';
+	} else {
+		messageKey = 'both';
+	}
+
+	this.data({
+		messageKey,
+		role
+	});
+	return false;
+}
+
+export default presentationalRoleEvaluate;
diff --git a/lib/checks/shared/presentational-role.json b/lib/checks/shared/presentational-role.json
@@ -0,0 +1,16 @@
+{
+	"id": "presentational-role",
+	"evaluate": "presentational-role-evaluate",
+	"metadata": {
+		"impact": "minor",
+		"messages": {
+			"pass": "Element's default semantics were overriden with role=\"${data.role}\"",
+			"fail": {
+				"default": "Element's default semantics were not overridden with role=\"none\" or role=\"presentation\"",
+				"globalAria": "Element's role is not presentational because it has a global ARIA attribute",
+				"focusable": "Element's role is not presentational because it is focusable",
+				"both": "Element's role is not presentational because it has a global ARIA attribute and is focusable"
+			}
+		}
+	}
+}
diff --git a/lib/checks/shared/role-none.json b/lib/checks/shared/role-none.json
@@ -1,6 +1,7 @@
 {
 	"id": "role-none",
 	"evaluate": "matches-definition-evaluate",
+	"deprecated": true,
 	"options": {
 		"matcher": {
 			"attributes": {

diff --git a/lib/checks/shared/role-presentation.json b/lib/checks/shared/role-presentation.json
@@ -1,6 +1,7 @@
 {
 	"id": "role-presentation",
 	"evaluate": "matches-definition-evaluate",
+	"deprecated": true,
 	"options": {
 		"matcher": {
 			"attributes": {

diff --git a/lib/core/base/metadata-function-map.js b/lib/core/base/metadata-function-map.js
@@ -69,6 +69,7 @@ import existsEvaluate from '../../checks/shared/exists-evaluate';
 import hasAltEvaluate from '../../checks/shared/has-alt-evaluate';
 import isOnScreenEvaluate from '../../checks/shared/is-on-screen-evaluate';
 import nonEmptyIfPresentEvaluate from '../../checks/shared/non-empty-if-present-evaluate';
+import presentationalRoleEvaluate from '../../checks/shared/presentational-role-evaluate';
 import svgNonEmptyTitleEvaluate from '../../checks/shared/svg-non-empty-title-evaluate';
 
 // mobile
@@ -231,6 +232,7 @@ const metadataFunctionMap = {
 	'has-alt-evaluate': hasAltEvaluate,
 	'is-on-screen-evaluate': isOnScreenEvaluate,
 	'non-empty-if-present-evaluate': nonEmptyIfPresentEvaluate,
+	'presentational-role-evaluate': presentationalRoleEvaluate,
 	'svg-non-empty-title-evaluate': svgNonEmptyTitleEvaluate,
 
 	// mobile

diff --git a/lib/rules/button-name.json b/lib/rules/button-name.json
@@ -17,8 +17,7 @@
 		"button-has-visible-text",
 		"aria-label",
 		"aria-labelledby",
-		"role-presentation",
-		"role-none",
+		"presentational-role",
 		"non-empty-title"
 	],
 	"none": []

diff --git a/lib/rules/frame-title.json b/lib/rules/frame-title.json
@@ -18,8 +18,7 @@
 		"aria-label",
 		"aria-labelledby",
 		"non-empty-title",
-		"role-presentation",
-		"role-none"
+		"presentational-role"
 	],
 	"none": []
 }
diff --git a/lib/rules/image-alt.json b/lib/rules/image-alt.json
@@ -18,8 +18,7 @@
 		"aria-label",
 		"aria-labelledby",
 		"non-empty-title",
-		"role-presentation",
-		"role-none"
+		"presentational-role"
 	],
 	"none": ["alt-space-value"]
 }
diff --git a/lib/rules/input-button-name.json b/lib/rules/input-button-name.json
@@ -18,8 +18,7 @@
 		"non-empty-value",
 		"aria-label",
 		"aria-labelledby",
-		"role-presentation",
-		"role-none",
+		"presentational-role",
 		"non-empty-title"
 	],
 	"none": []

diff --git a/lib/rules/label.json b/lib/rules/label.json
@@ -22,8 +22,7 @@
 		"explicit-label",
 		"non-empty-title",
 		"non-empty-placeholder",
-		"role-none",
-		"role-presentation"
+		"presentational-role"
 	],
 	"none": ["help-same-as-label", "hidden-explicit-label"]
 }
diff --git a/lib/rules/link-name.json b/lib/rules/link-name.json
@@ -18,8 +18,6 @@
 		"has-visible-text",
 		"aria-label",
 		"aria-labelledby",
-		"role-presentation",
-		"role-none",
 		"non-empty-title"
 	],
 	"none": ["focusable-no-name"]

diff --git a/lib/rules/object-alt.json b/lib/rules/object-alt.json
@@ -18,8 +18,7 @@
 		"aria-label",
 		"aria-labelledby",
 		"non-empty-title",
-		"role-presentation",
-		"role-none"
+		"presentational-role"
 	],
 	"none": []
 }
diff --git a/lib/rules/select-name.json b/lib/rules/select-name.json
@@ -20,8 +20,7 @@
 		"implicit-label",
 		"explicit-label",
 		"non-empty-title",
-		"role-none",
-		"role-presentation"
+		"presentational-role"
 	],
 	"none": ["help-same-as-label", "hidden-explicit-label"]
 }
diff --git a/test/checks/shared/presentational-role.js b/test/checks/shared/presentational-role.js
@@ -0,0 +1,66 @@
+describe('presentational-role', function() {
+	'use strict';
+
+	var fixture = document.getElementById('fixture');
+	var queryFixture = axe.testUtils.queryFixture;
+	var checkEvaluate = axe.testUtils.getCheckEvaluate('presentational-role');
+	var checkContext = axe.testUtils.MockCheckContext();
+
+	afterEach(function() {
+		fixture.innerHTML = '';
+		checkContext.reset();
+	});
+
+	it('should detect role="none" on the element', function() {
+		var vNode = queryFixture('<div id="target" role="none"></div>');
+
+		assert.isTrue(checkEvaluate.call(checkContext, null, null, vNode));
+		assert.deepEqual(checkContext._data.role, 'none');
+	});
+
+	it('should detect role="presentation" on the element', function() {
+		var vNode = queryFixture('<div id="target" role="presentation"></div>');
+
+		assert.isTrue(checkEvaluate.call(checkContext, null, null, vNode));
+		assert.deepEqual(checkContext._data.role, 'presentation');
+	});
+
+	it('should return false when role !== none', function() {
+		var vNode = queryFixture('<div id="target" role="cats"></div>');
+
+		assert.isFalse(checkEvaluate.call(checkContext, null, null, vNode));
+	});
+
+	it('should return false when there is no role attribute', function() {
+		var vNode = queryFixture('<div id="target"></div>');
+
+		assert.isFalse(checkEvaluate.call(checkContext, null, null, vNode));
+	});
+
+	it('should return false when the element is focusable', function() {
+		var vNode = queryFixture(
+			'<button id="target" role="none">Still a button</button>'
+		);
+
+		assert.isFalse(checkEvaluate.call(checkContext, null, null, vNode));
+		assert.deepEqual(checkContext._data.messageKey, 'focusable');
+	});
+
+	it('should return false when the element has global aria attributes', function() {
+		var vNode = queryFixture(
+			'<img id="target" role="none" aria-live="assertive" />'
+		);
+
+		assert.isFalse(checkEvaluate.call(checkContext, null, null, vNode));
+		assert.deepEqual(checkContext._data.messageKey, 'globalAria');
+	});
+
+	it('should return false when the element has global aria attributes and is focusable', function() {
+		var vNode = queryFixture(
+			'<button id="target" role="none" aria-live="assertive">Still a button</button>'
+		);
+
+		assert.isFalse(checkEvaluate.call(checkContext, null, null, vNode));
+		assert.deepEqual(checkContext._data.messageKey, 'both');
+	});
+});
diff --git a/test/integration/rules/button-name/button-name.html b/test/integration/rules/button-name/button-name.html
@@ -13,3 +13,9 @@
 <button id="buttonvalue" value="foo" tabindex="-1"></button>
 
 <input type="submit" value="submit" role="button" id="ignored" />
+
+<button id="fail1" role="presentation"></button>
+<button id="fail2" role="none"></button>
+
+<button id="pass1" role="presentation" disabled></button>
+<button id="pass2" role="none" disabled></button>
diff --git a/test/integration/rules/button-name/button-name.json b/test/integration/rules/button-name/button-name.json
@@ -7,7 +7,17 @@
 		["#alempty"],
 		["#albmissing"],
 		["#albempty"],
-		["#buttonvalue"]
+		["#buttonvalue"],
+		["#fail1"],
+		["#fail2"]
 	],
-	"passes": [["#text"], ["#al"], ["#alb"], ["#combo"], ["#buttonTitle"]]
+	"passes": [
+		["#text"],
+		["#al"],
+		["#alb"],
+		["#combo"],
+		["#buttonTitle"],
+		["#pass1"],
+		["#pass2"]
+	]
 }
diff --git a/test/integration/rules/frame-title/frame-title.html b/test/integration/rules/frame-title/frame-title.html
@@ -32,3 +32,28 @@
 	role="presentation"
 ></iframe>
 <iframe src="frame-title/frames/level1a.html" id="pass7" role="none"></iframe>
+
+<iframe
+	src="frame-title/frames/level1a.html"
+	id="violation4"
+	role="none"
+	aria-live="assertive"
+></iframe>
+<iframe
+	src="frame-title/frames/level1a.html"
+	id="violation5"
+	role="presentation"
+	aria-live="assertive"
+></iframe>
+<iframe
+	src="frame-title/frames/level1a.html"
+	id="violation6"
+	role="none"
+	tabindex="0"
+></iframe>
+<iframe
+	src="frame-title/frames/level1a.html"
+	id="violation7"
+	role="presentation"
+	tabindex="0"
+></iframe>
diff --git a/test/integration/rules/frame-title/frame-title.json b/test/integration/rules/frame-title/frame-title.json
@@ -4,7 +4,11 @@
 	"violations": [
 		["#violation1"],
 		["#violation1", "#violation2"],
-		["#violation3"]
+		["#violation3"],
+		["#violation4"],
+		["#violation5"],
+		["#violation6"],
+		["#violation7"]
 	],
 	"passes": [
 		["#violation1", "#pass1"],

diff --git a/test/integration/rules/image-alt/image-alt.html b/test/integration/rules/image-alt/image-alt.html
@@ -14,3 +14,8 @@
 <img src="img.jpg" id="pass7" role="none" />
 <img src="img.jpg" id="pass8" aria-labelledby="ninjamonkeys" />
 <img src="img.jpg" id="violation6" alt=" " />
+
+<img src="img.jpg" id="violation7" role="presentation" aria-live="assertive" />
+<img src="img.jpg" id="violation8" role="none" aria-live="assertive" />
+<img src="img.jpg" id="violation9" role="presentation" tabindex="0" />
+<img src="img.jpg" id="violation10" role="none" tabindex="0" />
diff --git a/test/integration/rules/image-alt/image-alt.json b/test/integration/rules/image-alt/image-alt.json
@@ -7,7 +7,11 @@
 		["#violation3"],
 		["#violation4"],
 		["#violation5"],
-		["#violation6"]
+		["#violation6"],
+		["#violation7"],
+		["#violation8"],
+		["#violation9"],
+		["#violation10"]
 	],
 	"passes": [
 		["#pass1"],

diff --git a/test/integration/rules/input-button-name/input-button-name.html b/test/integration/rules/input-button-name/input-button-name.html
@@ -18,4 +18,9 @@
 	<input type="button" title="Something" id="pass9" />
 	<input type="submit" title="Something" id="pass10" />
 	<input type="reset" title="Something" id="pass11" />
+
+	<input type="button" role="none" id="fail7" />
+	<input type="button" role="presentation" id="fail8" />
+	<input type="button" role="none" id="pass12" disabled />
+	<input type="button" role="presentation" id="pass13" disabled />
 </form>
diff --git a/test/integration/rules/input-button-name/input-button-name.json b/test/integration/rules/input-button-name/input-button-name.json
@@ -7,7 +7,9 @@
 		["#fail3"],
 		["#fail4"],
 		["#fail5"],
-		["#fail6"]
+		["#fail6"],
+		["#fail7"],
+		["#fail8"]
 	],
 	"passes": [
 		["#pass1"],
@@ -20,6 +22,8 @@
 		["#pass8"],
 		["#pass9"],
 		["#pass10"],
-		["#pass11"]
+		["#pass11"],
+		["#pass12"],
+		["#pass13"]
 	]
 }
diff --git a/test/integration/rules/link-name/link-name.html b/test/integration/rules/link-name/link-name.html
@@ -40,3 +40,6 @@
 <span role="link" href="#" tabindex="0" id="pass9" title="title text"></span>
 
 <a href="#" role="button">Does not apply</a>
+
+<a href="#" id="violation7" role="none"></a>
+<a href="#" id="violation8" role="presentation"></a>
diff --git a/test/integration/rules/link-name/link-name.json b/test/integration/rules/link-name/link-name.json
@@ -7,7 +7,9 @@
 		["#violation3"],
 		["#violation4"],
 		["#violation5"],
-		["#violation6"]
+		["#violation6"],
+		["#violation7"],
+		["#violation8"]
 	],
 	"passes": [
 		["#pass1"],

diff --git a/test/integration/rules/object-alt/object-alt.html b/test/integration/rules/object-alt/object-alt.html
@@ -10,3 +10,10 @@
 <object id="violation3"
 	><p style="display: none;">This object has no text.</p></object
 >
+<object id="pass7" role="none"></object>
+<object id="pass8" role="presentation"></object>
+
+<object id="violation4" role="none" tabindex="0"></object>
+<object id="violation5" role="presentation" tabindex="0"></object>
+<object id="violation6" role="none" aria-live="assertive"></object>
+<object id="violation7" role="presentation" aria-live="assertive"></object>