Skip to content

[A11Y-12318] Add SIA-R120. - #2175

Merged
pooya-si merged 4 commits into
A11Y-12317from
A11Y-12318
Sep 23, 2026
Merged

pooya-si merged 4 commits into
A11Y-12317from
A11Y-12318

Conversation

@pooya-si

@pooya-si pooya-si commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Add R120 to identify whether a document includes elements with duplicate access keys.

Jira Ticket

https://siteimprove-wgs.atlassian.net/browse/A11Y-12318

@changeset-bot

changeset-bot Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: bd28ed6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 76 packages
Name Type
@siteimprove/alfa-rules Minor
@siteimprove/alfa-act Minor
@siteimprove/alfa-affine Minor
@siteimprove/alfa-applicative Minor
@siteimprove/alfa-aria Minor
@siteimprove/alfa-array Minor
@siteimprove/alfa-bits Minor
@siteimprove/alfa-branched Minor
@siteimprove/alfa-cache Minor
@siteimprove/alfa-callback Minor
@siteimprove/alfa-cascade Minor
@siteimprove/alfa-clone Minor
@siteimprove/alfa-collection Minor
@siteimprove/alfa-comparable Minor
@siteimprove/alfa-continuation Minor
@siteimprove/alfa-css-feature Minor
@siteimprove/alfa-css Minor
@siteimprove/alfa-device Minor
@siteimprove/alfa-dom Minor
@siteimprove/alfa-eaa Minor
@siteimprove/alfa-earl Minor
@siteimprove/alfa-either Minor
@siteimprove/alfa-emitter Minor
@siteimprove/alfa-encoding Minor
@siteimprove/alfa-equatable Minor
@siteimprove/alfa-flags Minor
@siteimprove/alfa-fnv Minor
@siteimprove/alfa-foldable Minor
@siteimprove/alfa-functor Minor
@siteimprove/alfa-generator Minor
@siteimprove/alfa-graph Minor
@siteimprove/alfa-hash Minor
@siteimprove/alfa-http Minor
@siteimprove/alfa-iana Minor
@siteimprove/alfa-iterable Minor
@siteimprove/alfa-json-ld Minor
@siteimprove/alfa-json Minor
@siteimprove/alfa-lazy Minor
@siteimprove/alfa-list Minor
@siteimprove/alfa-map Minor
@siteimprove/alfa-mapper Minor
@siteimprove/alfa-math Minor
@siteimprove/alfa-monad Minor
@siteimprove/alfa-network Minor
@siteimprove/alfa-option Minor
@siteimprove/alfa-painting-order Minor
@siteimprove/alfa-parser Minor
@siteimprove/alfa-performance Minor
@siteimprove/alfa-predicate Minor
@siteimprove/alfa-record Minor
@siteimprove/alfa-rectangle Minor
@siteimprove/alfa-reducer Minor
@siteimprove/alfa-refinement Minor
@siteimprove/alfa-result Minor
@siteimprove/alfa-rng Minor
@siteimprove/alfa-sarif Minor
@siteimprove/alfa-selective Minor
@siteimprove/alfa-selector Minor
@siteimprove/alfa-sequence Minor
@siteimprove/alfa-set Minor
@siteimprove/alfa-slice Minor
@siteimprove/alfa-string Minor
@siteimprove/alfa-style Minor
@siteimprove/alfa-table Minor
@siteimprove/alfa-test Minor
@siteimprove/alfa-thunk Minor
@siteimprove/alfa-time Minor
@siteimprove/alfa-toolchain Minor
@siteimprove/alfa-trampoline Minor
@siteimprove/alfa-tree Minor
@siteimprove/alfa-trilean Minor
@siteimprove/alfa-tuple Minor
@siteimprove/alfa-url Minor
@siteimprove/alfa-wcag Minor
@siteimprove/alfa-web Minor
@siteimprove/alfa-xpath Minor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Duplicate tokens and keys across separate documents can currently produce false failures.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds stable rule R120 to detect duplicate HTML access keys.

Changes:

  • Implements duplicate-key detection and diagnostics.
  • Registers R120 and adds comprehensive rule tests.
  • Adds a minor changeset.
File summaries
File Description
.changeset/lucky-keys-listen.md Documents R120.
packages/alfa-rules/src/rules.ts Exports R120.
packages/alfa-rules/src/sia-r120/rule.ts Implements the rule.
packages/alfa-rules/src/tsconfig.json Includes the rule source.
packages/alfa-rules/test/sia-r120/rule.spec.tsx Tests rule behavior.
packages/alfa-rules/test/tsconfig.json Includes the rule tests.
Review details

Suppressed comments (1)

packages/alfa-rules/src/sia-r120/rule.ts:76

  • Attribute.tokens() preserves repeated tokens, so accesskey="a a" inserts this same element twice into the array. The length > 1 check then reports that “more than one element” declares a, even though there is no other element. Deduplicate each element's normalized token sequence before building and querying the map.
    attribute.tokens().map((token) => token.toLowerCase()),
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

requirements: [BestPractice.of("accesskey-unique")],
tags: [Scope.Page, Stability.Stable],
evaluate({ device, document }) {
const elements = getElementDescendants(document, Node.fullTree)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot you mentioned "access keys are scoped to their individual Document." and that ", matching keys in a parent document and an <iframe> are incorrectly reported as competing". Please provide the link to online documentation where it has been mentioned that the keys in a parent document and an <iframe> must be unique with no overlap across.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I couldn’t find a source that explicitly says a parent document and an <iframe> may or may not reuse the same accesskey. The references I was relying on were the HTML Standard’s accesskey section and the browsing-context/document model:

So my earlier note was an inference from those sections, not a directly quoted requirement.

@pooya-si pooya-si changed the title [11Y-12318] Add R120. [11Y-12318] Add SIA-R120. Sep 22, 2026
@pooya-si pooya-si changed the title [11Y-12318] Add SIA-R120. [A11Y-12318] Add SIA-R120. Sep 22, 2026
@pooya-si
pooya-si marked this pull request as ready for review September 22, 2026 03:19
@pooya-si
pooya-si requested a review from a team as a code owner September 22, 2026 03:19
@Siteimprove Siteimprove deleted a comment from JoeCroninSI Sep 22, 2026
@pooya-si
pooya-si merged commit 6a57c6b into A11Y-12317 Sep 23, 2026
6 checks passed
@pooya-si
pooya-si deleted the A11Y-12318 branch September 23, 2026 21:30
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.

3 participants