Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit f0facb2

Browse files
clshortfusemmalerba
clshortfuse
authored and
mmalerba
committed
fix(icon): fix aria roles and attributes (#10024)
mdIcon was improperly assuming its parent's text content would be announced didn't apply ARIA roles properly icon component: * Apply `img` role by default * Don't change ARIA if it's already valid * Prioritize `alt` attribute for aria-label * Apply `aria-hidden` attr if parent has ARIA level (2 levels deep) * Fallback to icon name as an aria label * Apply `aria-hidden` attr instead of `aria-hidden` when all checks fail * Add spec tests * Remove improper spec test based on text content aria service: * Add hasAriaLabel func to check for valid aria label * Add parentHasAriaLabel func to check parents for valid aria with role and tagName blacklists * Add spec tests Fixes #9629
1 parent e7de21d commit f0facb2

File tree

4 files changed

+247
-28
lines changed

4 files changed

+247
-28
lines changed

‎src/components/icon/icon.spec.js

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,16 @@ describe('MdIcon directive', function() {
301301

302302
describe('with ARIA support', function() {
303303

304+
it('should apply "img" role by default', function() {
305+
el = make('<md-icon md-svg-icon="android" ></md-icon>');
306+
expect(el.attr('role')).toEqual('img');
307+
});
308+
309+
it('should apply not replace current role', function() {
310+
el = make('<md-icon md-svg-icon="android" role="presentation" ></md-icon>');
311+
expect(el.attr('role')).toEqual('presentation');
312+
});
313+
304314
it('should apply aria-hidden="true" when parent has valid label', function() {
305315
el = make('<button aria-label="Android"><md-icon md-svg-icon="android"></md-icon></button>');
306316
expect(el.find('md-icon').attr('aria-hidden')).toEqual('true');
@@ -314,9 +324,17 @@ describe('MdIcon directive', function() {
314324
expect(el.find('md-icon').attr('aria-hidden')).toEqual('true');
315325
});
316326

317-
it('should apply aria-hidden="true" when parent has text content', function() {
318-
el = make('<button>Android <md-icon md-svg-icon="android"></md-icon></button>');
319-
expect(el.find('md-icon').attr('aria-hidden')).toEqual('true');
327+
it('should not apply aria-hidden="true" when parent has valid label but invalid role', function() {
328+
el = make('<button aria-label="Android" role="command"><md-icon md-svg-icon="android"></md-icon></button>');
329+
expect(el.find('md-icon').attr('aria-hidden')).toBeUndefined();
330+
331+
el = make('<md-radio-button aria-label="avatar 2" role="command"> '+
332+
'<div class="md-container"></div> '+
333+
'<div class="md-label"> '+
334+
'<md-icon md-svg-icon="android"></md-icon> '+
335+
'</div></md-radio-button>');
336+
337+
expect(el.find('md-icon').attr('aria-hidden')).toBeUndefined();
320338
});
321339

322340
it('should apply aria-hidden="true" when aria-label is empty string', function() {
@@ -339,6 +357,11 @@ describe('MdIcon directive', function() {
339357
el = make('<md-icon md-svg-icon="android"></md-icon>');
340358
expect(el.attr('aria-label')).toEqual('android');
341359
});
360+
361+
it('should apply use alt text for aria-label value when not set', function() {
362+
el = make('<md-icon md-svg-icon="android" alt="my android icon"></md-icon>');
363+
expect(el.attr('aria-label')).toEqual('my android icon');
364+
});
342365
});
343366
});
344367

‎src/components/icon/js/iconDirective.js

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -218,20 +218,29 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
218218
// If using a font-icon, then the textual name of the icon itself
219219
// provides the aria-label.
220220

221-
var label = attr.alt || attr.mdFontIcon || attr.mdSvgIcon || element.text();
222221
var attrName = attr.$normalize(attr.$attr.mdSvgIcon || attr.$attr.mdSvgSrc || '');
223222

224-
if ( !attr['aria-label'] ) {
225-
226-
if (label !== '' && !parentsHaveText() ) {
227-
228-
$mdAria.expect(element, 'aria-label', label);
229-
$mdAria.expect(element, 'role', 'img');
230-
231-
} else if ( !element.text() ) {
232-
// If not a font-icon with ligature, then
233-
// hide from the accessibility layer.
223+
/* Provide a default accessibility role of img */
224+
if (!attr.role) {
225+
$mdAria.expect(element, 'role', 'img');
226+
/* manually update attr variable */
227+
attr.role = 'img';
228+
}
234229

230+
/* Don't process ARIA if already valid */
231+
if ( attr.role === "img" && !attr.ariaHidden && !$mdAria.hasAriaLabel(element) ) {
232+
var iconName;
233+
if (attr.alt) {
234+
/* Use alt text by default if available */
235+
$mdAria.expect(element, 'aria-label', attr.alt);
236+
} else if ($mdAria.parentHasAriaLabel(element, 2)) {
237+
/* Parent has ARIA so we will assume it will describe the image */
238+
$mdAria.expect(element, 'aria-hidden', 'true');
239+
} else if (iconName = (attr.mdFontIcon || attr.mdSvgIcon || element.text())) {
240+
/* Use icon name as aria-label */
241+
$mdAria.expect(element, 'aria-label', iconName);
242+
} else {
243+
/* No label found */
235244
$mdAria.expect(element, 'aria-hidden', 'true');
236245
}
237246
}
@@ -247,21 +256,9 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
247256
element.append(svg);
248257
});
249258
}
250-
251259
});
252260
}
253261

254-
function parentsHaveText() {
255-
var parent = element.parent();
256-
if (parent.attr('aria-label') || parent.text()) {
257-
return true;
258-
}
259-
else if(parent.parent().attr('aria-label') || parent.parent().text()) {
260-
return true;
261-
}
262-
return false;
263-
}
264-
265262
function prepareForFontIcon() {
266263
if (!attr.mdSvgIcon && !attr.mdSvgSrc) {
267264
if (attr.mdFontIcon) {

‎src/core/services/aria/aria.js

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
6666
expectAsync: expectAsync,
6767
expectWithText: expectWithText,
6868
expectWithoutText: expectWithoutText,
69-
getText: getText
69+
getText: getText,
70+
hasAriaLabel: hasAriaLabel,
71+
parentHasAriaLabel: parentHasAriaLabel
7072
};
7173

7274
/**
@@ -169,7 +171,152 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
169171
}
170172
}
171173
}
172-
173174
return hasAttr;
174175
}
176+
177+
/**
178+
* Check if expected element has aria label attribute
179+
* @param element
180+
*/
181+
function hasAriaLabel(element) {
182+
var node = angular.element(element)[0] || element;
183+
184+
/* Check if compatible node type (ie: not HTML Document node) */
185+
if (!node.hasAttribute) {
186+
return false;
187+
}
188+
189+
/* Check label or description attributes */
190+
return node.hasAttribute('aria-label') || node.hasAttribute('aria-labelledby') || node.hasAttribute('aria-describedby');
191+
}
192+
193+
/**
194+
* Check if expected element's parent has aria label attribute and has valid role and tagName
195+
* @param element
196+
* @param {optional} level Number of levels deep search should be performed
197+
*/
198+
function parentHasAriaLabel(element, level) {
199+
level = level || 1;
200+
var node = angular.element(element)[0] || element;
201+
if (!node.parentNode) {
202+
return false;
203+
}
204+
if (performCheck(node.parentNode)) {
205+
return true;
206+
}
207+
level--;
208+
if (level) {
209+
return parentHasAriaLabel(node.parentNode, level);
210+
}
211+
return false;
212+
213+
function performCheck(parentNode) {
214+
if (!hasAriaLabel(parentNode)) {
215+
return false;
216+
}
217+
/* Perform role blacklist check */
218+
if (parentNode.hasAttribute('role')) {
219+
switch(parentNode.getAttribute('role').toLowerCase()) {
220+
case 'command':
221+
case 'definition':
222+
case 'directory':
223+
case 'grid':
224+
case 'list':
225+
case 'listitem':
226+
case 'log':
227+
case 'marquee':
228+
case 'menu':
229+
case 'menubar':
230+
case 'note':
231+
case 'presentation':
232+
case 'separator':
233+
case 'scrollbar':
234+
case 'status':
235+
case 'tablist':
236+
return false;
237+
}
238+
}
239+
/* Perform tagName blacklist check */
240+
switch(parentNode.tagName.toLowerCase()) {
241+
case 'abbr':
242+
case 'acronym':
243+
case 'address':
244+
case 'applet':
245+
case 'audio':
246+
case 'b':
247+
case 'bdi':
248+
case 'bdo':
249+
case 'big':
250+
case 'blockquote':
251+
case 'br':
252+
case 'canvas':
253+
case 'caption':
254+
case 'center':
255+
case 'cite':
256+
case 'code':
257+
case 'col':
258+
case 'data':
259+
case 'dd':
260+
case 'del':
261+
case 'dfn':
262+
case 'dir':
263+
case 'div':
264+
case 'dl':
265+
case 'em':
266+
case 'embed':
267+
case 'fieldset':
268+
case 'figcaption':
269+
case 'font':
270+
case 'h1':
271+
case 'h2':
272+
case 'h3':
273+
case 'h4':
274+
case 'h5':
275+
case 'h6':
276+
case 'hgroup':
277+
case 'html':
278+
case 'i':
279+
case 'ins':
280+
case 'isindex':
281+
case 'kbd':
282+
case 'keygen':
283+
case 'label':
284+
case 'legend':
285+
case 'li':
286+
case 'map':
287+
case 'mark':
288+
case 'menu':
289+
case 'object':
290+
case 'ol':
291+
case 'output':
292+
case 'pre':
293+
case 'presentation':
294+
case 'q':
295+
case 'rt':
296+
case 'ruby':
297+
case 'samp':
298+
case 'small':
299+
case 'source':
300+
case 'span':
301+
case 'status':
302+
case 'strike':
303+
case 'strong':
304+
case 'sub':
305+
case 'sup':
306+
case 'svg':
307+
case 'tbody':
308+
case 'td':
309+
case 'th':
310+
case 'thead':
311+
case 'time':
312+
case 'tr':
313+
case 'track':
314+
case 'tt':
315+
case 'ul':
316+
case 'var':
317+
return false;
318+
}
319+
return true;
320+
}
321+
}
175322
}

‎src/core/services/aria/aria.spec.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,58 @@ describe('$mdAria service', function() {
113113

114114
});
115115

116+
describe('aria label checks', function() {
117+
118+
it('should detect if element has valid aria-label string', inject(function($compile, $rootScope, $log, $mdAria) {
119+
var container = $compile('<div aria-label="hello"></div>')($rootScope);
120+
expect($mdAria.hasAriaLabel(container)).toBe(true);
121+
}));
122+
123+
it('should detect if element has valid aria-labelledby string', inject(function($compile, $rootScope, $log, $mdAria) {
124+
var container = $compile('<div aria-labelledby="myOtherElementId"></div>')($rootScope);
125+
expect($mdAria.hasAriaLabel(container)).toBe(true);
126+
}));
127+
128+
it('should detect if element has valid aria-describedby string', inject(function($compile, $rootScope, $log, $mdAria) {
129+
var container = $compile('<div aria-describedby="myOtherElementId"></div>')($rootScope);
130+
expect($mdAria.hasAriaLabel(container)).toBe(true);
131+
}));
132+
133+
});
134+
135+
describe('aria label parent checks', function() {
136+
137+
it('should detect if parent of element has valid aria label', inject(function($compile, $rootScope, $log, $mdAria) {
138+
var container = $compile('<button aria-label="hello"><img></img></button>')($rootScope);
139+
var imgElement = container.find('img');
140+
expect($mdAria.hasAriaLabel(imgElement)).toBe(false);
141+
expect($mdAria.parentHasAriaLabel(imgElement)).toBe(true);
142+
}));
143+
144+
it('should blacklist parent of element based on role', inject(function($compile, $rootScope, $log, $mdAria) {
145+
var container = $compile('<button role="log" aria-label="hello"><img></img></button>')($rootScope);
146+
var imgElement = container.find('img');
147+
expect($mdAria.hasAriaLabel(imgElement)).toBe(false);
148+
expect($mdAria.parentHasAriaLabel(imgElement)).toBe(false);
149+
}));
150+
151+
it('should blacklist parent of element based on tagName', inject(function($compile, $rootScope, $log, $mdAria) {
152+
var container = $compile('<span aria-label="hello"><img></img></span>')($rootScope);
153+
var imgElement = container.find('img');
154+
expect($mdAria.hasAriaLabel(imgElement)).toBe(false);
155+
expect($mdAria.parentHasAriaLabel(imgElement)).toBe(false);
156+
}));
157+
158+
it('should detect if second parent of element has valid aria label', inject(function($compile, $rootScope, $log, $mdAria) {
159+
var container = $compile('<button aria-label="hello"><div><img></img></div></button>')($rootScope);
160+
var imgElement = container.find('img');
161+
expect($mdAria.hasAriaLabel(imgElement)).toBe(false);
162+
expect($mdAria.parentHasAriaLabel(imgElement)).toBe(false);
163+
expect($mdAria.parentHasAriaLabel(imgElement, 2)).toBe(true);
164+
}));
165+
166+
});
167+
116168
describe('with disabled warnings', function() {
117169

118170
beforeEach(module('material.core', function($mdAriaProvider) {

0 commit comments

Comments
 (0)