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

checkbox: links in text are not rendered in a way that makes JAWS happy #11134

Closed
pieman72 opened this issue Feb 22, 2018 · 10 comments · Fixed by #11154
Closed

checkbox: links in text are not rendered in a way that makes JAWS happy #11134

pieman72 opened this issue Feb 22, 2018 · 10 comments · Fixed by #11154
Assignees
Labels
- Breaking Change 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 P3: important Important issues that really should be fixed when possible. resolution: fixed
Milestone

Comments

@pieman72
Copy link

pieman72 commented Feb 22, 2018

Bug, feature request, or proposal:

Bug

What is the expected behavior?

A link (<a href="...">) nested inside an <md-checkbox ...> element should be recognized as a link by screen readers such as JAWS. This entails being accessible via screen reader specific navigation shortcuts (U and V keys for JAWS), and upon receiving focus it should be announced that the focused element is a link.

What is the current behavior?

Such links are not accessible via shortcut keys, and are not announced as links.

CodePen and steps to reproduce the issue:

CodePen Demo] which shows your issue:

https://codepen.io/anon/pen/VQdGBX

Detailed Reproduction Steps:
  1. Enable JAWS prior to enabling Firefox
  2. Open Firefox and activate the JAWS screen reader
  3. Visit a page containing a <a> inside a <md-checkbox> (such as the CodePen example linked above)
  4. Use the U (unvisited link) key to browse through the not visited links with JAWS (use V key if link was visited before)
  5. Verify that the link inside the md-checkbox is reachable
  6. Using the TAB key, place the focus on that same link
  7. Listen to the screenreder's audio feedback, and Verify that the screen reader announces that a link is focused

What is the use-case or motivation for changing an existing behavior?

A11y - Currently, a standard practice (placing links inside a checkbox label) results in an inaccessible page.

Which versions of AngularJS, Material, OS, and browsers are affected?

Angular 1.6
Material 1.1
Windows 10 Enterprize
Firefox 52.6.0 ESR
JAWS 2018 (1712 10 ILM), NVDA 2017.4

Is there anything else we should know? Stack Traces, Screenshots, etc.

The HTML output by Angular results in the following accessibility tree:

checkbox "I agree to the privacy policy"

GenericContainer

link "privacy policy"

text "privacy policy"

I suspect a link typically should be inside a label, not nested directly within the checkbox.

@pieman72
Copy link
Author

Likely as a side effect of this, the text in the link is voiced twice by the screen reader. When there is no link, the content of the checkbox is "ignored" in the accessibility tree.

@Splaktar Splaktar added a11y This issue is related to accessibility needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community labels Feb 26, 2018
@Splaktar Splaktar self-assigned this Feb 26, 2018
@Splaktar Splaktar changed the title Links inside md-checkboxes are not accessible checkbox: links in text are not rendered in an accessible way Feb 26, 2018
@Splaktar
Copy link
Member

I enhanced the CodePen a bit for me to test with. This included adding a form and ng-model so that aria-checked would be set properly. Using ChromeVox on macOS, I don't really see any significant problems.

I will try to do some more testing using VoiceOver and Safari.

@Splaktar Splaktar changed the title checkbox: links in text are not rendered in an accessible way checkbox: links in text are not rendered in a way that makes JAWS happy Feb 26, 2018
@Splaktar Splaktar added the P4: minor Minor issues. May not be fixed without community contributions. label Feb 26, 2018
@Splaktar Splaktar added this to the 1.1.9 milestone Feb 26, 2018
@pieman72
Copy link
Author

The issue does not manifest itself in ChromeVox (neither in your example nor in mine), but it does occur in JAWS (at least on Firefox on Windows).

@jelbourn jelbourn added g3: reported The issue was reported by an internal or external product team. P3: important Important issues that really should be fixed when possible. and removed P4: minor Minor issues. May not be fixed without community contributions. labels Feb 26, 2018
@Splaktar
Copy link
Member

