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-roles): report error for fallback roles #1970

Merged
merged 7 commits into from
Jan 27, 2020
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
11 changes: 10 additions & 1 deletion lib/checks/aria/abstractrole.js
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
return axe.commons.aria.getRoleType(node.getAttribute('role')) === 'abstract';
const abstractRoles = axe.utils
.tokenList(virtualNode.attr('role'))
.filter(role => axe.commons.aria.getRoleType(role) === 'abstract');

if (abstractRoles.length > 0) {
this.data(abstractRoles);
return true;
}

return false;
5 changes: 4 additions & 1 deletion lib/checks/aria/abstractrole.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "serious",
"messages": {
"pass": "Abstract roles are not used",
"fail": "Abstract roles cannot be directly used"
"fail": {
"singular": "Abstract role cannot be directly used: ${data.values}",
"plural": "Abstract roles cannot be directly used: ${data.values}"
}
}
}
}
1 change: 1 addition & 0 deletions lib/checks/aria/fallbackrole.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return axe.utils.tokenList(virtualNode.attr('role')).length > 1;
11 changes: 11 additions & 0 deletions lib/checks/aria/fallbackrole.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"id": "fallbackrole",
"evaluate": "fallbackrole.js",
"metadata": {
"impact": "serious",
"messages": {
"pass": "Only one role value used",
"fail": "Use only one role value, since fallback roles are not supported in older browsers"
}
}
}
17 changes: 14 additions & 3 deletions lib/checks/aria/invalidrole.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
return !axe.commons.aria.isValidRole(node.getAttribute('role'), {
allowAbstract: true
});
const invalidRoles = axe.utils
.tokenList(virtualNode.attr('role'))
.filter(role => {
return !axe.commons.aria.isValidRole(role, {
allowAbstract: true
});
});

if (invalidRoles.length > 0) {
this.data(invalidRoles);
return true;
}

