Skip to content

fix(landmark-unique): exclude section/form with non-landmark roles from landmark match #5085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nami8824 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix(landmark-unique): exclude section/form with non-landmark roles from landmark match #5085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
342d816
fix(landmark-unique): exclude section/form with non-landmark roles fr…
nami8824 Apr 28, 2026
0afa4bd
fix(landmark-unique): match section/form with explicit landmark role …
nami8824 Apr 29, 2026
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
24 changes: 3 additions & 21 deletions lib/rules/landmark-unique-matches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,9 @@
import { getRoleType } from '../commons/aria';
import { isVisibleToScreenReaders } from '../commons/dom';
import { getRole } from '../commons/aria';
import { getAriaRolesByType } from '../commons/standards';
import { accessibleTextVirtual } from '../commons/text';

export default function landmarkUniqueMatches(node, virtualNode) {
return (
isLandmarkVirtual(virtualNode) && isVisibleToScreenReaders(virtualNode)
getRoleType(virtualNode) === 'landmark' &&
isVisibleToScreenReaders(virtualNode)
);
}

function isLandmarkVirtual(vNode) {
const landmarkRoles = getAriaRolesByType('landmark');
const role = getRole(vNode);
if (!role) {
return false;
}

const { nodeName } = vNode.props;

if (nodeName === 'section' || nodeName === 'form') {
const accessibleText = accessibleTextVirtual(vNode);
return !!accessibleText;
}

return landmarkRoles.indexOf(role) >= 0 || role === 'region';
}
15 changes: 13 additions & 2 deletions test/integration/rules/landmark-unique/landmark-unique-fail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@
<form id="violation-form-aria-label-1" aria-label="form-label"></form>
<form id="violation-form-aria-label-2" aria-label="form-label"></form>

<form
id="violation-form-search-1"
role="search"
aria-label="form-search-label"
></form>
<form
id="violation-form-search-2"
role="search"
aria-label="form-search-label"
></form>

<div id="form-label-1">form-with-label</div>
<div id="form-label-2">form-with-label</div>
<form
Expand Down Expand Up @@ -87,8 +98,8 @@
<div id="violation-role-region" role="region"></div>
<div id="violation-role-region-2" role="region"></div>

<div id="violation-role-search" role="search"></div>
<div id="violation-role-search-2" role="search"></div>
<form id="violation-role-search" role="search"></form>
<form id="violation-role-search-2" role="search"></form>

@nami8824 nami8824 Apr 30, 2026

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.

Ideally, I wanted to add new test cases for <section>/<form> with a landmark role but no aria-label. However, all landmark roles were already used in the no-label section of this file, so there was no available role to use for new elements. Instead, I replaced the existing <div role="search"> with <form role="search">.

<nav id="violation-nav" aria-label="duplicate label for nav"></nav>
<nav id="violation-nav-2" aria-label="duplicate label for nav"></nav>
Expand Down
1 change: 1 addition & 0 deletions test/integration/rules/landmark-unique/landmark-unique-fail.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
["#violation-main-1"],
["#violation-header-1"],
["#violation-form-aria-label-1"],
["#violation-form-search-1"],
["#violation-form-aria-labelledby-1"],
["#violation-aside-aria-label-1"],
["#violation-aside-aria-labelledby-1"],
Expand Down
11 changes: 11 additions & 0 deletions test/integration/rules/landmark-unique/landmark-unique-pass.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@
<form id="pass-form-aria-label-1" aria-label="form-label-1"></form>
<form id="pass-form-aria-label-2" aria-label="form-label-2"></form>

<form
id="pass-form-search-aria-label-1"
role="search"
aria-label="form-search-label-1"
></form>
<form
id="pass-form-search-aria-label-2"
role="search"
aria-label="form-search-label-2"
></form>
Comment on lines +8 to +17

@WilcoFiers WilcoFiers Apr 30, 2026

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.

This is the false positive of the original issue. Good to have that in the E2E suite.

Suggested change
<form
id="pass-form-search-aria-label-1"
role="search"
aria-label="form-search-label-1"
></form>
<form
id="pass-form-search-aria-label-2"
role="search"
aria-label="form-search-label-2"
></form>
<section
id="pass-section-tabpanel-aria-label-repeat-1"
role="tabpanel"
aria-label="My tab panel"
></section>
<section
id="pass-section-tabpanel-aria-label-repeat-2"
role="tabpanel"
aria-label="My tab panel"
></section>


