Skip to content

Commit

Permalink
fix: ignore invalid and allow redundant role in aria-allowed-role (#1118
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jeeyyy authored Sep 7, 2018
1 parent e4788bf commit a0f9b31
Show file tree
Hide file tree
Showing 10 changed files with 215 additions and 75 deletions.
104 changes: 59 additions & 45 deletions lib/commons/aria/get-element-unallowed-roles.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,46 @@
/* global aria */

/**
* Returns all roles applicable to element in a list
*
* @method getRoleSegments
* @private
* @param {Element} node
* @returns {Array} Roles list or empty list
*/

function getRoleSegments(node) {
let roles = [];

if (!node) {
return roles;
}

if (node.hasAttribute('role')) {
const nodeRoles = axe.utils.tokenList(
node.getAttribute('role').toLowerCase()
);
roles = roles.concat(nodeRoles);
}

if (node.hasAttributeNS('http://www.idpf.org/2007/ops', 'type')) {
const epubRoles = axe.utils
.tokenList(
node
.getAttributeNS('http://www.idpf.org/2007/ops', 'type')
.toLowerCase()
)
.map(role => `doc-${role}`);

roles = roles.concat(epubRoles);
}

// filter invalid roles
roles = roles.filter(role => axe.commons.aria.isValidRole(role));

return roles;
}

/**
* gets all unallowed roles for a given node
* @method getElementUnallowedRoles
Expand All @@ -9,38 +51,8 @@
*/
aria.getElementUnallowedRoles = function getElementUnallowedRoles(
node,
allowImplicit
allowImplicit = true
) {
/**
* Get roles applied to a given node
* @param {HTMLElement} node HTMLElement
* @return {Array<String>} return an array of roles applied to the node, if no roles, return an empty array.
*/
// TODO: not moving this to outer namespace yet, work with wilco to see overlap with his PR(WIP) - aria.getRole
function getRoleSegments(node) {
let roles = [];
if (!node) {
return roles;
}
if (node.hasAttribute('role')) {
const nodeRoles = axe.utils.tokenList(
node.getAttribute('role').toLowerCase()
);
roles = roles.concat(nodeRoles);
}
if (node.hasAttributeNS('http://www.idpf.org/2007/ops', 'type')) {
const epubRoles = axe.utils
.tokenList(
node
.getAttributeNS('http://www.idpf.org/2007/ops', 'type')
.toLowerCase()
)
.map(role => `doc-${role}`);
roles = roles.concat(epubRoles);
}
return roles;
}

const tagName = node.nodeName.toUpperCase();

// by pass custom elements
Expand All @@ -53,24 +65,26 @@ aria.getElementUnallowedRoles = function getElementUnallowedRoles(

// stores all roles that are not allowed for a specific element most often an element only has one explicit role
const unallowedRoles = roleSegments.filter(role => {
if (!axe.commons.aria.isValidRole(role)) {
// do not check made-up/ fake roles
// if role and implicit role are same, when allowImplicit: true
// ignore as it is a redundant role
if (allowImplicit && role === implicitRole) {
return false;
}

// check if an implicit role may be set explicit following a setting
if (!allowImplicit && role === implicitRole) {
// edge case: setting implicit role row on tr element is allowed when child of table[role='grid']
if (
!(
role === 'row' &&
tagName === 'TR' &&
axe.utils.matchesSelector(node, 'table[role="grid"] > tr')
)
) {
return true;
}
// Edge case:
// setting implicit role row on tr element is allowed when child of table[role='grid']
if (
!allowImplicit &&
!(
role === 'row' &&
tagName === 'TR' &&
axe.utils.matchesSelector(node, 'table[role="grid"] > tr')
)
) {
return true;
}

// check if role is allowed on element
if (!aria.isAriaRoleAllowedOnElement(node, role)) {
return true;
}
Expand Down
1 change: 1 addition & 0 deletions lib/commons/aria/get-role.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ aria.getRole = function getRole(
}
return aria.isValidRole(role, { allowAbstract: abstracts });
});

const explicitRole = validRoles[0];

// Get the implicit role, if permitted
Expand Down
41 changes: 31 additions & 10 deletions lib/commons/aria/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,15 @@ lookupTable.role = {
},
nameFrom: ['author'],
context: null,
unsupported: false
unsupported: false,
allowedElements: [
{
tagName: 'INPUT',
attributes: {
TYPE: 'TEXT'
}
}
]
},
command: {
nameFrom: ['author'],
Expand Down Expand Up @@ -1687,7 +1695,15 @@ lookupTable.role = {
nameFrom: ['author'],
context: null,
implicit: ['input[type="search"]'],
unsupported: false
unsupported: false,
allowedElements: [
{
tagName: 'INPUT',
attributes: {
TYPE: 'TEXT'
}
}
]
},
section: {
nameFrom: ['author', 'contents'],
Expand Down Expand Up @@ -1756,7 +1772,15 @@ lookupTable.role = {
nameFrom: ['author'],
context: null,
implicit: ['input[type="number"]'],
unsupported: false
unsupported: false,
allowedElements: [
{
tagName: 'INPUT',
attributes: {
TYPE: 'TEXT'
}
}
]
},
status: {
type: 'widget',
Expand Down Expand Up @@ -2148,13 +2172,6 @@ lookupTable.elementsAllowedNoRole = [
TYPE: 'TEL'
}
},
{
tagName: 'INPUT',
condition: elementConditions.CANNOT_HAVE_LIST_ATTRIBUTE,
attributes: {
TYPE: 'TEXT'
}
},
{
tagName: 'INPUT',
attributes: {
Expand Down Expand Up @@ -2368,6 +2385,10 @@ lookupTable.evaluateRoleForElement = {
return out;
case 'radio':
return role === 'menuitemradio';
case 'text':
return (
role === 'combobox' || role === 'searchbox' || role === 'spinbutton'
);
default:
return false;
}
Expand Down
7 changes: 7 additions & 0 deletions lib/rules/aria-allowed-role-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
return (
axe.commons.aria.getRole(node, {
noImplicit: true,
dpub: true,
fallback: true
}) !== null
);
1 change: 1 addition & 0 deletions lib/rules/aria-allowed-role.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"id": "aria-allowed-role",
"excludeHidden": false,
"selector": "[role]",
"matches": "aria-allowed-role-matches.js",
"tags": [
"cat.aria",
"best-practice"
Expand Down
49 changes: 33 additions & 16 deletions test/checks/aria/aria-allowed-role.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,37 @@ describe('aria-allowed-role', function() {
);
});

it('returns false when MENU has type context', function() {
it('returns true when INPUT type is text with role combobox', function() {
var node = document.createElement('input');
node.setAttribute('type', 'text');
node.setAttribute('role', 'combobox');
fixture.appendChild(node);
assert.isTrue(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
});

it('returns true when INPUT type is text with role spinbutton', function() {
var node = document.createElement('input');
node.setAttribute('type', 'text');
node.setAttribute('role', 'spinbutton');
fixture.appendChild(node);
assert.isTrue(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
});

it('returns true when INPUT type is text with role searchbox', function() {
var node = document.createElement('input');
node.setAttribute('type', 'text');
node.setAttribute('role', 'searchbox');
fixture.appendChild(node);
assert.isTrue(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
});

it('returns true when MENU has type context', function() {
var node = document.createElement('menu');
node.setAttribute('type', 'context');
node.setAttribute('role', 'navigation');
Expand Down Expand Up @@ -135,10 +165,8 @@ describe('aria-allowed-role', function() {
node.setAttribute('role', 'link');
node.href = '';
fixture.appendChild(node);
assert.isFalse(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
assert.deepEqual(checkContext._data, ['link']);
var actual = checks['aria-allowed-role'].evaluate.call(checkContext, node);
assert.isTrue(actual);
});

it('returns true <img> with a non-empty alt', function() {
Expand All @@ -159,17 +187,6 @@ describe('aria-allowed-role', function() {
assert.isFalse(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
// assert.deepEqual(checkContext._data, ['presentation']);
});

it('should not allow a <link> with a href to have any invalid role', function() {
var node = document.createElement('link');
node.setAttribute('role', 'invalid-role');
node.href = '\\example.com';
fixture.appendChild(node);
assert.isTrue(
checks['aria-allowed-role'].evaluate.call(checkContext, node)
);
});

it('should allow <select> without a multiple and size attribute to have a menu role', function() {
Expand Down
18 changes: 18 additions & 0 deletions test/commons/aria/get-element-unallowed-roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,22 @@ describe('aria.getElementUnallowedRoles', function() {
var actual = axe.commons.aria.getElementUnallowedRoles(node);
assert.isEmpty(actual);
});

it('returns false when role is implicit and allowImplicit is true (default)', function() {
var node = document.createElement('input');
var role = 'textbox';
node.setAttribute('role', role);
var actual = axe.commons.aria.getElementUnallowedRoles(node, true);
assert.isEmpty(actual);
});

it('returns false with implicit role of row for TR when allowImplicit is set to false via options', function() {
var node = document.createElement('table');
node.setAttribute('role', 'grid');
var row = document.createElement('tr');
row.setAttribute('role', 'row');
var actual = axe.commons.aria.getElementUnallowedRoles(row, false);
assert.isNotEmpty(actual);
assert.include(actual, 'row');
});
});
12 changes: 10 additions & 2 deletions test/integration/rules/aria-allowed-role/aria-allowed-role.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<section id="pass-section-role-doc-bib" role="doc-bibliography"></section>
<ul><li id='pass-li-role-doc-biblioentry' role="doc-biblioentry"></li></ul>
<aside id='pass-aside-doc-example' role='doc-example'></aside>
<div id='pass-div-has-any-role' role='divAnyRoleEvenInvalid'></div>
<div id='pass-div-has-any-role' role='contentinfo'></div>
<div id="pass-div-valid-role" role="link">ok</div>
<ol id="pass-ol-valid-role" role="directory"></ol>
<nav id="pass-nav-role-doc-index" role="doc-index"></nav>
Expand All @@ -27,6 +27,12 @@ <h1 id="pass-h1-role-doc-subtitle" role="doc-subtitle"></h1>
<header id='pass-header-valid-role' role="group"></header>
<footer id='pass-footer-valid-role' role="group"></footer>
<embed id='pass-embed-valid-role' role='img'>
<input type="text" role="textbox" id="pass-input-text-redundant-role"/>
<input type="text" role="textbox combobox" id="pass-input-multiple-roles"/>
<input type="text" role="searchbox invalidrole" id="pass-input-multiple-valid-and-invalid-roles"/>
<input aria-autocomplete="list" aria-label="some label" autocomplete="off" role="searchbox" type="text" aria-expanded="true" id='pass-input-text-role-searchbox'>
<input aria-autocomplete="list" aria-label="some label" autocomplete="off" role="combobox" type="text" aria-expanded="true" id='pass-input-text-role-combobox'>
<input aria-autocomplete="list" aria-label="some label" autocomplete="off" role="spinbutton" type="text" aria-expanded="true" id='pass-input-text-role-spinbutton'>
<input type="image" role="link" id="pass-input-image-valid-role">
<input type="checkbox" role='menuitemcheckbox' id='pass-input-checkbox-valid-role' >
<h1 id='pass-h1-valid-role' role='none'></h1>
Expand All @@ -46,4 +52,6 @@ <h1 id='pass-h1-valid-role' role='none'></h1>
<input type="image" id="fail-input-image-invalid-role" role='doc-afterword'>
<button id="fail-button-role-cell" role='cell'></button>
<aside id='fail-aside-doc-foreword' role='doc-foreword'></aside>
<aside id="fail-aside-role-tab" role='tab'></aside>
<aside id="fail-aside-role-tab" role='tab'></aside>
<button id='fail-button-role-gridcell' role="gridcell" title="IconCheckmark" aria-label="IconCheckmark icon"></button>
<input id='fail-input-role-gridcell-multiple-role' role="gridcell combobox">
12 changes: 10 additions & 2 deletions test/integration/rules/aria-allowed-role/aria-allowed-role.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
["#pass-section-role-doc-bib"],
["#pass-li-role-doc-biblioentry"],
["#pass-aside-doc-example"],
["#pass-div-has-any-role"],
["#pass-div-valid-role"],
["#pass-ol-valid-role"],
["#pass-nav-role-doc-index"],
Expand All @@ -31,6 +30,13 @@
["#pass-header-valid-role"],
["#pass-footer-valid-role"],
["#pass-embed-valid-role"],
["#pass-div-has-any-role"],
["#pass-input-text-redundant-role"],
["#pass-input-multiple-roles"],
["#pass-input-multiple-valid-and-invalid-roles"],
["#pass-input-text-role-searchbox"],
["#pass-input-text-role-combobox"],
["#pass-input-text-role-spinbutton"],
["#pass-input-image-valid-role"],
["#pass-input-checkbox-valid-role"],
["#pass-h1-valid-role"],
Expand All @@ -52,6 +58,8 @@
["#fail-input-image-invalid-role"],
["#fail-button-role-cell"],
["#fail-aside-doc-foreword"],
["#fail-aside-role-tab"]
["#fail-aside-role-tab"],
["#fail-button-role-gridcell"],
["#fail-input-role-gridcell-multiple-role"]
]
}
Loading

0 comments on commit a0f9b31

Please sign in to comment.