We currently parse everything inside of the <md-checkbox>here</md-checkbox> into <span> elements with other HTML elements preserved.
I.e. <span>Please accept</span><a href="blah">privacy policy</a><span>.</span>
This seems like it would clearly cause some extra screen reader output.

My research into more a11y-friendly/normal implementations would use:
<label for="cb"><input id="cb" type="checkbox">Please accept the <a href="blah">privacy policy</a></label>. Links within label elements seem to be within the standards. Angular Material does this.

However moving to the label/native input method would be a breaking change. We're going to try to solve this by testing without the <span> elements or using aria attributes and see if we can come up with a non-breaking way of solving this.

@pieman72
Copy link
Author

Understood. I agree with the "normal implementation" you described, but understand that it would be a breaking change. Your proposed alternative sounds good to me, thanks!

@Splaktar
Copy link
Member

Splaktar commented Mar 8, 2018

I am able to reproduce this on macOS and Safari with Voiceover. When navigating by links, the privacy policy link within the checkbox is skipped.

The extra <span> elements that I was previously seeing appear to have been an artifact of CodePen as I don't see them in Chrome or Safari where the following is rendered:

<md-checkbox ng-model="data.cb1" tabindex="0" type="checkbox" role="checkbox"
             class="ng-pristine ng-untouched ng-valid ng-not-empty md-checked"
             aria-label="I agree to the privacy policy." aria-checked="true"
             aria-invalid="false">
  <div class="md-container md-ink-ripple" md-ink-ripple="" md-ink-ripple-checkbox="">
    <div class="md-icon"></div>
  </div>
  <div ng-transclude="" class="md-label">
            I agree to the <a href="https://www.google.com/"
            ng-click="$event.stopPropagation()" class="ng-scope"> privacy policy</a>.
  </div>
</md-checkbox>

Setting tabindex="0" on the md-checkbox is generally what we want. However, this means that any focus within the element must be handled by the element itself (normally via the arrow keys). That makes sense for menus and selects, but doesn't make much sense in this case. This also means that a link within this element is not focusable via tabs or screen reader next link navigation.

I tried a number of approaches with different combinations of tabindex as well as trying to forward focus to links within the checkbox's md-labelvia JavaScript, but none of them produced satisfactory results. I also looked at the available aria attributes and did not see anything that would help with this situation. I did try out the aria-role="link" attribute, but adding that to an <a> element doesn't help, or make much sense.

For the time being, the second example in my CodePen appears to be the best approach to doing this in an accesible way with md-checkbox. I.e. keep the link text out of the md-checkbox and use an appropriate aria-label.

As far as I've been able to determine, supporting accessible links within md-checkbox labels will require a rewrite of much of the component in order to use a native <input type="checkbox">. This would be a significant breaking change that would need to wait for 1.2.0.

It's also something that Angular Material (v2+) has already done.

@Splaktar Splaktar added needs: team discussion This issue requires a decision from the team before moving forward. and removed needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community labels Mar 8, 2018
@jelbourn
Copy link
Member

jelbourn commented Mar 8, 2018

Would a workaround with aria-labelledby solve this? E.g.

<div class="my-checkbox-label" id="agree-label">
  I agree to the <a href="...">Terms of service</a>
</div>
<md-checkbox aria-labelledby="agree-label"></md-checkbox>

@Splaktar
Copy link
Member

Splaktar commented Mar 8, 2018

The following has the same visual layout and behavior while also working properly in Voiceover:

<md-checkbox ng-model="data.cb1" aria-labelledby="agree-label"></md-checkbox>
<span id="agree-label" onclick="data.cb1 = !data.cb1">
  I agree to the <a href="https://www.google.com/">privacy policy</a>.
</span>

To make this part of the directive, we need to solve some problems

  1. Generate a unique id for the label and use it in the aria-labelledby. Can be done via $mdUtil.nextUid().
  2. Append the label as a <span> after the directive. Should this only happen if there is a link (probably not breaking) or always (breaking change)?

