Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VoiceOver doesn't read the Combobox.Option (single value selection) #2129

Closed
afercia opened this issue Dec 29, 2022 · 6 comments · Fixed by #2153
Closed

VoiceOver doesn't read the Combobox.Option (single value selection) #2129

afercia opened this issue Dec 29, 2022 · 6 comments · Fixed by #2153
Assignees

Comments

@afercia
Copy link
Contributor

afercia commented Dec 29, 2022

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.7.1

What browser are you using?

Safari and VoiceOver (macOS)

Reproduction URL

Demo page https://headlessui.com/react/combobox or run the playground and go to http://localhost:3000/combobox/combobox-with-pure-tailwind

Describe your issue

Hello. Thank you for making headlessui components as accessible as possible, much appreciated.

I read some related issues / PRs and it seems to me there's a bit of misunderstanding about the usage of aria-selected on the combobox listbox. See for example #1812, #1599, and #1243.

Safari and VoiceOver (macOS) don't read the Combobox.Option because the aria-selected usage in a combobox listbox is supposed to be a bit different from the one in a standard listbox. It's a subtle difference but in all the comboboxes I've helped to build in the past it always turned out that other screen readers (NVDA, JAWS) are happy with aria-activedescendant and do not care much about aria-selected. Instead, VoiceOver refuses to read the options if aria-selected is omitted or used incorrectly.

In the ARIA authoring practices, the example that is closest to the headlessui Combobox (single selection) is the Editable Combobox with List Autocomplete.

Quoting from the Listbox Popup Role, Property, State section of the W3C combobox example:

  • [aria-selected="true"] Specified on an option in the listbox when it is visually highlighted as selected.
  • [aria-selected="true"] Occurs only when an option in the list is referenced by aria-activedescendant.

That is: in this pattern, aria-selected needs to be only set on the currently 'highlighted' option. It doesn't have anything to do with the 'selection' state, which is a concept that is extraneous to the W3C combobox pattern.

This is also mentioned in the main page of the Combobox pattern: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/#wai-aria-roles-states-and-properties-6

For a combobox that controls a listbox, grid, or tree popup, when a suggested value is visually indicated as the currently selected value, the option, gridcell, row, or treeitem containing that value has aria-selected set to true.

This is different from the (stand alone) Listbox pattern, where aria-selected is indeed used for the selection state.

In short: in the Combobox Listbox, aria-selected should be based on the active prop rather than the selected prop. Of course, the multiselectable pattern needs to be handled differently as well as the stand-alone Listbox.

Note: in #1599 (comment) it is mentioned that this woudl make VoiceOver announce every active option as selected. While slightly annoying, this is up to the screen reader implementation and it's in a way the expected behavior. Other screen readers opted to not announce selected, for example NVDA doesn't, but again this is a vendor's choice. Regardless, aria-selected=true needs to be set only on the highlighted option referenced by aria-activedescendant.

To reproduce the bug:

  • Go to the Combobox demo page https://headlessui.com/react/combobox or run the playground and go to http://localhost:3000/combobox/combobox-with-pure-tailwind
  • Use Safari and VoiceOver (macOS).
  • Note: to make sure VoiceOver runs with default preferences:
    • Open VoiceOver Utility.
    • In the macOS menu bar, select File > Reset All VoiceOver Preferences... then press Reset in the confirmation dialog.
  • Focus the combobox input field.
  • Press the Down Arrow key to open the listbox.
  • Press the Down Arrow key to navigate through the options.
  • Observe that VoiceOver doesn't read out the options.
  • Clear the current value and type 'co'.
  • Press the Down Arrow key to navigate through the options.
  • Observe that VoiceOver doesn't read out the options.
  • Clear the current value and enter another string, delete some characters, add new ones, etc..
  • Press the Down Arrow key to navigate through the options.
  • Observe that VoiceOver doesn't read out the options.

W3C example:

@afercia
Copy link
Contributor Author

afercia commented Dec 29, 2022

I'm attaching a few animated GIFs to better illustrate. Although there's no audio, you can see what VoiceOver reads in the VoiceOver caption panel.

W3C example of a Combobox list box (no multi selection). See aria-selected="true" is set only on the highlighted option and omitted from all the other options:

combobox-list

Demo page: options aren't read out:

bug

W3C demo: options are read out:

w3

Playground page running locally, where I changed aria-selected to be based on active ? true : undefined rather than on the selected prop. Of course this is just a quick try for testing purposes. A complete fix would need more work. With this simple change in place VoiceOver now reads out the option:

fix

@afercia
Copy link
Contributor Author

afercia commented Dec 29, 2022

For reference: See some tests data on a11ysupport.io:
https://a11ysupport.io/tests/apg__aria-1-2-combobox-with-list-autocomplete-example#sr-feature-index-5
where the announcement of 'selected' is considered the expected behavior.
VoiceOver and Narrator do announce selected and are marked as green. All other tested implementation opted to not announce it.

