Skip to content

refactor: extract landmark-roles util (preserving deliberate per-rule region exclusion)#2758

Merged
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:fix/landmark-roles-util
Apr 28, 2026
Merged

refactor: extract landmark-roles util (preserving deliberate per-rule region exclusion)#2758
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:fix/landmark-roles-util

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 27, 2026

Three a11y rules share a concept of "landmark ARIA role." Today they each maintain their own hand-list. The counts look like drift:

Rule Count Missing
template-no-redundant-role 7 region
template-no-nested-landmark 7 region
template-no-duplicate-landmark-elements 8

But the two 7-role lists aren't drift — the region exclusion is deliberate, documented, and was landed in #2694 after a "what's MDN say?" review. The reasoning:

<section> only gets the region landmark role when it has an accessible name (aria-label / aria-labelledby / title). Without one, <section> has role generic. A static linter cannot verify at lint time whether aria-label resolves to a non-empty name at runtime.

Source: WAI-ARIA APG HTML5 landmark examples.

The third rule (template-no-duplicate-landmark-elements) DOES include region because that rule inspects aria-label / aria-labelledby before classifying a node as a landmark — it can safely handle the full 8-role set.

What this PR does

Adds lib/utils/landmark-roles.js that makes the distinction explicit by exporting two sets:

  • ALL_LANDMARK_ROLES — the 8 WAI-ARIA 1.2 §5.3.4 landmark roles, derived from aria-query (non-abstract roles whose superClass chain includes 'landmark', minus DPub-ARIA doc-* roles).
  • LANDMARK_ROLES — the 7-role subset excluding region. The safe default for static-linting rules that can't verify accessible names.

Both sets are derived from aria-query's role metadata so future WAI-ARIA additions flow through on aria-query upgrade.

Rule migrations:

Also removes the region: ['section'] entry from template-no-redundant-role's ROLE_TO_ELEMENTS mapping, since #2694's reasoning applies to that rule too: <section role="region"> shouldn't be flagged as redundant when we can't verify the section has an accessible name.

Behavior vs. master

Spec references

// downstream callers can extend if needed). The exact size is intentionally
// not hard-coded: the derivation is what matters, so if aria-query adds a
// new non-abstract landmark upstream it will be picked up automatically.
const ALL_LANDMARK_ROLES = buildLandmarkRoleSet();
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.

why does this need a function now where it didn't before?

Copy link
Copy Markdown
Contributor Author

@johanrd johanrd Apr 28, 2026

Choose a reason for hiding this comment

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

The set is now computed by being derived by aria-query's role map rather than being hand-maintained. This aligns with other external-lib derivations and picks up any new landmark roles aria-query adds automatically.

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.

wouldn't importing it also do that? I don't understand

Copy link
Copy Markdown
Contributor Author

@johanrd johanrd Apr 28, 2026

Choose a reason for hiding this comment

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

aria-query doesn't expose a landmark-roles list as a top-level export. The landmark role only appears as a superClass entry inside the per-role definitions in roles. So to get "every non-abstract role that descends from landmark" you have to walk roles and filter on superClass, which is what buildLandmarkRoleSet() does. If aria-query ever adds a direct export we can swap to that.

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.

gotchya, thanks!

@NullVoxPopuli NullVoxPopuli merged commit d2d8c7f into ember-cli:master Apr 28, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants