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

feat: Add aria.getRole method #1017

Merged
merged 10 commits into from
Aug 7, 2018
14 changes: 7 additions & 7 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
| input-image-alt | Ensures <input type="image"> elements have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true |
| label-title-only | Ensures that every form element is not solely labeled using the title or aria-describedby attributes | Serious | cat.forms, best-practice | true |
| label | Ensures every form element has a label | Minor, Serious, Critical | cat.forms, wcag2a, wcag332, wcag131, section508, section508.22.n | true |
| landmark-banner-is-top-level | The banner landmark should not be contained in another landmark | Moderate | cat.semantics, best-practice | true |
| landmark-contentinfo-is-top-level | The contentinfo landmark should not be contained in another landmark | Moderate | cat.semantics, best-practice | true |
| landmark-main-is-top-level | The main landmark should not be contained in another landmark | Moderate | cat.semantics, best-practice | true |
| landmark-no-duplicate-banner | Ensures the document has no more than one banner landmark | Moderate | cat.semantics, best-practice | true |
| landmark-no-duplicate-contentinfo | Ensures the document has no more than one contentinfo landmark | Moderate | cat.semantics, best-practice | true |
| landmark-one-main | Ensures a navigation point to the primary content of the page. If the page contains iframes, each iframe should contain either no main landmarks or just one | Moderate | cat.semantics, best-practice | true |
| landmark-banner-is-top-level | Ensures the banner landmark is at top level | Moderate | cat.semantics, best-practice | true |
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor remark, shouldn't these documentation updates be a part of another build where these changes were made to the respective rules? Worth folding into a new PR?

| landmark-contentinfo-is-top-level | Ensures the contentinfo landmark is at top level | Moderate | cat.semantics, best-practice | true |
| landmark-main-is-top-level | Ensures the main landmark is at top level | Moderate | cat.semantics, best-practice | true |
| landmark-no-duplicate-banner | Ensures the page has at most one banner landmark | Moderate | cat.semantics, best-practice | true |
| landmark-no-duplicate-contentinfo | Ensures the page has at most one contentinfo landmark | Moderate | cat.semantics, best-practice | true |
| landmark-one-main | Ensures the page has only one main landmark and each iframe in the page has at most one main landmark | Moderate | cat.semantics, best-practice | true |
| layout-table | Ensures presentational <table> elements do not use <th>, <caption> elements or the summary attribute | Serious | cat.semantics, wcag2a, wcag131 | true |
| link-in-text-block | Links can be distinguished without relying on color | Serious | cat.color, experimental, wcag2a, wcag141 | true |
| link-name | Ensures links have discernible text | Serious | cat.name-role-value, wcag2a, wcag412, wcag244, section508, section508.22.a | true |
Expand All @@ -55,7 +55,7 @@
| p-as-heading | Ensure p elements are not used to style headings | Serious | cat.semantics, wcag2a, wcag131, experimental | true |
| page-has-heading-one | Ensure that the page, or at least one of its frames contains a level-one heading | Moderate | cat.semantics, best-practice | true |
| radiogroup | Ensures related <input type="radio"> elements have a group and that the group designation is consistent | Critical | cat.forms, best-practice | true |
| region | Ensures all content is contained within a landmark region | Moderate | cat.keyboard, best-practice | true |
| region | Ensures all page content is contained by landmarks | Moderate | cat.keyboard, best-practice | true |
| scope-attr-valid | Ensures the scope attribute is used correctly on tables | Moderate, Critical | cat.tables, best-practice | true |
| server-side-image-map | Ensures that server-side image maps are not used | Minor | cat.text-alternatives, wcag2a, wcag211, section508, section508.22.f | true |
| skip-link | Ensure all skip links have a focusable target | Moderate | cat.keyboard, best-practice | true |
Expand Down
4 changes: 3 additions & 1 deletion lib/checks/aria/invalidrole.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
return !axe.commons.aria.isValidRole(node.getAttribute('role'));
return !axe.commons.aria.isValidRole(node.getAttribute('role'), {
allowAbstract: true
});
8 changes: 2 additions & 6 deletions lib/checks/lists/dlitem.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
const parent = axe.commons.dom.getComposedParent(node);
const parentTagName = parent.nodeName.toUpperCase();

// Unlike with UL|OL+LI, DT|DD must be in a DL
if (parentTagName !== 'DL') {
return false;
}