@Splaktar Splaktar removed the needs: team discussion This issue requires a decision from the team before moving forward. label Mar 8, 2018
@Splaktar Splaktar modified the milestones: 1.1.9, 1.1.8 Mar 8, 2018
Splaktar added a commit that referenced this issue Mar 8, 2018
don't toggle the checkbox on link actions
don't output aria warnings when using aria-labelledby
require md-input-container when using links in checkbox labels
update license copyright year

Fixes #11134
@Splaktar
Copy link
Member

Splaktar commented Mar 8, 2018

I have the first draft of a PR up for this. To use a link in a checkbox label you would need to do the following:

<md-input-container>
  <md-checkbox ng-model="data.cb5">
    I agree to the <a href="/license">license</a>.
  </md-checkbox>
</md-input-container>

If you want to omit the md-input-container, then you would be responsible for laying out the label and styling properly as we can't ensure that will happen when we don't have control over the parent element and it's layout. If the only issue is the md-input-container margins, then just overriding those specific styles for the element may be easiest.

@Splaktar Splaktar added the has: Pull Request A PR has been created to address this issue label Mar 9, 2018
Splaktar added a commit that referenced this issue Mar 12, 2018
don't toggle the checkbox on link actions
don't output aria warnings when using aria-labelledby
require md-input-container when using links in checkbox labels
update license copyright year

Fixes #11134
Splaktar added a commit that referenced this issue Mar 12, 2018
don't toggle the checkbox on link actions
don't output aria warnings when using aria-labelledby
require md-input-container when using links in checkbox labels
update license copyright year

Fixes #11134
Splaktar added a commit that referenced this issue Mar 12, 2018
don't toggle the checkbox on link actions
don't output aria warnings when using aria-labelledby
require md-input-container when using links in checkbox labels
update license copyright year

Fixes #11134
@Splaktar Splaktar modified the milestones: 1.1.8, 1.1.9 Mar 17, 2018
@Splaktar Splaktar modified the milestones: 1.1.9, 1.2.0 Mar 29, 2018
@Splaktar
Copy link
Member

As this change is a breaking change, I've pushed it to 1.2.0. More details in #11154 (comment).

Splaktar added a commit that referenced this issue Jun 30, 2020
- JAWS and Voiceover should properly handle links in md-checkbox labels
- Wrapping the checkbox in a `md-input-container` is required
  - for using links in checkbox labels due to layout considerations
- Add new demos for different types of a11y checkbox labels
- don't toggle the checkbox on link actions
- don't output aria warnings when using `aria-labelledby`
- require `md-input-container` when using links in checkbox labels

Fixes #11134.
Splaktar added a commit that referenced this issue Jun 30, 2020
- JAWS and Voiceover should properly handle links in md-checkbox labels
- Wrapping the checkbox in a `md-input-container` is required
  - for using links in checkbox labels due to layout considerations
- Add new demos for different types of a11y checkbox labels
- don't toggle the checkbox on link actions
- don't output aria warnings when using `aria-labelledby`
- require `md-input-container` when using links in checkbox labels

Fixes #11134.

BREAKING CHANGE: If you've created a custom solution to style links within `md-checkbox` labels, then you may need to remove or change that code now. This is because we automatically detect `<a>` tags in these labels and re-render them in an accessible way.
Splaktar added a commit that referenced this issue Jun 30, 2020
- JAWS and Voiceover should properly handle links in md-checkbox labels
- Wrapping the checkbox in a `md-input-container` is required
  - for using links in checkbox labels due to layout considerations
- Add new demos for different types of a11y checkbox labels
- don't toggle the checkbox on link actions
- don't output aria warnings when using `aria-labelledby`
- require `md-input-container` when using links in checkbox labels

Fixes #11134.

BREAKING CHANGE: If you've created a custom solution to style links within `md-checkbox` labels, then you may need to remove or change that code now. This is because we automatically detect `<a>` tags in these labels and re-render them in an accessible way.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
- Breaking Change 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 P3: important Important issues that really should be fixed when possible. resolution: fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants