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

Permit ARIA 1.2 combobox (without textbox child) #2505

Closed
stevenyxu opened this issue Sep 2, 2020 · 15 comments
Closed

Permit ARIA 1.2 combobox (without textbox child) #2505

stevenyxu opened this issue Sep 2, 2020 · 15 comments
Assignees
Labels
feat New feature or enhancement pr A pr has been created for the issue rules Issue or false result from an axe-core rule
Milestone

Comments

@stevenyxu
Copy link

Expectation: A compliant ARIA 1.2 (draft) combobox should not raise axe violations.

Actual: ARIA 1.2 combobox raises aria-required-children violation. Demo: https://w3c.github.io/aria-practices/examples/combobox/combobox-select-only.html. Currently raises 1 violation, aria-required-children.

Motivation: ARIA 1.2 is redefining combobox (draft spec) to no longer require a textbox child because of various issues with the 1.1 combobox. Aside from resolving long-standing labelling and SR support issues, this is particularly useful for web applications that want to emulate a native select or provide a rich selection widget (e.g. a form select with a popup that includes multiselection, options filter, select all/none). The current common practice is to use a popup button as the trigger rather than a combobox, but this is problematic in a form setting because button has problems with aria-required (prohibited by spec, flagged by axe, though supported in practice) and aria-invalid (permitted by spec, but not supported in practice). I've tested the new pattern with combobox trigger on JAWS, NVDA, ChromeVox on ChromeOS, VoiceOver, iOS VoiceOver, and Android TalkBack, and they all correctly announce current value, requiredness, and invalidness. In light of the apparent widespread support and that it's in a draft spec, it would be nice to permit it in axe. A couple of projects I'm involved with including Angular Material have already cut over to the new pattern because of the clear a11y wins, and we're currently suppressing (in tooling) and manually ignoring (when using axe extension) the axe violation.


axe-core version: 3.5.5
axe-webdriver, extension or other integration version: 4.5.3

Browser and Assistive Technology versions

For Tooling issues:
- Platform:  Tested on Chrome 84 OS X, reproducible running axe-core as a standalone library on node v10.
@straker
Copy link
Contributor

straker commented Sep 3, 2020

Thanks for the issue. We've been discussing this very pattern over in #2478. The TL;DR is that we want to support the pattern, but we need to figure out what to do with the old 1.1 pattern first.

@stevenyxu
Copy link
Author

Thanks @straker. I took a look at #2478 and just want to understand the 1.1 blocker a bit better. Would it be feasible to support role=combobox aria-controls=some-popup-id now and exempt this node from having a required textbox and/or popup child? Thus axe permits the following (using listbox without option children for simplicity, but would extend to other popups):

  • ARIA 1.0 combobox <input role=combobox aria-owns=listbox-id><div id=listbox-id role=listbox></div>. Already OK.
  • ARIA 1.1 combobox <div role=combobox><input aria-controls=listbox-id><div id=listbox-id role=listbox></div>. Already OK.
  • ARIA 1.2 combobox <input role=combobox aria-controls=listbox-id><div id=listbox-id role=listbox></div> or for the <select> emulation use case <div role=combobox aria-controls=listbox-id></div><div id=listbox-id role=listbox></div>. Newly supported.

From what I can tell, adding support (in the form of not flagging as a violation) for the 1.2 pattern could be done independently of whether the 1.1 pattern should be sunset here. The practice of putting aria-controls on the combobox node itself is new compared to both 1.0 and 1.1. Is there some edge case that prevents this from being feasible?

@straker
Copy link
Contributor

straker commented Sep 11, 2020

The main blocker is that it will require special casing and an edge case of guessing.

First, which roles require owning other roles is dictated by our aira-roles standards object. In ARIA 1.0 and 1.1, the combobox is required to own a listbox and textbox (with 1.1 also allowing grid, tree, and dialog if used with aria-haspopup). However, in the 1.2 pattern the comboxbox is not required to own anything. We cannot support two different role specs at the same time very easily and we have to figure out what to do there. This is the main blocker and what we mean when we say we have to figure out what to do with the old 1.1 pattern.

Secondly, there is an edge case when the input has both aria-controls and aria-owns. Neither the 1.0 nor 1.2 pattern disallow this (the 1.2 pattern only says authors are strongly recommended to use aria-controls). Knowing which case we should flag that is will be difficult and potentially full of false positives.

If we flag it as a 1.0 pattern then we need to make sure it owns a listbox, if it doesn't we flag it as a missing required-child. But if the author intended the 1.2 pattern it should pass. If the author intended the 1.2 pattern, should we tell them to remove the aria-owns attribute? aria-owns is a global attribute so allowed anywhere but goes against the design of the pattern. There's a few more edge case things we need to figure out.

Hopefully that helps clarify the blocker. We have this planned to be fixed for our 4.1.0 release, so we should be working on it in the coming weeks.

@stevenyxu
Copy link
Author

Thanks for the discussion, @straker. That make sense to me. It's unfortunate the singular churn of combobox role makes a static mapping hard for axe. I can imagine creating a different ruleset for [role=combobox][aria-controls] and [role=combobox]:not([aria-controls]) but I'm sure there may be more graceful ways to handle this. Looking forward to seeing what your team comes up with here.

My vote regarding the aria-controls and aria-owns edge case would just be to steer them towards the clean 1.2 pattern, which appears to be widely supported from our testing both when the combobox is a plain popup trigger (in the <select> emulation case) and when it's a true input/contenteditable. If someone has done both atop the 1.0 pattern, it's as easy as removing aria-owns. If someone has done both atop the 1.1 pattern (unlikely because 1.1 already tells you to put aria-controls on the textbox inside), it's a bit more involved, but the mapping to the 1.2 pattern should still be straightforward. If someone has done both atop 1.2, then yeah removing aria-owns seems sensible and easy.

Anyway sounds like your team is on top of it, so thank you and let me know if you need any help or testing from our side.

@backwardok
Copy link

Given the poor support for the 1.1 combobox pattern, and the recommendation explicitly in 1.2 to not use the 1.1 pattern, could the combobox check for aria-required-children be removed? I also wonder if a patch version could be made for 3.x to address this issue without requiring updating to 4.x quite yet. In the interim, I have to disable this check altogether, which seems worse. Thanks!

@AutoSponge
Copy link
Contributor

AutoSponge commented Feb 11, 2021

For another example, here's a demo using angular material that gets flagged in axe: https://angular-ph3vth-e514sq.stackblitz.io/. In this example, the listbox is dynamically created (axe complains it doesn't have the right child) then continues to flag in the open state. I assume it's because the listbox isn't the first child (nested in a div). The listbox is directly linked with aria-controls.

If this example is accessible, then the rule aria-required-children is a false positive.

@WilcoFiers WilcoFiers modified the milestones: axe-core 4.2, axe-core 4.3 Feb 24, 2021
@smhigley
Copy link
Contributor

smhigley commented Mar 2, 2021

@straker / @WilcoFiers I did some testing, and I think axe wouldn't require any special casing or version detection to make this work well. I created a few combobox variations of the 1.1 and 1.2 patterns that mix and match having a child input and switching aria-controls for aria-owns: https://codepen.io/smhigley/full/zYoEMWP. (I left off 1.0, because if you switch aria-owns for aria-controls, it just becomes 1.2).

As you can see, there is no practical support problem with removing the child input for the 1.1 pattern, or leaving out aria-owns in favor of aria-controls in the 1.1 pattern. So, for all patterns, the following are true:

  • No pattern requires a child input
  • All patterns require one of aria-owns or aria-controls, but it doesn't matter which one

It seems like if axe removed the aria-required-children for combobox entirely, and added a check for either aria-owns or aria-controls regardless of pattern version, that should cover all cases.

This makes sense from a technical perspective: the thing doing the heavy lifting is aria-activedescendant, and it doesn't really matter if the combo/listbox association is done via aria-owns or aria-controls. And a 1.1 pattern without a child input is just a select-only 1.2 pattern with aria-owns :)

@straker
Copy link
Contributor

straker commented Mar 8, 2021

Thanks for all the amazing tests! I talked with @WilcoFiers and we agree that your suggestion seems like the best approach. We'll see about getting this into the 4.2 release.

@smhigley
Copy link
Contributor

smhigley commented Mar 9, 2021

Woot, thanks so much! 🎉

@WilcoFiers
Copy link
Contributor

Unfortunately, we had to postpone this one, will take this up in 4.3.

@straker
Copy link
Contributor

straker commented Jul 8, 2021

Just a heads up. This is going into the 4.3 release happening next week.

@padmavemulapati
Copy link

Validated with latest develop branch code base,
aria-required-children violation is showing on combobox , able to reproduce it with the old axe-core version(4.0.2)
test-script:

<div aria-controls=\"listbox1\" aria-expanded=\"false\" aria-haspopup=\"listbox\" aria-labelledby=\"combo1-label\" id=\"combo1\" class=\"combo-input\" role=\"combobox\" tabindex=\"0\">Choose a Fruit</div>

image

and now it got fixed:
not seeing violations on aria-required-children on combobox
image

@straker straker closed this as completed Jul 9, 2021
@vskh
Copy link

vskh commented Jul 17, 2021

Seems, this recent update might have changed some other aspects of combobox validation in a way that was not a problem before.

Suddenly, Fluent UI's dropdowns started to be highlighted with 'aria-required-children' rule based on missing 'aria-controls' attribute in collapsed state.

If I understand correctly the part of WAI-ARIA that the rule refers to in part applicable to combobox, it mentions that

When a combobox is expanded, authors MUST ensure it contains or owns an element that has a role of listbox, tree, grid, or dialog. This element is the combobox popup. When the combobox is expanded, authors MUST set aria-controls on the textbox element to a value that refers to the combobox popup element.

i.e. 'aria-controls' is only required when the combobox is in expanded state (which mentioned dropdown implementation complies with, as the listbox part is dynamically inserted and not present in the DOM before expansion). There are no other mentions of these attributes with regards to behavior in a collapsed state. But axe-core requires those attributes unconditionally.

I'd like to ask if my understanding is correct or am I missing something? Once confirmed, I'd be happy to work on a patch.

@straker
Copy link
Contributor

straker commented Jul 19, 2021

Thanks for letting us know. It does indeed look like we missed that as part of the implementation. Would you mind opening a new issue so we can track it?

@vskh
Copy link

vskh commented Jul 21, 2021

Ooops, sorry, did not see you've created one and opened #3091. Will close as a dupe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or enhancement pr A pr has been created for the issue rules Issue or false result from an axe-core rule
Projects
None yet
Development

No branches or pull requests

9 participants