Fix role computation of elements based on names#2145
Open
Jym77 wants to merge 8 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: bbf8039 The changes in this PR will be included in the next version bump. This PR includes changesets to release 76 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Author
|
!pr extract |
There was a problem hiding this comment.
Pull request overview
This PR updates Alfa’s implicit role computation for elements whose role depends on author-provided naming mechanisms (notably <section>, <aside> in sectioning scope, and <img alt="">), and simplifies affected rules by removing the previous workaround predicate in favor of a narrower <form>-specific landmark exclusion.
Changes:
- Adjust implicit role mapping in
@siteimprove/alfa-ariaso<section>/<aside>without an author-provided name resolve togeneric, and<img alt="">with an author-provided name resolve toimg(otherwise remain presentational/omitted). - Replace the broad
DOM.hasIncorrectRoleWithoutNameworkaround withDOM.isNotLandmarkWithoutNameto handle the special<form>landmark behavior. - Add/adjust tests and changesets to cover the updated role/landmark behaviors.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/alfa-rules/test/sia-r2/rule.spec.tsx | Adds rule coverage for <img alt=""> name edge cases impacting pass/fail outcomes. |
| packages/alfa-rules/src/sia-r57/rule.ts | Switches landmark filtering workaround to isNotLandmarkWithoutName (form-only). |
| packages/alfa-rules/src/sia-r56/rule.ts | Updates landmark grouping applicability to exclude unnamed <form> landmarks via new predicate. |
| packages/alfa-rules/src/sia-r55/rule.ts | Same as R56: filters out unnamed <form> landmarks using new predicate. |
| packages/alfa-rules/src/sia-r40/rule.ts | Removes the workaround reject, relying on corrected role computation for region. |
| packages/alfa-aria/test/tsconfig.json | Swaps removed predicate test for the new predicate test file. |
| packages/alfa-aria/test/role.spec.tsx | Adds role computation tests for named/unnamed <section>, scoped/unscoped <aside>, and alt-less named images. |
| packages/alfa-aria/test/dom/predicate/is-not-landmark-without-name.spec.tsx | Introduces tests for the new <form> landmark-exclusion predicate. |
| packages/alfa-aria/test/dom/predicate/has-incorrect-role-without-name.spec.tsx | Removes tests for the deprecated predicate. |
| packages/alfa-aria/src/tsconfig.json | Removes old predicate source file and adds the new predicate source file. |
| packages/alfa-aria/src/index.ts | Exposes DOM.isNotLandmarkWithoutName instead of DOM.hasIncorrectRoleWithoutName. |
| packages/alfa-aria/src/feature.ts | Implements name-dependent implicit role logic for <aside>, <section>, and <img alt="">. |
| packages/alfa-aria/src/dom/predicate/is-not-landmark-without-name.ts | Adds new predicate capturing “unnamed <form> is not a landmark” logic. |
| packages/alfa-aria/src/dom/predicate/has-incorrect-role-without-name.ts | Removes the broad workaround predicate implementation. |
| packages/alfa-aria/src/dom/dom.ts | Updates DOM predicate exports to match the new predicate set. |
| docs/review/api/alfa-aria.api.md | Updates API review surface to reflect predicate rename/removal. |
| .changeset/wacky-cats-stand.md | Declares the breaking predicate replacement in @siteimprove/alfa-aria. |
| .changeset/old-pillows-wash.md | Notes the fix for alt-less but named images being exposed as img. |
| .changeset/chubby-dolls-type.md | Notes the fix for <aside>/<section> name-dependent role computation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #298
Resolves #1757
<aside>and<section>without a name are correctly getting the generic role as default.<img alt="" />with a name are correctly getting the image role as default.Browsers do currently differ on the corner cases (empty or whitespace
aria-labelortitle,aria-labelledbyreferring to invalid or empty elements). We mostly follow what Chrome does as it is the most frequently used browser. Hopefully these behaviours will be clarified in the specifications or the WPT and browsers will align; at which point we should follow suite.<form>without a name are special. They still need to be treated as forms by Assistive Technologies (for interaction purpose, …) but are not treated as landmarks. The oldhasIncorrectRoleWithoutNamepredicate has been renamedisNotLandmarkWithoutNameand only handles the<form>case now.