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

Remove aria-required-children for role="tree" #1444

Closed
robpaveza opened this issue Mar 18, 2019 · 5 comments
Closed

Remove aria-required-children for role="tree" #1444

robpaveza opened this issue Mar 18, 2019 · 5 comments
Assignees
Labels
fix Bug fixes rules Issue or false result from an axe-core rule support

Comments

@robpaveza
Copy link

Expectation:

<ol role="tree"></ol>

should generate no errors.

Actual: It generates the following error:

aria-required-children: Certain ARIA roles must contain particular children WCAG 1.3.1

Motivation: A tree that has no leaves is still a tree, and therefore role="treeitem" should not be a required child of an element of role="tree".

axe-core version: 3.0.0-alpha1, 3.1.1

@smhigley
Copy link
Contributor

We ran into this issue as well in a product that allows user interaction to insert tree items. The empty role="tree" exists before any tree items have been inserted.

There are examples of empty trees existing on desktop applications in Windows applications, so it's not a completely unknown pattern. An example might be in something like this: https://www.lightningdesignsystem.com/components/dueling-picklist/ where instead of listboxes there are selectible tree views, and all items are removed from one side.

@WilcoFiers
Copy link
Contributor

I agree. I think generally the aria-required-children rule isn't quite right and should probably be more of an "allowed children" type rule, similar to what we've done with the list rules.

Thank you for reporting. We'll definitely pick this up.

@WilcoFiers WilcoFiers added fix Bug fixes rules Issue or false result from an axe-core rule labels Mar 26, 2019
@WilcoFiers WilcoFiers self-assigned this Apr 17, 2019
@WilcoFiers WilcoFiers modified the milestone: Q2 2018 update Apr 17, 2019
@WilcoFiers WilcoFiers removed their assignment Apr 19, 2019
@aellsey aellsey added this to the HTMLTools Sprint 2 milestone Apr 21, 2019
@straker
Copy link
Contributor

straker commented Apr 30, 2019

@WilcoFiers how exactly would you like to handle this? It seems related to #874 and #383

@WilcoFiers WilcoFiers modified the milestones: HTMLTools Sprint 2, Axe-core 3.3 May 1, 2019
@WilcoFiers
Copy link
Contributor

@straker The aria-required-children check has an option for reviewEmpty, which when we put a role in there, puts the results into "needs review" rather than failing it. I think it should have the following items in it, so that they are put into needs review:

  • doc-bibliography
  • doc-endnotes
  • list
  • listbox
  • table
  • grid
  • treegrid
  • tree
  • group
  • rowgroup
  • tablist

That would make only the following roles fail if they are left empty:

  • menu
  • menubar
  • row

@chandana7393
Copy link

Verified for the required test file, not seeing any voilations, but observed "Needs review" issue.
(Attached the screenshot)
need_review_no_children
Tested Environment:
Axe-coconut - 3.8.0.14704v
Chrome - 74v
OS - Windows 10 64 bit

aarongable pushed a commit to chromium/chromium that referenced this issue Jun 28, 2019
…timeouts

By disabling the following rules we can expect tests to run ~4 seconds faster.
Tested with https://crrev.com/c/1635874.

Disabled slow rules:
- color-contrast
- image-redundant-alt

Disabled rules with bugs:
- aria-required-children, see: dequelabs/axe-core#1444

Disabled low value rules:
- meta-refresh
- blink
- audio-caption
- object-alt
- video-caption
- video-description
- radiogroup
- marquee
- html-has-lang
- html-lang-valid
- meta-viewport-large
- meta-viewport
- valid-lang

Any test may override this list to re-enable rules on specific UI.

Bug: 963183
Change-Id: I9094360c73c45168df4ac99aea0d659103950eb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638125
Commit-Queue: John Emau <[email protected]>
Reviewed-by: Joel Einbinder <[email protected]>
Cr-Commit-Position: refs/heads/master@{#673389}
devtools-bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Jun 28, 2019
…timeouts

By disabling the following rules we can expect tests to run ~4 seconds faster.
Tested with https://crrev.com/c/1635874.

Disabled slow rules:
- color-contrast
- image-redundant-alt

Disabled rules with bugs:
- aria-required-children, see: dequelabs/axe-core#1444

Disabled low value rules:
- meta-refresh
- blink
- audio-caption
- object-alt
- video-caption
- video-description
- radiogroup
- marquee
- html-has-lang
- html-lang-valid
- meta-viewport-large
- meta-viewport
- valid-lang

Any test may override this list to re-enable rules on specific UI.

Bug: 963183
Change-Id: I9094360c73c45168df4ac99aea0d659103950eb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638125
Commit-Queue: John Emau <[email protected]>
Reviewed-by: Joel Einbinder <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#673389}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 97b5560ae4622a82e2dddd8875dbb9bc6a6810f0
babot pushed a commit to binaryage/dirac that referenced this issue Jun 29, 2019
…timeouts

By disabling the following rules we can expect tests to run ~4 seconds faster.
Tested with https://crrev.com/c/1635874.

Disabled slow rules:
- color-contrast
- image-redundant-alt

Disabled rules with bugs:
- aria-required-children, see: dequelabs/axe-core#1444

Disabled low value rules:
- meta-refresh
- blink
- audio-caption
- object-alt
- video-caption
- video-description
- radiogroup
- marquee
- html-has-lang
- html-lang-valid
- meta-viewport-large
- meta-viewport
- valid-lang

Any test may override this list to re-enable rules on specific UI.

Bug: 963183
Change-Id: I9094360c73c45168df4ac99aea0d659103950eb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638125
Commit-Queue: John Emau <[email protected]>
Reviewed-by: Joel Einbinder <[email protected]>
Cr-Commit-Position: refs/heads/master@{#673389}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fixes rules Issue or false result from an axe-core rule support
Projects
None yet
Development

No branches or pull requests

8 participants