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

fix(aria-allowed-role): add gridcell, separator, slider and treeitem to allowed roles of button element #4398

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

msereniti
Copy link
Contributor

Closes: #4397

@msereniti msereniti requested a review from a team as a code owner April 4, 2024 14:18
@CLAassistant
Copy link

CLAassistant commented Apr 4, 2024

CLA assistant check
All committers have signed the CLA.

straker
straker previously approved these changes Apr 4, 2024
@straker
Copy link
Contributor

straker commented Apr 4, 2024

Thanks for the pr. I noticed we're also missing roles of gridcell, separator, and treeitem. It makes sense to update them in one go, so if you're willing we could add them to this pr (with an update to the pr title to reflect that).

If not though I've gone ahead and approved the pr and we can merge as is as well

@straker
Copy link
Contributor

straker commented Apr 4, 2024

Awesome, thanks for updating those. There is a single failing test in https://github.com/dequelabs/axe-core/blob/develop/test/integration/rules/aria-allowed-role/aria-allowed-role.html#L222-L227 where we were failing a button with role=gridcell but that's allowed now. The fix would be to update the id to pass-button-role-gridcell and then update the companion json file to remove [#fail-button-role-gridcell] from the violations list and add the new [#pass-button-role-gridcell] to the passes list.

…treeitem` to allowed roles of button element
@msereniti
Copy link
Contributor Author

Looks like all the tests are passing now 👀

Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Reviewed for security

@straker straker changed the title fix(aria-allowed-role): added slider to allowed roles of button element fix(aria-allowed-role): add gridcell, separator, slider and treeitem to allowed roles of button element Apr 5, 2024
@straker straker merged commit 4788bf8 into dequelabs:develop Apr 5, 2024
20 of 22 checks passed
@msereniti msereniti deleted the patch-1 branch April 8, 2024 15:09
WilcoFiers added a commit that referenced this pull request May 6, 2024
###
[4.9.1](v4.9.0...v4.9.1)
(2024-05-06)

### Bug Fixes

- Prevent errors when loading axe in a page with prototype.js
- **aria-allowed-attr:** allow meter role allowed aria-\* attributes on
meter element
([#4435](#4435))
([7ac6392](7ac6392))
- **aria-allowed-role:** add gridcell, separator, slider and treeitem to
allowed roles of button element
([#4398](#4398))
([4788bf8](4788bf8))
- **aria-roles:** correct abstract roles (types) for
aria-roles([#4421](#4421))
- **aria-valid-attr-value:** aria-controls & aria-haspopup incomplete
([#4418](#4418))
- fix building axe-core translation files with region locales
([#4396](#4396))
([5c318f3](5c318f3)),
closes [#4388](#4388)
- **invalidrole:** allow upper and mixed case role names
([#4358](#4358))
([105016c](105016c)),
closes [#2695](#2695)
- **isVisibleOnScreen:** account for position: absolute elements inside
overflow container
([#4405](#4405))
([2940f6e](2940f6e)),
closes [#4016](#4016)
- **label-content-name-mismatch:** better dismiss and wysiwyg symbolic
text characters
([#4402](#4402))
- **region:** Decorative images ignored by region rule
([#4412](#4412))
- **target-size:** ignore descendant elements in shadow dom
([#4410](#4410))
([6091367](6091367))
- **target-size:** pass for element that has nearby elements that are
obscured ([#4422](#4422))
([3a90bb7](3a90bb7)),
closes [#4387](#4387)


This PR was opened by a robot 🤖 🎉 (And updated by @WilcoFiers
)
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 this pull request may close these issues.

Should slider role be allowed for button?
3 participants