Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions lib/rules/template-no-duplicate-landmark-elements.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
// This rule inspects aria-label / aria-labelledby before classifying a node
// as a landmark (see getLabel + the dynamic-label skip in GlimmerElementNode),
// so it can safely include `region` — it won't flag an unnamed <section> as
// a landmark duplicate. Use the full spec-listed 8-role set.
const { ALL_LANDMARK_ROLES: LANDMARK_ROLES } = require('../utils/landmark-roles');

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
Expand Down Expand Up @@ -33,18 +39,6 @@ module.exports = {
form: 'form',
};

// Landmark ARIA roles
const LANDMARK_ROLES = new Set([
'banner',
'complementary',
'contentinfo',
'form',
'main',
'navigation',
'region',
'search',
]);

// Sectioning elements that strip banner/contentinfo roles from header/footer
const SECTIONING_ELEMENTS = new Set(['article', 'aside', 'main', 'nav', 'section']);
const elementStack = [];
Expand Down
10 changes: 1 addition & 9 deletions lib/rules/template-no-nested-landmark.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
const LANDMARK_ROLES = new Set([
'banner',
'complementary',
'contentinfo',
'form',
'main',
'navigation',
'search',
]);
const { LANDMARK_ROLES } = require('../utils/landmark-roles');

const LANDMARK_ELEMENTS = new Set(['header', 'aside', 'footer', 'form', 'main', 'nav']);

Expand Down
19 changes: 7 additions & 12 deletions lib/rules/template-no-redundant-role.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { roles } = require('aria-query');
const { LANDMARK_ROLES } = require('../utils/landmark-roles');

const DEFAULT_CONFIG = {
checkAllHTMLElements: true,
Expand All @@ -19,17 +20,6 @@ function createErrorMessageAnyElement(element, role) {
return `Use of redundant or invalid role: ${role} on <${element}> detected.`;
}

// https://www.w3.org/TR/html-aria/#docconformance
const LANDMARK_ROLES = new Set([
'banner',
'main',
'complementary',
'search',
'form',
'navigation',
'contentinfo',
]);

const ALLOWED_ELEMENT_ROLES = [
{ name: 'nav', role: 'navigation' },
{ name: 'form', role: 'search' },
Expand Down Expand Up @@ -114,7 +104,12 @@ const ROLE_TO_ELEMENTS = {
navigation: ['nav'],
option: ['option'],
radio: ['input'],
region: ['section'],
// `region` is intentionally NOT mapped to `<section>` here. `<section>`
// only gets the `region` landmark role when it has an accessible name
// (aria-label / aria-labelledby / title); without one it has role
// `generic`. A static linter cannot verify accessible-name presence.
// Spec: https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/HTML5.html
// See #2694 where the same reasoning was applied to template-no-nested-landmark.
row: ['tr'],
rowgroup: ['tbody', 'tfoot', 'thead'],
rowheader: ['th'],
Expand Down
46 changes: 46 additions & 0 deletions lib/utils/landmark-roles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const { roles } = require('aria-query');

// Non-abstract landmark roles derived from aria-query's role taxonomy: any
// role whose superClass chain includes 'landmark', minus DPub-ARIA `doc-*`
// (which share that superClass but are outside this plugin's rules' scope —
// 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!


// The subset that's safe for static-linting rules to treat as landmarks
// without further verification. `region` is EXCLUDED because a static linter
// cannot determine at lint time whether the element has an accessible name
// (via aria-label / aria-labelledby / title), which is required for the
// `region` role to actually apply to a `<section>` per HTML-AAM.
//
// See PR #2694 for the full rationale and spec citation
// (https://www.w3.org/TR/html-aam-1.0/#el-section):
// `<section>` without an accessible name has role `generic`, not `region`.
//
// Most a11y rules that enumerate landmarks should use this subset.
// Rules that DO verify accessible names (e.g. template-no-duplicate-
// landmark-elements, which inspects aria-label / aria-labelledby before
// classifying a node as a landmark) should import ALL_LANDMARK_ROLES.
const LANDMARK_ROLES = new Set([...ALL_LANDMARK_ROLES].filter((role) => role !== 'region'));

function buildLandmarkRoleSet() {
const result = new Set();
for (const [role, def] of roles) {
if (def.abstract) {
continue;
}
if (role.startsWith('doc-')) {
continue;
}
const descendsFromLandmark = (def.superClass || []).some((chain) => chain.includes('landmark'));
if (descendsFromLandmark) {
result.add(role);
}
}
return result;
}

module.exports = { LANDMARK_ROLES, ALL_LANDMARK_ROLES };
2 changes: 1 addition & 1 deletion tests/lib/rules/template-no-nested-landmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ ruleTester.run('template-no-nested-landmark', rule, {

// `<section>` only gets the `region` landmark role when it has an accessible name
// (aria-label/aria-labelledby/title). Without one it has the generic role — see
// https://www.w3.org/TR/html-aam-1.0/#el-section
// https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/HTML5.html
// This rule does not inspect accessible names, so unnamed sections are excluded.
'<template><section><section>Content</section></section></template>',
// `role="region"` is the landmark role a named `<section>` gets. Nesting it is
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/template-no-redundant-role.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ ruleTester.run('template-no-redundant-role', rule, {
// <button> resolves to `tab` (non-redundant — button's implicit is
// `button`, not `tab`). WAI-ARIA §4.1 fallback-list semantics.
'<template><button role="tab button"></button></template>',
// `region` is a landmark role, but <div> has no implicit role — role="region"
// on a <div> is not redundant.
'<template><div role="region"></div></template>',
{
code: '<template><div role="region"></div></template>',
options: [{ checkAllHTMLElements: false }],
},
// Per #2694: <section> has implicit role `generic` when unnamed and
// `region` when it has an accessible name. role="region" on <section> is
// therefore not unconditionally redundant — #38 aligns with this.
'<template><section role="region" aria-label="Quick facts">...</section></template>',
],
invalid: [
{
Expand Down Expand Up @@ -223,6 +234,10 @@ hbsRuleTester.run('template-no-redundant-role', rule, {
// NOT redundant.
'<select role="listbox"></select>',
'<select role="listbox" size="1"></select>',
// Per #2694: <section> has implicit role `generic` when unnamed and
// `region` when it has an accessible name. role="region" on <section> is
// therefore not unconditionally redundant — #38 aligns with this.
'<section role="region" aria-label="Quick facts">...</section>',
],
invalid: [
{
Expand Down
61 changes: 61 additions & 0 deletions tests/lib/utils/landmark-roles-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
'use strict';

const { LANDMARK_ROLES, ALL_LANDMARK_ROLES } = require('../../../lib/utils/landmark-roles');

// The WAI-ARIA 1.2 §5.3.4 landmark roles known at authoring time. Asserting
// set-inclusion (not an exact-equality / size match) keeps the test robust
// to future aria-query updates that add non-abstract landmark roles — the
// derivation (non-abstract + superClass contains 'landmark' + not doc-*)
// is what we actually care about.
const KNOWN_LANDMARK_ROLES = [
'banner',
'complementary',
'contentinfo',
'form',
'main',
'navigation',
'region',
'search',
];

describe('ALL_LANDMARK_ROLES', () => {
it('includes every WAI-ARIA 1.2 §5.3.4 landmark role', () => {
for (const role of KNOWN_LANDMARK_ROLES) {
expect(ALL_LANDMARK_ROLES.has(role)).toBe(true);
}
// Floor, not ceiling — lets the set grow if aria-query adds new
// non-abstract landmark roles upstream.
expect(ALL_LANDMARK_ROLES.size).toBeGreaterThanOrEqual(KNOWN_LANDMARK_ROLES.length);
});

it('excludes DPub-ARIA doc-* roles', () => {
for (const role of ALL_LANDMARK_ROLES) {
expect(role.startsWith('doc-')).toBe(false);
}
});
});

describe('LANDMARK_ROLES (the statically-verifiable subset)', () => {
it('includes every known landmark role except region', () => {
for (const role of KNOWN_LANDMARK_ROLES) {
if (role === 'region') {
continue;
}
expect(LANDMARK_ROLES.has(role)).toBe(true);
}
// Floor, not ceiling — mirrors the ALL_LANDMARK_ROLES rationale, minus
// the deliberate region exclusion.
expect(LANDMARK_ROLES.size).toBeGreaterThanOrEqual(KNOWN_LANDMARK_ROLES.length - 1);
});

it('excludes region (cannot verify accessible-name presence statically)', () => {
expect(LANDMARK_ROLES.has('region')).toBe(false);
expect(ALL_LANDMARK_ROLES.has('region')).toBe(true);
});

it('excludes non-landmark roles (button, link, article)', () => {
expect(LANDMARK_ROLES.has('button')).toBe(false);
expect(LANDMARK_ROLES.has('link')).toBe(false);
expect(LANDMARK_ROLES.has('article')).toBe(false);
});
});
Loading