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

icon: Providing empty alt or aria-label attributes do not fully hide icon from assistive tech #10721

Closed
katiehockman opened this issue Jun 6, 2017 · 6 comments · Fixed by #12019
Assignees
Labels
a11y This issue is related to accessibility g3: reported The issue was reported by an internal or external product team. has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed.
Milestone

Comments

@katiehockman
Copy link

katiehockman commented Jun 6, 2017

Actual Behavior:

  • What is the issue? *
    Defining alt="" and aria-label="" on md-icon elements do not fully hide the icon from assistive technology. The documentation states that defining those attributes as empty will put aria-hidden="true" on the element, but this doesn't happen.
  • What is the expected behavior? *
    Defining either (or both) alt="" or aria-label="" should set aria-hidden="true" to the md-icon element. The icon name should not be used as an alternative aria-label if no aria-label is available. By defining aria-hidden="true" this element this will fully hide the element from assistive technology.

CodePen (or steps to reproduce the issue): *

  • Details: Add aria-label="" to an md-icon with role="img" and/or add alt="" to an md-icon with role="img". Use a screen reader to navigate to the image element - It will be navigable by a screen reader but will have no label.

AngularJS Versions: *

  • AngularJS Version:
  • AngularJS Material Version: 1.1.4

Additional Information:

  • Browser Type: * Chrome
  • Browser Version: * Version 58.0.3029.110 (64-bit)
  • OS: * macOS Sieera Version 10.12.5
  • Stack Traces:
  • Screen Reader: Voiceover (likely others as well)
@ThomasBurleson
Copy link
Contributor

@katiehockman - do you have a reproducible issue/Plunkr ?

@ThomasBurleson ThomasBurleson added needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue a11y This issue is related to accessibility labels Jun 13, 2017
@ThomasBurleson ThomasBurleson added this to the 1.1.5 milestone Jun 13, 2017
@katiehockman
Copy link
Author

https://codepen.io/katiehockman/pen/GEjRKz
Thanks!

@ThomasBurleson
Copy link
Contributor

@EladBezalel - can you fix this one also?

@clshortfuse
Copy link
Contributor

Note that the codepen is testing against 1.1.0.

Still, in master, there's no logic in mdIcon that checks if the attribute exists, only if it is truthy (if (attr.alt)). Performing a hasAttribute check would fix it. Alternatively, a null-check (alt == null) could work as well, though that would include an implicit conversion.

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Jun 15, 2017

Here is an updated CodePen using latest/HEAD from AngularJS Material: https://codepen.io/angular-flex-layout/pen/eRBVpX

screen shot 2017-06-15 at 6 47 33 pm

Note the link to explicitly load the material-design icons:

<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">

@ThomasBurleson ThomasBurleson removed the needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue label Jun 15, 2017
@katiehockman
Copy link
Author

katiehockman commented Jun 26, 2017

The demo provided still has the issue where providing only an empty alt attribute still creates an aria-label with the inner HTML of the md-icon element. Since the documentation for alt says that "If an empty string is provided, icon will be hidden from accessibility layer", the current implementation will mislead users.

For example:

<md-icon alt="">something</md-icon>
    creates
<md-icon alt="" aria-label="something">something</md-icon>

A few other thoughts/issues:

  • Adding an alt attribute with some label text (and no aria-label) creates an element with both an aria-label and an alt attribute with identical text. This could cause unexpected results with assistive technology.
  • Why do both aria-label and alt need to be in the documentation? When I tested with Voiceover, the alt attribute isn't read by assistive technology, only the aria-label is. So it is perhaps okay to allow users to provide either an aria-label or an alt attribute, but it should always propogate to an aria-label on the rendered element.
  • The documentation should not say that empty alt/aria-label hides from the accessibility layer with aria-hidden="true". That's not what's happening here, and could be misleading for developers.

@ThomasBurleson ThomasBurleson modified the milestones: 1.1.5, 1.1.6 Sep 5, 2017
@Splaktar Splaktar modified the milestones: 1.1.6, 1.1.7 Jan 17, 2018
@Splaktar Splaktar added the P2: required Issues that must be fixed. label Jan 29, 2018
@Splaktar Splaktar modified the milestones: 1.1.7, 1.1.8 Jan 29, 2018
@Splaktar Splaktar added needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community needs: Pull Request labels Feb 18, 2018
@Splaktar Splaktar modified the milestones: 1.1.8, 1.1.9 Mar 16, 2018
@Splaktar Splaktar modified the milestones: 1.1.9, 1.1.10 Apr 19, 2018
@Splaktar Splaktar modified the milestones: 1.1.10, 1.1.11 Jun 19, 2018
@Splaktar Splaktar removed this from the 1.1.11 milestone Sep 10, 2018
@Splaktar Splaktar added this to the 1.1.12 milestone Sep 10, 2018
@Splaktar Splaktar modified the milestones: 1.1.12, 1.1.13 Jan 3, 2019
@Splaktar Splaktar modified the milestones: 1.1.13, 1.1.14 Feb 10, 2019
@Splaktar Splaktar modified the milestones: 1.1.14, g3: sync Feb 15, 2019
@Splaktar Splaktar modified the milestones: g3: sync, 1.1.23 Apr 30, 2020
@Splaktar Splaktar changed the title mdIcon: Providing empty alt or aria-label attributes do not fully hide icon from assistive tech icon: Providing empty alt or aria-label attributes do not fully hide icon from assistive tech Jun 8, 2020
@Splaktar Splaktar modified the milestones: 1.1.23, 1.2.0 Jun 14, 2020
@Splaktar Splaktar modified the milestones: 1.2.0, 1.2.1 Jul 6, 2020
@Splaktar Splaktar added g3: reported The issue was reported by an internal or external product team. and removed for: external contributor needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community labels Sep 21, 2020
@Splaktar Splaktar self-assigned this Sep 21, 2020
@Splaktar Splaktar added the has: Pull Request A PR has been created to address this issue label Sep 21, 2020
Splaktar added a commit that referenced this issue Sep 21, 2020
…hem from a11y

- change behavior around how `aria-label` and `aria-hidden` is applied
  to match documentation
- clarify documentation around `alt` and `aria-label` behavior
- fix/improve Closure types
- remove unused variable and out of date comments
- replace blacklist with block-list in comments

Fixes #10721
Splaktar added a commit that referenced this issue Sep 22, 2020
…hem from a11y

- change behavior around how `aria-label` and `aria-hidden` is applied
  to match documentation
- clarify documentation around `alt` and `aria-label` behavior
- fix/improve Closure types
- remove unused variable and out of date comments
- replace blacklist with block-list in comments

Fixes #10721
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility g3: reported The issue was reported by an internal or external product team. has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants