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.

Sign up for GitHub

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

Jump to bottom

well formed lists and ARIA #463

Closed
dylanb opened this issue Jul 31, 2017 · 5 comments
Closed

well formed lists and ARIA #463

dylanb opened this issue Jul 31, 2017 · 5 comments
Labels
rules Issue or false result from an axe-core rule

Comments

@dylanb
Copy link
Contributor

dylanb commented Jul 31, 2017

Question from a WorldSpace customer: why does aXe not find this?

<ul role=”menubar”>
<li>stuff</li>
</ul>

I would think this would violate either 1.3.1 Lists or 4.1.1 Well Formed, since the list item has been orphaned due to the changed nature of the <ul> element.
@marcysutton
Copy link
Contributor

It also sounds like a bug since menuitem, menuitemradio and menuitemcheckbox are required owned elements for menu, and therefore menubar (through subclassing).

@WilcoFiers thought menubar was exempt when we first brought this up in Slack. I'd argue the WAI-ARIA spec would benefit from required owned children being explicitly called out as it's possible ARIA interpretation caused us to write the aXe rule incorrectly. But it's some of the oldest code in the repo: edc7406

@WilcoFiers
Copy link
Contributor

I think there are actually two problems here. Firstly, ARIA is wrong, it should fail that, but secondly, even if that role was something else, the li should have caught that it's contained within a ul that has it's role changed.

What's strange about subclassing of owned items is, what happens if you set owned items on the superclass and the subclass? I suppose it's just a theoretical question, but it isn't immediately obvious what that would be, which is why I would have expected those aren't inherited. Turns out, I was quite wrong about that :P

@WilcoFiers WilcoFiers added the rules Issue or false result from an axe-core rule label Aug 9, 2017
AlmeroSteyn added a commit to AlmeroSteyn/axe-core that referenced this issue Sep 10, 2017
@AlmeroSteyn
fix: check for role changes in lists and list items
cf47512 
change function of dlitem, listitem, only-dlitems and only-listitems checks to take into account role changes to either parent elements or child list item elements. Previously changing the role of either elements did not trigger the correct symantic markup errors.

closes dequelabs#463
@AlmeroSteyn AlmeroSteyn mentioned this issue Sep 10, 2017
Closed
AlmeroSteyn added a commit to AlmeroSteyn/axe-core that referenced this issue Sep 10, 2017
@AlmeroSteyn
fix: check for required menuitem elements inside a menubar
b66cda9 
change the lookup table for `menubar` to own the `menuitem` role so that it can be checked as a required child. Previouslty it was possible to create a `menubar` with no `menuitems`.

related to dequelabs#463
@jeeyyy jeeyyy mentioned this issue Jun 8, 2018
Merged
4 tasks
@jeeyyy
Copy link
Contributor

jeeyyy commented Jun 8, 2018

PR - #949

@jeeyyy jeeyyy closed this as completed Jun 8, 2018
@WilcoFiers WilcoFiers reopened this Jul 2, 2018
@WilcoFiers
Copy link
Contributor

Reopened because the PR isn't merged yet.

WilcoFiers pushed a commit that referenced this issue Jul 18, 2018
@jeeyyy @WilcoFiers
fix(rule): update for lists, list-items to handle role changes. (#949)
3a8729b 
This rule updates list(items) to cater to role changes to either parent or child list elements.

This rule is a taken over PR from the community contribution (PR: #518 @AlmeroSteyn - thanks for the initial work).

Closes issue:
- #463

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
@jeeyyy
Copy link
Contributor

jeeyyy commented Jul 18, 2018

PR #949 is merged to develop. Closing.

@jeeyyy jeeyyy closed this as completed Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
rules Issue or false result from an axe-core rule
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dylanb @WilcoFiers @marcysutton @jeeyyy