Skip to content

Commit

Permalink
fix: Ignore abstracts in determining element roles
Browse files Browse the repository at this point in the history
  • Loading branch information
WilcoFiers committed Jul 20, 2018
1 parent 3a8729b commit 1af6088
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 76 deletions.
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 |
| 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);
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));
}
);
});
10 changes: 10 additions & 0 deletions test/commons/aria/roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ describe('aria.isValidRole', function() {
it('should return false if role is not found in the lookup table', function() {
assert.isFalse(axe.commons.aria.isValidRole('cats'));
});

it('returns false for abstract roles by default', function() {
assert.isFalse(axe.commons.aria.isValidRole('input'));
});

it('returns true for abstract roles with { allowAbstract: true } ', function() {
assert.isTrue(
axe.commons.aria.isValidRole('input', { allowAbstract: true })
);
});
});

describe('aria.getRolesWithNameFromContents', function() {
Expand Down
13 changes: 13 additions & 0 deletions test/commons/table/is-data-cell.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,17 @@ describe('table.isDataCell', function() {
assert.isTrue(axe.commons.table.isDataCell(target1));
assert.isFalse(axe.commons.table.isDataCell(target2));
});

it('should ignore abstract roles', function() {
fixture.innerHTML =
'<table>' +
'<tr><td id="target1" role="section">heading</td></tr>' +
'<tr><th id="target2" role="section">heading</th></tr>' +
'</table>';

var target1 = $id('target1');
var target2 = $id('target2');
assert.isTrue(axe.commons.table.isDataCell(target1));
assert.isFalse(axe.commons.table.isDataCell(target2));
});
});
6 changes: 6 additions & 0 deletions test/rule-matches/heading-matches.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,10 @@ describe('heading-matches', function() {
h1.setAttribute('role', 'bruce');
assert.isTrue(rule.matches(h1));
});

it('should return true on headings with their role changes to an abstract role', function() {
var h1 = document.createElement('h1');
h1.setAttribute('role', 'widget');
assert.isTrue(rule.matches(h1));
});
});

0 comments on commit 1af6088

Please sign in to comment.