Skip to content
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.

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

fix(aria-allowed-attr): allow aria-required=false when normally not allowed #4532

Merged
merged 1 commit into from
Jul 10, 2024
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
12 changes: 11 additions & 1 deletion lib/checks/aria/aria-allowed-attr-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ export default function ariaAllowedAttrEvaluate(node, options, virtualNode) {

// Unknown ARIA attributes are tested in aria-valid-attr
for (const attrName of virtualNode.attrNames) {
if (validateAttr(attrName) && !allowed.includes(attrName)) {
if (
validateAttr(attrName) &&
!allowed.includes(attrName) &&
!ignoredAttrs(attrName, virtualNode.attr(attrName))
) {
invalid.push(attrName);
}
}
Expand All @@ -57,3 +61,9 @@ export default function ariaAllowedAttrEvaluate(node, options, virtualNode) {
}
return false;
}

function ignoredAttrs(attrName, attrValue) {
// allow aria-required=false as screen readers consistently ignore it
// @see https://github.com/dequelabs/axe-core/issues/3756
return attrName === 'aria-required' && attrValue === 'false';
}
86 changes: 56 additions & 30 deletions test/checks/aria/aria-allowed-attr.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
describe('aria-allowed-attr', function () {
describe('aria-allowed-attr', () => {
'use strict';

var queryFixture = axe.testUtils.queryFixture;
var checkContext = axe.testUtils.MockCheckContext();
const queryFixture = axe.testUtils.queryFixture;
const checkContext = axe.testUtils.MockCheckContext();

afterEach(function () {
afterEach(() => {
checkContext.reset();
});

it('should detect incorrectly used attributes', function () {
var vNode = queryFixture(
it('should detect incorrectly used attributes', () => {
const vNode = queryFixture(
'<div role="link" id="target" tabindex="1" aria-selected="true"></div>'
);

Expand All @@ -21,8 +21,8 @@ describe('aria-allowed-attr', function () {
assert.deepEqual(checkContext._data, ['aria-selected="true"']);
});

it('should not report on required attributes', function () {
var vNode = queryFixture(
it('should not report on required attributes', () => {
const vNode = queryFixture(
'<div role="checkbox" id="target" tabindex="1" aria-checked="true"></div>'
);

Expand All @@ -33,8 +33,8 @@ describe('aria-allowed-attr', function () {
);
});

it('should detect incorrectly used attributes - implicit role', function () {
var vNode = queryFixture(
it('should detect incorrectly used attributes - implicit role', () => {
const vNode = queryFixture(
'<a href="#" id="target" tabindex="1" aria-selected="true"></a>'
);

Expand All @@ -46,8 +46,8 @@ describe('aria-allowed-attr', function () {
assert.deepEqual(checkContext._data, ['aria-selected="true"']);
});

it('should return true for global attributes if there is no role', function () {
var vNode = queryFixture(
it('should return true for global attributes if there is no role', () => {
const vNode = queryFixture(
'<div id="target" tabindex="1" aria-busy="true" aria-owns="foo"></div>'
);

Expand All @@ -59,8 +59,8 @@ describe('aria-allowed-attr', function () {
assert.isNull(checkContext._data);
});

it('should return false for non-global attributes if there is no role', function () {
var vNode = queryFixture(
it('should return false for non-global attributes if there is no role', () => {
const vNode = queryFixture(
'<div id="target" tabindex="1" aria-selected="true" aria-owns="foo"></div>'
);

Expand All @@ -72,8 +72,8 @@ describe('aria-allowed-attr', function () {
assert.deepEqual(checkContext._data, ['aria-selected="true"']);
});

it('should not report on invalid attributes', function () {
var vNode = queryFixture(
it('should not report on invalid attributes', () => {
const vNode = queryFixture(
'<div role="dialog" id="target" tabindex="1" aria-cats="true"></div>'
);

Expand All @@ -85,8 +85,8 @@ describe('aria-allowed-attr', function () {
assert.isNull(checkContext._data);
});

it('should not report on allowed attributes', function () {
var vNode = queryFixture(
it('should not report on allowed attributes', () => {
const vNode = queryFixture(
'<div role="radio" id="target" tabindex="1" aria-required="true" aria-checked="true"></div>'
);

Expand All @@ -98,8 +98,34 @@ describe('aria-allowed-attr', function () {
assert.isNull(checkContext._data);
});

it('should return undefined for custom element that has no role and is not focusable', function () {
var vNode = queryFixture(
it('should not report on aria-required=false', () => {
const vNode = queryFixture(
'<button id="target" aria-required="false"></button>'
);

assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.isNull(checkContext._data);
});

it('should return false for unallowed aria-required=true', () => {
const vNode = queryFixture(
'<button id="target" aria-required="true"></button>'
);

assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.deepEqual(checkContext._data, ['aria-required="true"']);
});

it('should return undefined for custom element that has no role and is not focusable', () => {
const vNode = queryFixture(
'<my-custom-element id="target" aria-expanded="true"></my-custom-element>'
);

Expand All @@ -111,8 +137,8 @@ describe('aria-allowed-attr', function () {
assert.isNotNull(checkContext._data);
});

it("should return false for custom element that has a role which doesn't allow the attribute", function () {
var vNode = queryFixture(
it("should return false for custom element that has a role which doesn't allow the attribute", () => {
const vNode = queryFixture(
'<my-custom-element role="insertion" id="target" aria-expanded="true"></my-custom-element>'
);

Expand All @@ -124,8 +150,8 @@ describe('aria-allowed-attr', function () {
assert.isNotNull(checkContext._data);
});

it('should return false for custom element that is focusable', function () {
var vNode = queryFixture(
it('should return false for custom element that is focusable', () => {
const vNode = queryFixture(
'<my-custom-element tabindex="1" id="target" aria-expanded="true"></my-custom-element>'
);

Expand All @@ -137,8 +163,8 @@ describe('aria-allowed-attr', function () {
assert.isNotNull(checkContext._data);
});

describe('options', function () {
it('should allow provided attribute names for a role', function () {
describe('options', () => {
it('should allow provided attribute names for a role', () => {
axe.configure({
standards: {
ariaRoles: {
Expand All @@ -149,7 +175,7 @@ describe('aria-allowed-attr', function () {
}
});

var vNode = queryFixture(
const vNode = queryFixture(
'<div role="mccheddarton" id="target" aria-checked="true" aria-selected="true"></div>'
);

Expand All @@ -171,7 +197,7 @@ describe('aria-allowed-attr', function () {
);
});

it('should handle multiple roles provided in options', function () {
it('should handle multiple roles provided in options', () => {
axe.configure({
standards: {
ariaRoles: {
Expand All @@ -185,10 +211,10 @@ describe('aria-allowed-attr', function () {
}
});

var vNode = queryFixture(
const vNode = queryFixture(
'<div role="bagley" id="target" aria-selected="true"></div>'
);
var options = {
const options = {
mccheddarton: ['aria-selected'],
bagley: ['aria-selected']
};
Expand Down
3 changes: 3 additions & 0 deletions test/integration/rules/aria-allowed-attr/passes.html
Original file line number Diff line number Diff line change
Expand Up @@ -2169,3 +2169,6 @@
<input type="checkbox" aria-checked="mixed" id="pass99" />

<search aria-expanded="true" id="pass100"></search>

<!-- Ignored ARIA attributes -->
<button id="pass101" aria-required="false"></button>
3 changes: 2 additions & 1 deletion test/integration/rules/aria-allowed-attr/passes.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
["#pass97"],
["#pass98"],
["#pass99"],
["#pass100"]
["#pass100"],
["#pass101"]
]
}
Loading