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

roles that allow aria-expanded and aria-roles.js question #3339

Closed
scottaohara opened this issue Jan 8, 2022 · 2 comments · Fixed by #4110 or organich/lighthouse#4 · May be fixed by abdulrahman305/docs#4, Stanislav1975/sitespeed.io#4 or Stanislav1975/sitespeed.io#11
Closed

roles that allow aria-expanded and aria-roles.js question #3339

scottaohara opened this issue Jan 8, 2022 · 2 comments · Fixed by #4110 or organich/lighthouse#4 · May be fixed by abdulrahman305/docs#4, Stanislav1975/sitespeed.io#4 or Stanislav1975/sitespeed.io#11
Assignees
@WilcoFiers
Labels
feat New feature or enhancement standards Issues in the ARIA standards objects (lib/standards)
Milestone
Axe-core 4.8

Comments

@scottaohara
Copy link
Contributor

scottaohara commented Jan 8, 2022 •
edited
Loading

ARIA 1.2 has fixed an error where ARIA 1.1 allowed aria-expanded on a number of elements that didn't make sense. e.g., role=alert aria-expanded=.... ARIA issue where this topic was discussed - fixed in 2019, so has been part of the initial and current ARIA 1.2 CR draft.

I noticed in the aria-roles.js file that aria-expanded is still marked as an allowed attribute on these roles, which at first made sense to me because the file notes on the first line that it is referencing ARIA 1.1... so since they were allowed then, if axe is still referencing ARIA 1.1 then they would still be allowed, though their use by authors is suspect / probably incorrect.

However, upon further review of the file, there are a number of roles that were introduces in ARIA 1.2 that are also included... which again also makes sense as many of them have been implemented by browsers... but this then made me wonder if it would be a good idea for axe to also correct the aria-expanded allowances, even though the correction is tied to ARIA 1.2.

Regardless of when axe might want to pull in the correction, I'd be happy to create a PR to remove the aria-expanded allowances from the necessary roles in this file.
@WilcoFiers
Copy link
Contributor

Thanks for the update Scott. It's a little late for 4.4, but putting it on the list for 4.5.

@WilcoFiers WilcoFiers added this to the Axe-core 4.5 milestone Jan 10, 2022
@WilcoFiers WilcoFiers added fix Bug fixes standards Issues in the ARIA standards objects (lib/standards) feat New feature or enhancement and removed fix Bug fixes labels Jan 10, 2022
scottaohara added a commit to scottaohara/axe-core that referenced this issue Jan 10, 2022
@scottaohara
update role allowances for aria-expanded
1223a35 
All roles reviewed to largely remove (but in a few cases add) `aria-expanded` as a supported attribute.  

This matches both work done in ARIA 1.2 to correct for roles that should not have allowed the attribute, while also incorporating some ARIA 1.2 updates where some roles had support for the attribute added.

closes dequelabs#3339
@scottaohara scottaohara mentioned this issue Jan 10, 2022
Closed
@WilcoFiers WilcoFiers modified the milestones: Axe-core 4.5, Axe-core 4.6 May 20, 2022
@straker straker modified the milestones: Axe-core 4.7, Axe-core 4.8 Mar 22, 2023
@WilcoFiers WilcoFiers self-assigned this Jul 28, 2023
@WilcoFiers WilcoFiers mentioned this issue Jul 31, 2023
Merged
WilcoFiers added a commit that referenced this issue Aug 8, 2023
@WilcoFiers @scottaohara @straker
fix(aria-allowed-attr): pass aria-expanded on checkbox & switch (#4110)
fcf76e0 
* update role allowances for aria-expanded

All roles reviewed to largely remove (but in a few cases add) `aria-expanded` as a supported attribute.  

This matches both work done in ARIA 1.2 to correct for roles that should not have allowed the attribute, while also incorporating some ARIA 1.2 updates where some roles had support for the attribute added.

closes #3339

* fix(aria-allowed-attr): allow aria-expanded on more roles

* Integration tests

* Update lib/standards/aria-roles.js

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

* Tweak comments

---------

Co-authored-by: Scott O'Hara <scottaohara@users.noreply.github.com>
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
@padmavemulapati
Copy link

Validated using axe-core develop branch code base,
Test snippet:

<body>
    <button id="alert-button" aria-expanded="true" aria-controls="alert-message">
        Show Alert
    </button>

    <div id="alert-message" role="alert" >
        This is an important alert message.
    </div>

Not seeing any failure even aria-expanded make it false or even after removed completely

Environment:

Label Value
Product axe-core
Version 4.7.2-canary.49eaa0e
OS-Details _MAC- Intel Core i7 - 11.6.8 _
BrowserDetails Chrome Version 1115.0.5790.114 (Official Build) (64-bit)

@attest-team-ci attest-team-ci mentioned this issue Sep 6, 2023
Merged
@abdulrahman305 abdulrahman305 mentioned this issue Jul 3, 2024
Open
@Stanislav1975 Stanislav1975 mentioned this issue Aug 9, 2024
Open
@Stanislav1975 Stanislav1975 mentioned this issue Aug 19, 2024
Open
@organich organich mentioned this issue Sep 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@WilcoFiers WilcoFiers

Labels
feat New feature or enhancement standards Issues in the ARIA standards objects (lib/standards)
Projects
None yet
Milestone
Axe-core 4.8
4 participants
@WilcoFiers @straker @scottaohara @padmavemulapati