@afercia
Copy link
Contributor Author

afercia commented Dec 29, 2022

Worth mentioning that to fully meet the ARIA pattern, there are a couple things to improve:

  • The input misses aria-autocomplete="list" attribute. This attribute gives assistive technology a hint about the kind of autocompletion in use.
  • aria-activedescendant is set also on the listbox. In the combobox listbox this is incorrect: it is supposed to be set only on the input. Again, this is different from the stand-alone Listbox pattern.

@afercia
Copy link
Contributor Author

afercia commented Dec 29, 2022

There's another thing to mention about VoiceOver: it is known to fail anyways when the combobox has no value, or at least no value when the listbox is open. See the 'partial' support for the aria-activedescendant attribute: https://a11ysupport.io/tests/apg__aria-1-2-combobox-with-list-autocomplete-example#sr-feature-index-0

This is really a VoiceOver bug.

In my testing, there is a way to fix it but it would require some manipulation of the input value, which is sort-of a DOM method. I do realize such a fix wouldn't be entirely desirable in this kind of components. The fox would be very small though and could live in a sort of separated DOM utils. I'm mentioning it in case uyou may want to consider it.

Basically, VoiceOver fails when the Listbox opens and the input has no value. This can be reproduced on the W3C example at https://www.w3.org/WAI/ARIA/apg/example-index/combobox/combobox-autocomplete-list.html

  • Do not enter any value in the input.
  • Press Down Arrow key to open the listbox.
  • Navigate through the options by using the Down/Up Arrow keys.
  • The options are not read out.
  • Enter some value in the input field.
  • Navigate through the options by using the Down/Up Arrow keys.
  • The options are now read out.

In the headlessui demo page at https://headlessui.com/react/combobox

  • Even if aria-selected gets fixed as described above, this but will still bite.
  • Even when the input field has an initial value.

I found that 'clearing' and repopulating the input value on the fly when the listbox opens makes VoiceOver behave way better. In the OpenCombobox method, before declaring and assigning activeOptionIndex, something along these lines woul dwork:

    let input = state.dataRef.current.inputRef.current;
    let inputValue = input.value;
    input.value = '';
    input.value = inputValue;

Maybe coded a bit better and elegantly than an a11y specialist like me is able to do 🙂

As said, I do realize this is sort of DOM manipulation that shouldn't live in a component. Also, it's a workaround to fix a specific screen reader bug. I doubt Apple will ever fix this upstream so maybe worth considering it. Keeping it in a separate DOM utility method may alleviate the impact on the code cleanliness.

@afercia
Copy link
Contributor Author

afercia commented Dec 29, 2022

I apologize because this is a real TL;DR. A very long reading. Also, I'm afraid I won't have time to contribute with a pull request but do please ping me if any help is needed.
Thanks for your patience and an early Happy New Year everyone! 🎉

@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

This is a very in-depth bug report and I really appreciate that! I will dig in the first thing on Monday in the New Year. Thanks again!

@RobinMalfait RobinMalfait self-assigned this Jan 5, 2023
RobinMalfait added a commit that referenced this issue Jan 5, 2023
This PR improves the Combobox because there are some differences
compared to the Listbox that we didn't properly account for.

Big thanks to @afercia for providing us with a wonderful issue report
that talks about the differences.

This commit fixes a few issues:

- The `aria-selected` value was always based on the `selected` value
  (which is correct for the `Listbox`) but it should be based on the
  visually selected value (in our case, this is the `active` value).
- We dropped an incorrect `aria-activedescendant` attribute from the
  `ComboboxOptions`, it should only exist on the `ComboboxInput`.
- We added a missing `aria-autocomplete` to the `ComboboxInput`, with a
  value of `list` so that assistive technology better understands what
  kind of Combobox we are dealing with.
- This also includes a VoiceOver bug fix where the announcements aren't
  properly happening unless a change to the `ComboboxInput` is made.
  Now we will trigger a change to the input when we open the `Combobox`,
  we will set the value to `''` and re-set to whatever value was
  present. We also make sure that the cursor position / selected range
  is reset so that the end user doesn't feel any differences (except for
  a better working VoiceOver on macOS).

Fixes: #2129
Co-authored-by: Andrea Fercia <a.fercia@gmail.com>
RobinMalfait added a commit that referenced this issue Jan 24, 2023
* add the `aria-autocomplete` attribute

* drop the `aria-activedescendant` attribute on the `Combobox.Options` component

It is only required on the `Combobox.Input` component.

* improve triggering VoiceOver when opening the `Combobox`

We do this by mutating the `input` value for a split second to trigger a
change that VoiceOver will pick up. We will also ensure to restore the
value and the selection / cursor position so that the end user won't
notice a difference at all.

* update changelog

Fixes: #2129
Co-authored-by: Andrea Fercia <a.fercia@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants