dequelabs / axe-core

Accessibility engine for automated Web UI testing
https://www.deque.com/axe/
Mozilla Public License 2.0
5.89k stars 771 forks source link

fix(landmark-unique): follow spec, aside -> landmark #4469

Closed gabalafou closed 3 months ago

gabalafou commented 4 months ago

Update the landmark-unique rule matcher for aside elements so that they are treated as landmarks using the same criteria specified in Sections 3.4.8 and 3.4.9 of the HTML Accessibility API Mappings 1.0.

Closes: #4460

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

straker commented 4 months ago

Thanks for being willing to take on a pr for this. Unfortunately I don't think this is the correct place for the change.

We have a file implicit-html-roles that dictates what the role of an element is when it has no role attribute. In there we assign aside the role of complimentary. Instead what we need to do is figure is two things:

  1. Is the aside part of a sectioning content. We can determine this using the code from footer and header. If it's not part of a sectioning content then the function should return complementary
  2. Does the aside have an accessible name. Again, we can determine this by using the local hasAccessibleName function similar to how the from role works. If it does then return complementary, otherwise return null to signify no implicit role.

Fixing that should then fix the landmark unique issue as the aside will no longer be determined to have a complementary role.

gabalafou commented 4 months ago

Oh that's great. Putting this aside-complementary logic in the landmark-unique matcher felt off to me, but I was patterning off of what was already there. Given what you wrote, I think the isHeaderFooterLandmark() function is redundant. In fact, I just checked and if I comment it out, then run npm run build && npm run test, all the tests pass.

Would you like me to update this pull request?

straker commented 4 months ago

Yep, that'd be great.

straker commented 3 months ago

Awesome work! Thanks for the pr.

Reviewed for security.