return false;
5 changes: 4 additions & 1 deletion lib/checks/aria/invalidrole.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "critical",
"messages": {
"pass": "ARIA role is valid",
"fail": "Role must be one of the valid ARIA roles"
"fail": {
"singular": "Role must be one of the valid ARIA roles: ${data.values}",
"plural": "Roles must be one of the valid ARIA roles: ${data.values}"
}
}
}
}
2 changes: 1 addition & 1 deletion lib/rules/aria-roles.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
},
"all": [],
"any": [],
"none": ["invalidrole", "abstractrole", "unsupportedrole"]
"none": ["fallbackrole", "invalidrole", "abstractrole", "unsupportedrole"]
straker marked this conversation as resolved.
Show resolved Hide resolved
}
40 changes: 40 additions & 0 deletions test/checks/aria/fallbackrole.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
describe('fallbackrole', function() {
'use strict';

var fixture = document.getElementById('fixture');
var queryFixture = axe.testUtils.queryFixture;

afterEach(function() {
fixture.innerHTML = '';
});

it('should return true if fallback role is used', function() {
var virtualNode = queryFixture(
'<div id="target" role="button foobar">Foo</div>'
);
assert.isTrue(
checks.fallbackrole.evaluate(virtualNode.actualNode, 'radio', virtualNode)
);
});

it('should return false if fallback role is not used', function() {
var virtualNode = queryFixture('<div id="target" role="button">Foo</div>');
assert.isFalse(
checks.fallbackrole.evaluate(virtualNode.actualNode, 'radio', virtualNode)
);
});

it('should return false if applied to an invalid role', function() {
var virtualNode = queryFixture('<div id="target" role="foobar">Foo</div>');
assert.isFalse(
checks.fallbackrole.evaluate(virtualNode.actualNode, 'radio', virtualNode)
);
});

it('should return false if applied to an invalid role', function() {
var virtualNode = queryFixture('<div id="target" role="foobar">Foo</div>');
assert.isFalse(
checks.fallbackrole.evaluate(virtualNode.actualNode, 'radio', virtualNode)
);
});
});
78 changes: 69 additions & 9 deletions test/checks/shared/abstractrole.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,86 @@ describe('abstractrole', function() {
'use strict';

var fixture = document.getElementById('fixture');
var queryFixture = axe.testUtils.queryFixture;
var checkContext = axe.testUtils.MockCheckContext();

afterEach(function() {
fixture.innerHTML = '';
checkContext.reset();
});

it('should return false if applied to a concrete role', function() {
fixture.innerHTML = '<div id="target" role="alert">Contents</div>';
var node = fixture.querySelector('#target');
assert.isFalse(checks.abstractrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="alert">Contents</div>'
);
assert.isFalse(
checks.abstractrole.evaluate.call(
checkContext,
virtualNode.actualNode,
'radio',
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return false if applied to a nonsensical role', function() {
fixture.innerHTML = '<div id="target" role="foo">Contents</div>';
var node = fixture.querySelector('#target');
assert.isFalse(checks.abstractrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="foo">Contents</div>'
);
assert.isFalse(
checks.abstractrole.evaluate.call(
checkContext,
virtualNode.actualNode,
'radio',
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return true if applied to an abstract role', function() {
fixture.innerHTML = '<div id="target" role="widget">Contents</div>';
var node = fixture.querySelector('#target');
assert.isTrue(checks.abstractrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="widget">Contents</div>'
);
assert.isTrue(
checks.abstractrole.evaluate.call(
checkContext,
virtualNode.actualNode,
'radio',
virtualNode
)
);
assert.deepEqual(checkContext._data, ['widget']);
});

it('should return false if applied to multiple concrete roles', function() {
var virtualNode = queryFixture(
'<div id="target" role="alert button">Contents</div>'
);
assert.isFalse(
checks.abstractrole.evaluate.call(
checkContext,
virtualNode.actualNode,
'radio',
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return true if applied to at least one abstract role', function() {
var virtualNode = queryFixture(
'<div id="target" role="alert widget structure">Contents</div>'
);
assert.isTrue(
checks.abstractrole.evaluate.call(
checkContext,
virtualNode.actualNode,
'radio',
virtualNode
)
);
assert.deepEqual(checkContext._data, ['widget', 'structure']);
});
});
91 changes: 79 additions & 12 deletions test/checks/shared/invalidrole.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,99 @@ describe('invalidrole', function() {
'use strict';

var fixture = document.getElementById('fixture');
var queryFixture = axe.testUtils.queryFixture;
var checkContext = axe.testUtils.MockCheckContext();

afterEach(function() {
fixture.innerHTML = '';
checkContext.reset();
});

it('should return true if applied to an empty role', function() {
fixture.innerHTML = '<div id="target" role="">Contents</div>';
var node = fixture.querySelector('#target');
assert.isTrue(checks.invalidrole.evaluate(node, 'radio'));
var virtualNode = queryFixture('<div id="target" role="">Contents</div>');
assert.isTrue(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.deepEqual(checkContext._data, ['']);
});

it('should return true if applied to a nonsensical role', function() {
fixture.innerHTML = '<div id="target" role="foo">Contents</div>';
var node = fixture.querySelector('#target');
assert.isTrue(checks.invalidrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="foo">Contents</div>'
);
assert.isTrue(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.deepEqual(checkContext._data, ['foo']);
});

it('should return false if applied to a concrete role', function() {
fixture.innerHTML = '<div id="target" role="alert">Contents</div>';
var node = fixture.querySelector('#target');
assert.isFalse(checks.invalidrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="alert">Contents</div>'
);
assert.isFalse(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return false if applied to an abstract role', function() {
fixture.innerHTML = '<div id="target" role="widget">Contents</div>';
var node = fixture.querySelector('#target');
assert.isFalse(checks.invalidrole.evaluate(node, 'radio'));
var virtualNode = queryFixture(
'<div id="target" role="widget">Contents</div>'
);
assert.isFalse(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return false if applied to multiple valid roles', function() {
var virtualNode = queryFixture(
'<div id="target" role="alert button">Contents</div>'
);
assert.isFalse(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.isNull(checkContext._data);
});

it('should return true if applied to at least one nonsensical role', function() {
var virtualNode = queryFixture(
'<div id="target" role="alert button foo bar">Contents</div>'
);
assert.isTrue(
checks.invalidrole.evaluate.call(
checkContext,
virtualNode.actualNode,
null,
virtualNode
)
);
assert.deepEqual(checkContext._data, ['foo', 'bar']);
});
});
2 changes: 2 additions & 0 deletions test/integration/rules/aria-roles/aria-roles.html
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,6 @@
<div role="lol" id="fail13">fail</div>
<div role="text" id="fail14">ok</div>
<!-- unsupported roles -->
<!-- fallback roles -->
<div role="button alert" id="fail15">fail</div>
</div>
3 changes: 2 additions & 1 deletion test/integration/rules/aria-roles/aria-roles.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
["#fail11"],
["#fail12"],
["#fail13"],
["#fail14"]
["#fail14"],
["#fail15"]
],
"passes": [
["#pass1"],
Expand Down