const parentRole = (parent.getAttribute('role') || '').toLowerCase();

if (!parentRole) {
if (!parentRole || !axe.commons.aria.isValidRole(parentRole)) {
return true;
}

if (!axe.commons.aria.isValidRole(parentRole)) {
return false;
}

if (parentRole === 'list') {
return true;
}
Expand Down
18 changes: 9 additions & 9 deletions lib/checks/lists/listitem.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
const parent = axe.commons.dom.getComposedParent(node);
if (!parent) {
return false;
// Can only happen with detached DOM nodes and roots:
return undefined;
}

const ALLOWED_TAGS = ['UL', 'OL'];
const parentTagName = parent.nodeName.toUpperCase();

const parentRole = (parent.getAttribute('role') || '').toLowerCase();
if (parentRole && !axe.commons.aria.isValidRole(parentRole)) {

if (parentRole === 'list') {
return true;
}

if (parentRole && axe.commons.aria.isValidRole(parentRole)) {
return false;
}

const isListRole = parentRole === 'list';
return (
(ALLOWED_TAGS.includes(parentTagName) && (!parentRole || isListRole)) ||
isListRole
);
return ['UL', 'OL'].includes(parentTagName);
25 changes: 25 additions & 0 deletions lib/commons/aria/get-role.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/* global aria, axe */

Copy link
Contributor

Choose a reason for hiding this comment

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

Add jsdoc comments please, with @return decorating what is expected.

aria.getRole = function getRole(
node,
{ noImplicit, tokenList, abstracts, dpub } = {}
) {
const roleAttr = (node.getAttribute('role') || '').trim().toLowerCase();
const roleList = tokenList ? axe.utils.tokenList(roleAttr) : [roleAttr];

// Get the first valid role:
const validRoles = roleList.filter(role => {
if (!dpub && role.substr(0, 4) === 'doc-') {
return false;
}
return aria.isValidRole(role, { allowAbstract: abstracts });
});
const role = validRoles[0];
Copy link
Contributor

@jeeyyy jeeyyy Jul 28, 2018

Choose a reason for hiding this comment

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

need a length check on validRoles here, there is a chance that this can be an empty array, if role is not defined in a given node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't matter. In that case role undefined, which is what it should be.


// Get the impgit licit role, if permitted
Copy link
Contributor

Choose a reason for hiding this comment

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

Edit: In comment - implicit

if (!role && !noImplicit) {
return aria.implicitRole(node);
}

return role || null;
};
12 changes: 6 additions & 6 deletions lib/commons/aria/roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
* @memberof axe.commons.aria
* @instance
* @param {String} role The role to check
* @param {Object} options Use `allowAbstract` if you want abstracts
* @return {Boolean}
*/
aria.isValidRole = function(role) {
'use strict';
if (aria.lookupTable.role[role]) {
return true;
aria.isValidRole = function(role, { allowAbstract } = {}) {
const roleDefinition = aria.lookupTable.role[role];
if (!roleDefinition) {
return false;
}

return false;
return allowAbstract ? true : roleDefinition.type !== 'abstract';
};

/**
Expand Down
81 changes: 56 additions & 25 deletions test/checks/lists/dlitem.js
Original file line number Diff line number Diff line change
@@ -1,60 +1,91 @@
describe('dlitem', function () {
describe('dlitem', function() {
'use strict';

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

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

it('should pass if the dlitem has a parent <dl>', function () {
it('should pass if the dlitem has a parent <dl>', function() {
var checkArgs = checkSetup('<dl><dt id="target">My list item</dl>');

assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
});

it('should fail if the dt element has an incorrect parent', function () {
it('should fail if the dt element has an incorrect parent', function() {
var checkArgs = checkSetup('<video><dt id="target">My list item</video>');

assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

it('should pass if the dt element has a parent <dl> with role="list"', function () {
var checkArgs = checkSetup('<dl role="list"><dt id="target">My list item</dl>');
it('should pass if the dt element has a parent <dl> with role="list"', function() {
var checkArgs = checkSetup(
'<dl role="list"><dt id="target">My list item</dl>'
);
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
})
});

it('should fail if the dt element has a parent <dl> with role="presentation"', function () {
var checkArgs = checkSetup('<dl role="presentation"><dt id="target">My list item</dl>');
it('should fail if the dt element has a parent <dl> with role="presentation"', function() {
var checkArgs = checkSetup(
'<dl role="presentation"><dt id="target">My list item</dl>'
);
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

it('should fail if the dt element has a parent <dl> with a changed role', function(){
var checkArgs = checkSetup('<dl role="menubar"><dt id="target">My list item</dl>');
it('should fail if the dt element has a parent <div> with role="list"', function() {
var checkArgs = checkSetup(
'<div role="list"><dt id="target">My list item</div>'
);
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return true in a shadow DOM pass', function () {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<dl><slot></slot></dl>';

var checkArgs = checkSetup(node, 'dt');
it('should pass if the dt element has a parent <dl> with an abstract role', function() {
var checkArgs = checkSetup(
'<dl role="section"><dt id="target">My list item</dl>'
);
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return false in a shadow DOM fail', function () {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';
it('should pass if the dt element has a parent <dl> with an invalid role', function() {
var checkArgs = checkSetup(
'<dl role="invalid-role"><dt id="target">My list item</dl>'
);
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
});

var checkArgs = checkSetup(node, 'dt');
it('should fail if the dt element has a parent <dl> with a changed role', function() {
var checkArgs = checkSetup(
'<dl role="menubar"><dt id="target">My list item</dl>'
);
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});
});

(shadowSupport.v1 ? it : xit)(
'should return true in a shadow DOM pass',
function() {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<dl><slot></slot></dl>';

var checkArgs = checkSetup(node, 'dt');
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
}
);

(shadowSupport.v1 ? it : xit)(
'should return false in a shadow DOM fail',
function() {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';

var checkArgs = checkSetup(node, 'dt');
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
}
);
});
67 changes: 45 additions & 22 deletions test/checks/lists/listitem.js
Original file line number Diff line number Diff line change
@@ -1,55 +1,78 @@
describe('listitem', function () {
describe('listitem', function() {
'use strict';

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

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

it('should pass if the listitem has a parent <ol>', function () {
it('should pass if the listitem has a parent <ol>', function() {
var checkArgs = checkSetup('<ol><li id="target">My list item</li></ol>');
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
});

it('should pass if the listitem has a parent <ul>', function () {
it('should pass if the listitem has a parent <ul>', function() {
var checkArgs = checkSetup('<ul><li id="target">My list item</li></ul>');
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
});

it('should pass if the listitem has a parent role=list', function () {
var checkArgs = checkSetup('<div role="list"><li id="target">My list item</li></div>');
it('should pass if the listitem has a parent role=list', function() {
var checkArgs = checkSetup(
'<div role="list"><li id="target">My list item</li></div>'
);
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
});

it('should fail if the listitem has an incorrect parent', function () {
it('should fail if the listitem has an incorrect parent', function() {
var checkArgs = checkSetup('<div><li id="target">My list item</li></div>');
assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
});

it('should fail if the listitem has a parent <ol> with changed role', function () {
var checkArgs = checkSetup('<ol role="menubar"><li id="target">My list item</li></ol>');
it('should fail if the listitem has a parent <ol> with changed role', function() {
var checkArgs = checkSetup(
'<ol role="menubar"><li id="target">My list item</li></ol>'
);
assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return true in a shadow DOM pass', function () {
var node = document.createElement('div');
node.innerHTML = '<li>My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<ul><slot></slot></ul>';
var checkArgs = checkSetup(node, 'li');
it('should pass if the listitem has a parent <ol> with an invalid role', function() {
var checkArgs = checkSetup(
'<ol role="invalid-role"><li id="target">My list item</li></ol>'
);
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return false in a shadow DOM fail', function () {
var node = document.createElement('div');
node.innerHTML = '<li>My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';
var checkArgs = checkSetup(node, 'li');
assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
it('should pass if the listitem has a parent <ol> with an abstract role', function() {
var checkArgs = checkSetup(
'<ol role="section"><li id="target">My list item</li></ol>'
);
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)(
'should return true in a shadow DOM pass',
function() {
var node = document.createElement('div');
node.innerHTML = '<li>My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<ul><slot></slot></ul>';
var checkArgs = checkSetup(node, 'li');
assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
}
);

(shadowSupport.v1 ? it : xit)(
'should return false in a shadow DOM fail',
function() {
var node = document.createElement('div');
node.innerHTML = '<li>My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';
var checkArgs = checkSetup(node, 'li');
assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
}
);
});
Loading