Skip to content

Commit 07c5d91

Browse files
strakerWilcoFiers
straker
and
WilcoFiers
authored
fix(aria-prohibited-attr): allow aria-label/ledby on decendants of widget (#4541)
Closes: #2953 --------- Co-authored-by: Wilco Fiers <[email protected]>
1 parent f019068 commit 07c5d91

File tree

4 files changed

+167
-36
lines changed

4 files changed

+167
-36
lines changed

‎lib/checks/aria/aria-prohibited-attr-evaluate.js‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { getRole } from '../../commons/aria';
1+
import { getRole, getRoleType } from '../../commons/aria';
22
import { sanitize, subtreeText } from '../../commons/text';
33
import standards from '../../standards';
4+
import memoize from '../../core/utils/memoize';
45

56
/**
67
* Check that an element does not use any prohibited ARIA attributes.
@@ -36,6 +37,7 @@ export default function ariaProhibitedAttrEvaluate(
3637
const role = getRole(virtualNode, { chromium: true });
3738

3839
const prohibitedList = listProhibitedAttrs(
40+
virtualNode,
3941
role,
4042
nodeName,
4143
elementsAllowedAriaLabel
@@ -64,13 +66,32 @@ export default function ariaProhibitedAttrEvaluate(
6466
return true;
6567
}
6668

67-
function listProhibitedAttrs(role, nodeName, elementsAllowedAriaLabel) {
69+
function listProhibitedAttrs(vNode, role, nodeName, elementsAllowedAriaLabel) {
6870
const roleSpec = standards.ariaRoles[role];
6971
if (roleSpec) {
7072
return roleSpec.prohibitedAttrs || [];
7173
}
72-
if (!!role || elementsAllowedAriaLabel.includes(nodeName)) {
74+
if (
75+
!!role ||
76+
elementsAllowedAriaLabel.includes(nodeName) ||
77+
getClosestAncestorRoleType(vNode) === 'widget'
78+
) {
7379
return [];
7480
}
7581
return ['aria-label', 'aria-labelledby'];
7682
}
83+
84+
const getClosestAncestorRoleType = memoize(
85+
function getClosestAncestorRoleTypeMemoized(vNode) {
86+
if (!vNode) {
87+
return;
88+
}
89+
90+
const role = getRole(vNode, { noPresentational: true, chromium: true });
91+
if (role) {
92+
return getRoleType(role);
93+
}
94+
95+
return getClosestAncestorRoleType(vNode.parent);
96+
}
97+
);

‎test/checks/aria/aria-prohibited-attr.js‎

Lines changed: 127 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
describe('aria-prohibited-attr', function () {
1+
describe('aria-prohibited-attr', () => {
22
'use strict';
33

4-
var checkContext = axe.testUtils.MockCheckContext();
5-
var checkSetup = axe.testUtils.checkSetup;
6-
var checkEvaluate = axe.testUtils.getCheckEvaluate('aria-prohibited-attr');
4+
const checkContext = axe.testUtils.MockCheckContext();
5+
const checkSetup = axe.testUtils.checkSetup;
6+
const checkEvaluate = axe.testUtils.getCheckEvaluate('aria-prohibited-attr');
77

8-
afterEach(function () {
8+
afterEach(() => {
99
checkContext.reset();
1010
});
1111

12-
it('should return true for prohibited attributes and no content', function () {
13-
var params = checkSetup(
12+
it('should return true for prohibited attributes and no content', () => {
13+
const params = checkSetup(
1414
'<div id="target" role="code" aria-hidden="false" aria-label="foo"></div>'
1515
);
1616
assert.isTrue(checkEvaluate.apply(checkContext, params));
@@ -22,8 +22,8 @@ describe('aria-prohibited-attr', function () {
2222
});
2323
});
2424

25-
it('should return undefined for prohibited attributes and content', function () {
26-
var params = checkSetup(
25+
it('should return undefined for prohibited attributes and content', () => {
26+
const params = checkSetup(
2727
'<div id="target" role="code" aria-hidden="false" aria-label="foo">Contents</div>'
2828
);
2929
assert.isUndefined(checkEvaluate.apply(checkContext, params));
@@ -35,8 +35,8 @@ describe('aria-prohibited-attr', function () {
3535
});
3636
});
3737

38-
it('should return true for multiple prohibited attributes', function () {
39-
var params = checkSetup(
38+
it('should return true for multiple prohibited attributes', () => {
39+
const params = checkSetup(
4040
'<div id="target" role="code" aria-hidden="false" aria-label="foo" aria-labelledby="foo"></div>'
4141
);
4242
assert.isTrue(checkEvaluate.apply(checkContext, params));
@@ -49,8 +49,10 @@ describe('aria-prohibited-attr', function () {
4949
});
5050
});
5151

52-
it('should return undefined if element has no role and has text content (singular)', function () {
53-
var params = checkSetup('<div id="target" aria-label="foo">Contents</div>');
52+
it('should return undefined if element has no role and has text content (singular)', () => {
53+
const params = checkSetup(
54+
'<div id="target" aria-label="foo">Contents</div>'
55+
);
5456
assert.isUndefined(checkEvaluate.apply(checkContext, params));
5557
assert.deepEqual(checkContext._data, {
5658
nodeName: 'div',
@@ -60,8 +62,8 @@ describe('aria-prohibited-attr', function () {
6062
});
6163
});
6264

63-
it('should return undefined if element has no role and has text content (plural)', function () {
64-
var params = checkSetup(
65+
it('should return undefined if element has no role and has text content (plural)', () => {
66+
const params = checkSetup(
6567
'<div id="target" aria-label="foo" aria-labelledby="foo">Contents</div>'
6668
);
6769
assert.isUndefined(checkEvaluate.apply(checkContext, params));
@@ -73,8 +75,8 @@ describe('aria-prohibited-attr', function () {
7375
});
7476
});
7577

76-
it('should return true if element has no role and no text content (singular)', function () {
77-
var params = checkSetup('<div id="target" aria-label="foo"></div>');
78+
it('should return true if element has no role and no text content (singular)', () => {
79+
const params = checkSetup('<div id="target" aria-label="foo"></div>');
7880
assert.isTrue(checkEvaluate.apply(checkContext, params));
7981
assert.deepEqual(checkContext._data, {
8082
nodeName: 'div',
@@ -84,8 +86,8 @@ describe('aria-prohibited-attr', function () {
8486
});
8587
});
8688

87-
it('should return true if element has no role and no text content (plural)', function () {
88-
var params = checkSetup(
89+
it('should return true if element has no role and no text content (plural)', () => {
90+
const params = checkSetup(
8991
'<div id="target" aria-label="foo" aria-labelledby="foo"></div>'
9092
);
9193
assert.isTrue(checkEvaluate.apply(checkContext, params));
@@ -97,45 +99,139 @@ describe('aria-prohibited-attr', function () {
9799
});
98100
});
99101

100-
it('should return false if all attributes are allowed', function () {
101-
var params = checkSetup(
102+
it('should return false if all attributes are allowed', () => {
103+
const params = checkSetup(
102104
'<div id="target" role="button" aria-label="foo" aria-labelledby="foo">Contents</div>'
103105
);
104106
assert.isFalse(checkEvaluate.apply(checkContext, params));
105107
});
106108

107-
it('should return false if no prohibited attributes are used', function () {
108-
var params = checkSetup(
109+
it('should return false if no prohibited attributes are used', () => {
110+
const params = checkSetup(
109111
'<div id="target" role="code" aria-selected="true">Contents</div>'
110112
);
111113
assert.isFalse(checkEvaluate.apply(checkContext, params));
112114
});
113115

114-
it('should return false if prohibited attributes have no value', function () {
115-
var params = checkSetup(
116+
it('should return false if prohibited attributes have no value', () => {
117+
const params = checkSetup(
116118
'<div id="target" role="code" aria-label=" " aria-labelledby=" ">Contents</div>'
117119
);
118120
assert.isFalse(checkEvaluate.apply(checkContext, params));
119121
});
120122

121-
it('should allow `elementsAllowedAriaLabel` nodes to have aria-label', function () {
122-
var params = checkSetup(
123+
it('should allow `elementsAllowedAriaLabel` nodes to have aria-label', () => {
124+
const params = checkSetup(
123125
'<div id="target" aria-label="hello world"></div>',
124126
{ elementsAllowedAriaLabel: ['div'] }
125127
);
126128
assert.isFalse(checkEvaluate.apply(checkContext, params));
127129
});
128130

129-
it('should not allow `elementsAllowedAriaLabel` nodes with a prohibited role', function () {
130-
var params = checkSetup(
131+
it('should not allow `elementsAllowedAriaLabel` nodes with a prohibited role', () => {
132+
const params = checkSetup(
131133
'<div id="target" role="code" aria-label="hello world"></div>',
132134
{ elementsAllowedAriaLabel: ['div'] }
133135
);
134136
assert.isTrue(checkEvaluate.apply(checkContext, params));
135137
});
136138

137-
it('should allow elements that have an implicit role in chromium', function () {
138-
var params = checkSetup('<svg id="target" aria-label="hello world"></svg>');
139+
it('should allow elements that have an implicit role in chromium', () => {
140+
const params = checkSetup(
141+
'<svg id="target" aria-label="hello world"></svg>'
142+
);
139143
assert.isFalse(checkEvaluate.apply(checkContext, params));
140144
});
145+
146+
describe('widget ancestor', () => {
147+
it('should allow aria-label', () => {
148+
const params = checkSetup(`
149+
<button>
150+
<span>
151+
<span id="target" aria-label="hello world"></span>
152+
</span>
153+
</button>
154+
`);
155+
assert.isFalse(checkEvaluate.apply(checkContext, params));
156+
});
157+
158+
it('should allow aria-labelledby', () => {
159+
const params = checkSetup(`
160+
<div id="foo">hello world</div>
161+
<button>
162+
<span>
163+
<span id="target" aria-labelledby="foo"></span>
164+
</span>
165+
</button>
166+
`);
167+
assert.isFalse(checkEvaluate.apply(checkContext, params));
168+
});
169+
170+
it('should skip "role=none" roles in between ancestor', () => {
171+
const params = checkSetup(`
172+
<button>
173+
<h1 role="none">
174+
<span id="target" aria-label="hello world"></span>
175+
</h1>
176+
</button>
177+
`);
178+
assert.isFalse(checkEvaluate.apply(checkContext, params));
179+
});
180+
181+
it('should skip "role=presentation" roles in between ancestor', () => {
182+
const params = checkSetup(`
183+
<a href="#">
184+
<h1 role="presentation">
185+
<span id="target" aria-label="hello world"></span>
186+
</h1>
187+
</a>
188+
`);
189+
assert.isFalse(checkEvaluate.apply(checkContext, params));
190+
});
191+
192+
it('should not allow aria-label on descendant of non-widget', () => {
193+
const params = checkSetup(`
194+
<div role="grid">
195+
<span>
196+
<span id="target" aria-label="foo"></span>
197+
</span>
198+
</div>
199+
`);
200+
assert.isTrue(checkEvaluate.apply(checkContext, params));
201+
});
202+
203+
it('should not allow aria-labelledby on descendant of non-widget', () => {
204+
const params = checkSetup(`
205+
<div id="foo">hello world</div>
206+
<div role="grid">
207+
<span>
208+
<span id="target" aria-labelledby="foo"></span>
209+
</span>
210+
</div>
211+
`);
212+
assert.isTrue(checkEvaluate.apply(checkContext, params));
213+
});
214+
215+
it('should use closet non-presentational ancestor', () => {
216+
const params = checkSetup(`
217+
<button>
218+
<span role="grid">
219+
<span id="target" aria-label="foo"></span>
220+
</span>
221+
</button>
222+
`);
223+
assert.isTrue(checkEvaluate.apply(checkContext, params));
224+
});
225+
226+
it('should use closet chromium role', () => {
227+
const params = checkSetup(`
228+
<button>
229+
<label>
230+
<span id="target" aria-label="foo"></span>
231+
</label>
232+
</button>
233+
`);
234+
assert.isTrue(checkEvaluate.apply(checkContext, params));
235+
});
236+
});
141237
});

‎test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.html‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
<div id="pass3" aria-labelledby=" ">Foo</div>
44
<div role="alert" aria-selected="true" id="pass4"></div>
55
<div role="row" aria-colcount="value" id="pass5"></div>
6+
<div role="button"><span id="pass6" aria-label="value"></span></div>
7+
<div role="button"><span id="pass7" aria-labelledby="value"></span></div>
68

79
<div role="caption" aria-label="value" id="fail1"></div>
810
<div role="caption" aria-labelledby="value" id="fail2"></div>
@@ -35,6 +37,8 @@
3537
<div role="mark" aria-labelledby="value" id="fail27"></div>
3638
<div role="suggestion" aria-label="value" id="fail28"></div>
3739
<div role="suggestion" aria-labelledby="value" id="fail29"></div>
40+
<div role="grid"><span id="fail30" aria-label="value"></span></div>
41+
<div role="grid"><span id="fail31" aria-labelledby="value"></span></div>
3842

3943
<div id="incomplete1" aria-label="foo">Foo</div>
4044
<div id="incomplete2" aria-labelledby="missing">Foo</div>

‎test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.json‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
{
22
"description": "aria-prohibited-attr tests",
33
"rule": "aria-prohibited-attr",
4-
"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"], ["#pass5"]],
4+
"passes": [
5+
["#pass1"],
6+
["#pass2"],
7+
["#pass3"],
8+
["#pass4"],
9+
["#pass5"],
10+
["#pass6"],
11+
["#pass7"]
12+
],
513
"incomplete": [["#incomplete1"], ["#incomplete2"], ["#incomplete3"]],
614
"violations": [
715
["#fail1"],
@@ -32,6 +40,8 @@
3240
["#fail26"],
3341
["#fail27"],
3442
["#fail28"],
35-
["#fail29"]
43+
["#fail29"],
44+
["#fail30"],
45+
["#fail31"]
3646
]
3747
}

0 commit comments

Comments
 (0)