<div id="form-label-1">form-with-label-1</div>
<div id="form-label-2">form-with-label-2</div>
<form id="pass-form-aria-labelledby-1" aria-labelledby="form-label-1"></form>
Expand Down
2 changes: 2 additions & 0 deletions test/integration/rules/landmark-unique/landmark-unique-pass.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
["#pass-header"],
["#pass-form-aria-label-1"],
["#pass-form-aria-label-2"],
["#pass-form-search-aria-label-1"],
["#pass-form-search-aria-label-2"],
Comment on lines +9 to +10

@WilcoFiers WilcoFiers Apr 30, 2026

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.

Suggested change
["#pass-form-search-aria-label-1"],
["#pass-form-search-aria-label-2"],
["#pass-section-tabpanel-aria-label-repeat-1"],
["#pass-section-tabpanel-aria-label-repeat-2"],

@nami8824 nami8824 Apr 30, 2026

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.

Thanks for the suggestion. I tried applying the suggested JSON change by adding these selectors, but the integration test fails with "Element not found".
If I add the suggested elements only to landmark-unique-pass.html, without listing them in landmark-unique-pass.json, the test passes. Would you prefer that approach?
If the rule-matches test already verifies that these elements are not matched, I think we may not need additional integration coverage for this case.

Also, I originally added the form + role="search" + aria-label cases because I did not see existing integration coverage for form/section + accessible name + explicit landmark role passing. I also added similar coverage in test/rule-matches/landmark-unique-matches.js. If that coverage is not useful here, I can remove those cases.

["#pass-form-aria-labelledby-1"],
["#pass-form-aria-labelledby-2"],
["#pass-aside-aria-label-1"],
Expand Down
70 changes: 69 additions & 1 deletion test/rule-matches/landmark-unique-matches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('landmark-unique-matches', function () {
assert.isFalse(rule.matches(node, virtualNode));
});

describe('form and section elements must have accessible names to be matched', function () {
describe('form and section elements match as landmarks conditionally based on accessible name and role', function () {
const sectionFormElements = ['section', 'form'];

sectionFormElements.forEach(function (elementType) {
Expand All @@ -61,6 +61,54 @@ describe('landmark-unique-matches', function () {
}
);

it(
'should match because it is a ' +
elementType +
' with an explicit landmark role and a label',
function () {
const explicitRole =
elementType === 'section' ? 'navigation' : 'search';
axeFixtureSetup(
'<' +
elementType +
' role="' +
explicitRole +
'" aria-label="sample label">some ' +
elementType +
'</' +
elementType +
'>'
);
const node = fixture.querySelector(elementType);
const virtualNode = axe.utils.getNodeFromTree(axe._tree[0], node);
assert.isTrue(rule.matches(node, virtualNode));
}
);

it(
'should match because it is a ' +
elementType +
' with an explicit landmark role but no label',
function () {
const explicitRole =
elementType === 'section' ? 'navigation' : 'search';
axeFixtureSetup(
'<' +
elementType +
' role="' +
explicitRole +
'">some ' +
elementType +
'</' +
elementType +
'>'
);
const node = fixture.querySelector(elementType);
const virtualNode = axe.utils.getNodeFromTree(axe._tree[0], node);
assert.isTrue(rule.matches(node, virtualNode));
}
);

it(
'should not match because it is a ' + elementType + ' without a label',
function () {
Expand All @@ -78,6 +126,26 @@ describe('landmark-unique-matches', function () {
assert.isFalse(rule.matches(node, virtualNode));
}
);

it(
'should not match because it is a ' +
elementType +
' with a label but a non-landmark role',
function () {
axeFixtureSetup(
'<' +
elementType +
' role="tabpanel" aria-label="sample label">some ' +
elementType +
'</' +
elementType +
'>'
);
const node = fixture.querySelector(elementType);
const virtualNode = axe.utils.getNodeFromTree(axe._tree[0], node);
assert.isFalse(rule.matches(node, virtualNode));
}
);
});
});

Expand Down
Loading