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

Conversation

Splaktar
Copy link
Contributor

@Splaktar Splaktar commented Sep 11, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

When selecting a value from an auto-complete, Chromevox reads it out as "{md-autocomplete aria-label} {Value} collapsed combo box". It's unclear to the user which part of the text read is the combo-box-name, and which is the value.

Issue Number:
Fixes #10838

What is the new behavior?

With the updates Chromevox will announce the following:
"{md-autocomplete aria-label} {Value} selected Collapsed Combo box"

If required:
"{md-autocomplete aria-label} {Value} selected Collapsed Required Combo box"

  • implement WAI-ARIA 1.0 recommendation for autocomplete
  • avoid changing listbox selection or announcing changes when using arrow keys when a selection has been made and the listbox is closed
  • set aria-setsize and aria-posinset for dropdown options

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Tested on Chromevox for ChromeOS 69 and VoiceOver for macOS High Sierra and Mojave.

@Splaktar Splaktar added type: bug a11y This issue is related to accessibility browser: Chrome g3: reported The issue was reported by an internal or external product team. P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Sep 11, 2018
@Splaktar Splaktar added this to the 1.1.11 milestone Sep 11, 2018
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Sep 11, 2018
@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Sep 11, 2018
@Splaktar
Copy link
Contributor Author

Splaktar commented Sep 17, 2018

Please presubmit this, but do not merge yet.

It looks like this might break JAWS 17 (from 2015) support. I'm waiting to hear back if this works with JAWS 2018.
I'm waiting to hear back whether this improves or hurts VoiceOver iOS support.

That testing is happening in #10970.

@Splaktar Splaktar assigned mmalerba and unassigned andrewseguin Sep 19, 2018
@mmalerba
Copy link
Contributor

It looks like this causes some AX errors:

Errors:
Error: AX_ARIA_04 (ARIA state and property values must be valid) failed on the following elements (1 - 2 of 2):
.md-show-clear-button
#input-16
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_04 for more information.

Error: AX_ARIA_01 (Elements with ARIA roles must use a valid, non-abstract ARIA role) failed on the following element:
#input-16
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_01 for more information.

Error: AX_ARIA_02 (ARIA attributes which refer to other elements by ID should refer to elements which exist in the DOM) failed on the following element:
#input-16
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_02 for more information.


Warnings:
Warning: AX_ARIA_06 (aria-owns should not be used if ownership is implicit in the DOM) failed on the following element:
.md-show-clear-button
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_06 for more information.

@Splaktar Splaktar added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs pr: presubmit-failures and removed pr: merge ready This PR is ready for a caretaker to review labels Sep 19, 2018
@Splaktar
Copy link
Contributor Author

For AX_ARIA_06, removed aria-owns from md-autocomplete-wrap.

For AX_ARIA_01, this is because searchbox does not exist in WAI-ARIA 1.0 and the GoogleChrome/accessibility-developer-tools only support 1.0, not 1.1. I've changed it to textbox which did exist in 1.0.

For AX_ARIA_04 on input, I think that I've solved this by only setting aria-activedescendant when a matching option has the selected_option id.

For AX_ARIA_04 on .md-show-clear-button, this appears to be caused by aria-haspopup only allowing true or false in 1.0, where listbox is a valid value in 1.1. I've set this to true now.

The whole GoogleChrome/accessibility-developer-tools project actually appears to be unmaintained and abandoned at this time (no commits since Nov 2017). It looks like Lighthouse and the Google Chrome audits are using aXe instead.

I'm not sure what the AX_ARIA_02 error is referring to as aria-labelledby is not used in the demos. However, it's not clear if this audit was run against the demo site or some other app.

@mmalerba
Copy link
Contributor

I think it was run against some other app. If you push the updated code I'll re-run the presubmit

@Splaktar Splaktar changed the title feat(autocomplete): upgrade to WAI-ARIA 1.1 combobox guidelines fix(autocomplete): screen reader indicates selected option on focus Sep 19, 2018
@Splaktar Splaktar changed the title fix(autocomplete): screen reader indicates selected option on focus fix(autocomplete): Chromevox indicates selected option on focus Sep 19, 2018
@Splaktar Splaktar force-pushed the updateAutocompleteToAria1.1 branch from 9f425e8 to 95ad50f Compare September 19, 2018 23:32
@Splaktar
Copy link
Contributor Author

OK, I've pushed an update. This implementation is a bit of a blend of WAI-ARIA 1.0 and 1.1 combobox with the 1.1 pieces trying to be done in a 1.0 compatible way. It still fixes the original Chromevox issue and works reasonably well with macOS VoiceOver. I'll ask over in #10970 for a re-test on JAWS and iOS VoiceOver.

@Splaktar
Copy link
Contributor Author

I also tested this with Lighthouse and got a score of 86 on the autocomplete demo page. All of the issues are from the docs site itself and not the autocomplete elements. I'm going to open another PR for 1.1.12 to fix the docs site a11y issues.

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs pr: presubmit-failures labels Sep 19, 2018
@mmalerba
Copy link
Contributor

I'm still seeing a couple AX_ARIA_02 and AX_ARIA_04 errors. It might be that the test just needs to be modified somehow

@Splaktar Splaktar added the Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. label Sep 21, 2018
@Splaktar Splaktar added pr: presubmit-failures and removed pr: merge ready This PR is ready for a caretaker to review labels Sep 21, 2018
@Splaktar
Copy link
Contributor Author

@mmalerba I'll need more information about the use case and errors here in order to be able to move forward.

@Splaktar Splaktar modified the milestones: 1.1.11, 1.1.12 Sep 24, 2018
@Splaktar Splaktar assigned jelbourn and unassigned mmalerba Oct 30, 2018
@Splaktar Splaktar force-pushed the updateAutocompleteToAria1.1 branch from 95ad50f to ad1f0de Compare October 30, 2018 19:24
@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Oct 30, 2018
@Splaktar
Copy link
Contributor Author

After getting PR #11475 merged, our autocomplete demo page has a Lighthouse a11y score of 87. With the changes in this branch, it gets up to 89.

We'll need these two in to get 100:

@Splaktar Splaktar modified the milestones: 1.1.12, 1.1.11 Nov 2, 2018
@Splaktar Splaktar force-pushed the updateAutocompleteToAria1.1 branch 3 times, most recently from 65deebb to 92b3790 Compare November 2, 2018 02:59
@Splaktar
Copy link
Contributor Author

Splaktar commented Nov 2, 2018

I did another round of testing and made some updates to completely align with the Legacy ARIA 1.0 Combobox With List Autocomplete Example instead of the ARIA 1.1 Combobox with Listbox Popup Example as screen reader support for WAI-ARIA 1.1 is very minimal today.

- implement WAI-ARIA 1.0 recommendation for autocomplete
- avoid changing listbox selection or announcing changes when
 using arrow keys when a selection has been made and the listbox is
 closed
- set aria-setsize and aria-posinset for dropdown options

Fixes #10838. Relates to #10970.
@Splaktar Splaktar force-pushed the updateAutocompleteToAria1.1 branch from 92b3790 to 8bd23f8 Compare November 2, 2018 03:29
@Splaktar Splaktar removed Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. pr: presubmit-failures labels Nov 2, 2018
@jelbourn jelbourn merged commit ca10bd0 into master Nov 6, 2018
@Splaktar Splaktar deleted the updateAutocompleteToAria1.1 branch November 7, 2018 18:38
marosoft pushed a commit to marosoft/material that referenced this pull request Nov 11, 2018
…lar#11441)

- implement WAI-ARIA 1.0 recommendation for autocomplete
- avoid changing listbox selection or announcing changes when
 using arrow keys when a selection has been made and the listbox is
 closed
- set aria-setsize and aria-posinset for dropdown options

Fixes angular#10838. Relates to angular#10970.
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 browser: Chrome cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ g3: reported The issue was reported by an internal or external product team. P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autocomplete: screen reader should differentiate between the name of the combo box, and the selected option
5 participants