fix(image-redundant-alt): prevent redundant issues of image tree (#1616)

* fix(image-redundant-alt): prevent redundant issues of image tree * use aria.getRole, rename variable
dequelabs · Jun 7, 2019 · af81897 · af81897
1 parent a36f72a
commit af81897
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 57 deletions.
diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md
@@ -42,7 +42,7 @@
 | html-lang-valid                     | Ensures the lang attribute of the &lt;html&gt; element has a valid value                                                   | Serious            | cat.language, wcag2a, wcag311                                                | true               |
 | html-xml-lang-mismatch              | Ensure that HTML elements with both valid lang and xml:lang attributes agree on the base language of the page              | Moderate           | cat.language, wcag2a, wcag311                                                | true               |
 | image-alt                           | Ensures &lt;img&gt; elements have alternate text or a role of none or presentation                                         | Critical           | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a          | true               |
-| image-redundant-alt                 | Ensure button and link text is not repeated as image alternative                                                           | Minor              | cat.text-alternatives, best-practice                                         | true               |
+| image-redundant-alt                 | Ensure image alternative is not repeated as text                                                                           | Minor              | cat.text-alternatives, best-practice                                         | true               |
 | input-button-name                   | Ensures input buttons have discernible text                                                                                | Critical           | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a            | true               |
 | input-image-alt                     | Ensures &lt;input type=&quot;image&quot;&gt; elements have alternate text                                                  | Critical           | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a          | true               |
 | label-content-name-mismatch         | Ensures that elements labelled through their content must have their visible text as part of their accessible name         | Serious            | wcag21a, wcag253, experimental                                               | true               |

diff --git a/lib/checks/label/duplicate-img-label.js b/lib/checks/label/duplicate-img-label.js
@@ -1,19 +1,17 @@
-const text = axe.commons.text.visibleVirtual(virtualNode, true).toLowerCase();
-if (text === '') {
+const { aria, text, dom } = axe.commons;
+
+if (['none', 'presentation'].includes(aria.getRole(node))) {
 	return false;
 }
 
-// Get all visible images in the composed tree of the current node
-const images = axe.utils
-	.querySelectorAll(virtualNode, 'img')
-	// Ignore hidden or role=none/presentation images
-	.filter(
-		({ actualNode }) =>
-			axe.commons.dom.isVisible(actualNode) &&
-			!['none', 'presentation'].includes(actualNode.getAttribute('role'))
-	);
-
-// See if any of the images duplicate the node's text
-return images.some(
-	img => text === axe.commons.text.accessibleTextVirtual(img).toLowerCase()
+const parent = dom.findUpVirtual(
+	virtualNode,
+	'button, [role="button"], a[href], p, li, td, th'
 );
+const parentVNode = axe.utils.getNodeFromTree(parent);
+const visibleText = text.visibleVirtual(parentVNode, true).toLowerCase();
+if (visibleText === '') {
+	return false;
+}
+
+return visibleText === text.accessibleTextVirtual(virtualNode).toLowerCase();
diff --git a/lib/rules/img-redundant-alt.json b/lib/rules/img-redundant-alt.json
@@ -1,10 +1,10 @@
 {
 	"id": "image-redundant-alt",
-	"selector": "button, [role=\"button\"], a[href], p, li, td, th",
+	"selector": "img",
 	"tags": ["cat.text-alternatives", "best-practice"],
 	"metadata": {
-		"description": "Ensure button and link text is not repeated as image alternative",
-		"help": "Text of buttons and links should not be repeated in the image alternative"
+		"description": "Ensure image alternative is not repeated as text",
+		"help": "Alternative text of images should not be repeated as text"
 	},
 	"all": [],
 	"any": [],

diff --git a/test/checks/label/duplicate-img-label.js b/test/checks/label/duplicate-img-label.js
@@ -10,21 +10,8 @@ describe('duplicate-img-label', function() {
 		axe._tree = undefined;
 	});
 
-	it('should return false if no img is present', function() {
-		fixture.innerHTML = '<button id="target">Plain text</button>';
-		var node = fixture.querySelector('#target');
-		axe.testUtils.flatTreeSetup(fixture);
-		assert.isFalse(
-			checks['duplicate-img-label'].evaluate(
-				node,
-				undefined,
-				axe.utils.getNodeFromTree(node)
-			)
-		);
-	});
-
 	it('should return false if no text is present', function() {
-		fixture.innerHTML = '<button id="target"><img alt="Plain text"></button>';
+		fixture.innerHTML = '<button><img id="target" alt="Plain text"></button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		var result = checks['duplicate-img-label'].evaluate(
@@ -37,7 +24,7 @@ describe('duplicate-img-label', function() {
 
 	it('should return false if aria-label duplicates img alt', function() {
 		fixture.innerHTML =
-			'<button id="target" aria-label="Plain text"><img alt="Plain text"></button>';
+			'<button aria-label="Plain text"><img id="target" alt="Plain text"></button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isFalse(
@@ -51,7 +38,7 @@ describe('duplicate-img-label', function() {
 
 	it('should return false if img and text have different text', function() {
 		fixture.innerHTML =
-			'<button id="target"><img alt="Alt text">Plain text</button>';
+			'<button><img id="target" alt="Alt text">Plain text</button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isFalse(
@@ -65,7 +52,7 @@ describe('duplicate-img-label', function() {
 
 	it('should return true if img and text have the same text', function() {
 		fixture.innerHTML =
-			'<button id="target"><img alt="Plain text">Plain text</button>';
+			'<button><img id="target" alt="Plain text">Plain text</button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isTrue(
@@ -79,7 +66,7 @@ describe('duplicate-img-label', function() {
 
 	it('should return true if img has ARIA label with the same text', function() {
 		fixture.innerHTML =
-			'<button id="target"><img aria-label="Plain text">Plain text</button>';
+			'<button><img id="target" aria-label="Plain text">Plain text</button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isTrue(
@@ -92,7 +79,7 @@ describe('duplicate-img-label', function() {
 	});
 
 	it('should return false if img and text are both blank', function() {
-		fixture.innerHTML = '<button id="target"><img alt=""></button>';
+		fixture.innerHTML = '<button><img id="target" alt=""></button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isFalse(
@@ -106,7 +93,7 @@ describe('duplicate-img-label', function() {
 
 	it('should return false if img and text have superset/subset text', function() {
 		fixture.innerHTML =
-			'<button id="target"><img alt="Plain text and more">Plain text</button>';
+			'<button><img id="target" alt="Plain text and more">Plain text</button>';
 		var node = fixture.querySelector('#target');
 		axe.testUtils.flatTreeSetup(fixture);
 		assert.isFalse(
@@ -125,11 +112,16 @@ describe('duplicate-img-label', function() {
 			button.setAttribute('role', 'button');
 			button.innerHTML = 'My button';
 			var shadow = button.attachShadow({ mode: 'open' });
-			shadow.innerHTML = '<slot></slot><img alt="My button">';
-			var checkArgs = checkSetup(button);
-
+			shadow.innerHTML = '<slot></slot><img id="target" alt="My button">';
+			fixture.appendChild(button);
+			axe.testUtils.flatTreeSetup(fixture);
+			var node = shadow.querySelector('#target');
 			assert.isTrue(
-				checks['duplicate-img-label'].evaluate.apply(null, checkArgs)
+				checks['duplicate-img-label'].evaluate(
+					node,
+					undefined,
+					axe.utils.getNodeFromTree(node)
+				)
 			);
 		}
 	);
@@ -139,13 +131,19 @@ describe('duplicate-img-label', function() {
 		function() {
 			var button = document.createElement('div');
 			button.setAttribute('role', 'button');
-			button.innerHTML = '<img alt="My button">';
+			button.innerHTML = '<img id="target" alt="My button">';
 			var shadow = button.attachShadow({ mode: 'open' });
 			shadow.innerHTML = '<span>My button</span> <slot></slot>';
-			var checkArgs = checkSetup(button);
 
+			fixture.appendChild(button);
+			axe.testUtils.flatTreeSetup(fixture);
+			var node = button.querySelector('#target');
 			assert.isTrue(
-				checks['duplicate-img-label'].evaluate.apply(null, checkArgs)
+				checks['duplicate-img-label'].evaluate(
+					node,
+					undefined,
+					axe.utils.getNodeFromTree(node)
+				)
 			);
 		}
 	);

diff --git a/test/integration/rules/image-redundant-alt/image-redundant-alt.html b/test/integration/rules/image-redundant-alt/image-redundant-alt.html
@@ -1,18 +1,20 @@
-<button id="pass1"><img alt="Img alt text" />Link text</button>
-<a href="#" id="pass2"><img alt="Img alt text" />Link text</a>
-<a href="#" id="pass3"><img alt="Link text" role="presentation" />Link text</a>
-<a href="#" id="pass4"><img alt="Link text" aria-hidden="true" />Link text</a>
+<button><img id="pass1" alt="Img alt text" />Link text</button>
+<a href="#"><img id="pass2" alt="Img alt text" />Link text</a>
+<a href="#"><img id="pass3" alt="Link text" role="presentation" />Link text</a>
 
-<button id="fail1"><img alt="button text" />Button text</button>
-<a href="#" id="fail2"><img alt="Link text" />Link text</a>
-<p id="fail3"><img alt="paragraph text" />paragraph text</p>
+<button><img id="fail1" alt="button text" />Button text</button>
+<a href="#"><img id="fail2" alt="Link text" />Link text</a>
+<p><img id="fail3" alt="paragraph text" />paragraph text</p>
 <table>
 	<tr>
-		<th id="fail4"><img alt="header cell text" />header cell text</th>
-		<td id="fail5"><img alt="data cell text" />data cell text</td>
+		<th><img id="fail4" alt="header cell text" />header cell text</th>
+		<td><img id="fail5" alt="data cell text" />data cell text</td>
 	</tr>
 </table>
 
 <ul>
-	<li id="fail6"><img alt="list text" />list text</li>
+	<li><img id="fail6" alt="list text" />list text</li>
 </ul>
+
+<!-- ignore -->
+<a href="#"><img alt="Link text" aria-hidden="true" />Link text</a>
diff --git a/test/integration/rules/image-redundant-alt/image-redundant-alt.json b/test/integration/rules/image-redundant-alt/image-redundant-alt.json
@@ -9,5 +9,5 @@
 		["#fail5"],
 		["#fail6"]
 	],
-	"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"]]
+	"passes": [["#pass1"], ["#pass2"], ["#pass3"]]
 }