From 119fe33a11fa6b8ad5471b717d3d70c71e878b96 Mon Sep 17 00:00:00 2001 From: Jey Date: Fri, 31 Aug 2018 06:37:16 +0100 Subject: [PATCH 1/9] fix: ignore invalid and allow redundant role in aria-allowed-role --- .../aria/get-element-unallowed-roles.js | 13 +++++++------ lib/commons/aria/get-role.js | 8 +++++++- lib/rules/aria-allowed-role-matches.js | 18 ++++++++++++++++++ lib/rules/aria-allowed-role.json | 1 + test/checks/aria/aria-allowed-role.js | 13 +++++-------- .../aria-allowed-role/aria-allowed-role.html | 1 - .../aria-allowed-role/aria-allowed-role.json | 1 - 7 files changed, 38 insertions(+), 17 deletions(-) create mode 100644 lib/rules/aria-allowed-role-matches.js diff --git a/lib/commons/aria/get-element-unallowed-roles.js b/lib/commons/aria/get-element-unallowed-roles.js index 2fbb6dd337..39131fd021 100644 --- a/lib/commons/aria/get-element-unallowed-roles.js +++ b/lib/commons/aria/get-element-unallowed-roles.js @@ -16,7 +16,7 @@ aria.getElementUnallowedRoles = function getElementUnallowedRoles( * @param {HTMLElement} node HTMLElement * @return {Array} 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 + // TODO: not moving this to outer namespace yet, work with wilco to see overlap with - aria.getRole function getRoleSegments(node) { let roles = []; if (!node) { @@ -53,11 +53,6 @@ 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 - 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'] @@ -71,6 +66,12 @@ aria.getElementUnallowedRoles = function getElementUnallowedRoles( return true; } } + + // if role and implicit role are same, it is redundant - ignore + if (role === implicitRole) { + return false; + } + if (!aria.isAriaRoleAllowedOnElement(node, role)) { return true; } diff --git a/lib/commons/aria/get-role.js b/lib/commons/aria/get-role.js index ae9aa3ce3b..e833265067 100644 --- a/lib/commons/aria/get-role.js +++ b/lib/commons/aria/get-role.js @@ -12,11 +12,12 @@ * @param {boolean} options.fallback Allow fallback roles * @param {boolean} options.abstracts Allow role to be abstract * @param {boolean} options.dpub Allow role to be any (valid) doc-* roles + * @param {boolean} options.segments Allows getting full list of roles applied * @returns {string|null} Role or null */ aria.getRole = function getRole( node, - { noImplicit, fallback, abstracts, dpub } = {} + { noImplicit, fallback, abstracts, dpub, segments } = {} ) { const roleAttr = (node.getAttribute('role') || '').trim().toLowerCase(); const roleList = fallback ? axe.utils.tokenList(roleAttr) : [roleAttr]; @@ -28,6 +29,11 @@ aria.getRole = function getRole( } return aria.isValidRole(role, { allowAbstract: abstracts }); }); + + if (segments) { + return validRoles; + } + const explicitRole = validRoles[0]; // Get the implicit role, if permitted diff --git a/lib/rules/aria-allowed-role-matches.js b/lib/rules/aria-allowed-role-matches.js new file mode 100644 index 0000000000..fb6abcbd29 --- /dev/null +++ b/lib/rules/aria-allowed-role-matches.js @@ -0,0 +1,18 @@ +const { isValidRole, getRole } = axe.commons.aria; +const roleSegments = getRole(node, { + allowImplicit: false, + segments: true, + fallback: true, + dpub: true +}); + +// trap any redundant roles +const validRoles = roleSegments.filter(role => { + return isValidRole(role); +}); + +if (!validRoles.length) { + return false; +} + +return true; diff --git a/lib/rules/aria-allowed-role.json b/lib/rules/aria-allowed-role.json index ad71e25d1e..3c0c8941a0 100644 --- a/lib/rules/aria-allowed-role.json +++ b/lib/rules/aria-allowed-role.json @@ -2,6 +2,7 @@ "id": "aria-allowed-role", "excludeHidden": false, "selector": "[role]", + "matches": "aria-allowed-role-matches.js", "tags": [ "cat.aria", "best-practice" diff --git a/test/checks/aria/aria-allowed-role.js b/test/checks/aria/aria-allowed-role.js index 181266f3a6..87fb2d1d46 100644 --- a/test/checks/aria/aria-allowed-role.js +++ b/test/checks/aria/aria-allowed-role.js @@ -135,10 +135,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 with a non-empty alt', function() { @@ -162,14 +160,13 @@ describe('aria-allowed-role', function() { // assert.deepEqual(checkContext._data, ['presentation']); }); - it('should not allow a with a href to have any invalid role', function() { + it('return false for a 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) - ); + var actual = checks['aria-allowed-role'].evaluate.call(checkContext, node); + assert.isFalse(actual); }); it('should allow without a multiple and size attribute to have a menu role', function() { diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index a17ba4e2ab..e350efdcdc 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -26,6 +26,7 @@

+

diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index bcc8b55925..ebbd7db81f 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -30,6 +30,7 @@ ["#pass-header-valid-role"], ["#pass-footer-valid-role"], ["#pass-embed-valid-role"], + ["#pass-input-text-redundant-role"], ["#pass-input-image-valid-role"], ["#pass-input-checkbox-valid-role"], ["#pass-h1-valid-role"], diff --git a/test/rule-matches/aria-allowed-role-matches.js b/test/rule-matches/aria-allowed-role-matches.js new file mode 100644 index 0000000000..cb8d383122 --- /dev/null +++ b/test/rule-matches/aria-allowed-role-matches.js @@ -0,0 +1,45 @@ +describe('aria-allowed-role-matches', function() { + 'use strict'; + + var fixture = document.getElementById('fixture'); + var rule; + + beforeEach(function() { + rule = axe._audit.rules.find(function(rule) { + return rule.id === 'aria-allowed-role'; + }); + }); + + afterEach(function() { + fixture.innerHTML = ''; + }); + + it('is a function', function() { + assert.isFunction(rule.matches); + }); + + it('return false (no matches) for a 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.isFalse(rule.matches(node)); + }); + + it('return true for input with redundant role', function() { + var node = document.createElement('input'); + node.setAttribute('type', 'text'); + node.setAttribute('role', 'textbox'); + node.href = '\\example.com'; + fixture.appendChild(node); + assert.isTrue(rule.matches(node)); + }); + + it('return true for element with valid role', function() { + var node = document.createElement('ol'); + node.setAttribute('role', 'listbox'); + node.href = '\\example.com'; + fixture.appendChild(node); + assert.isTrue(rule.matches(node)); + }); +}); From 4ea825a9fac81f6267ecc3ce0ef6db4c5d2593dd Mon Sep 17 00:00:00 2001 From: Jey Date: Mon, 3 Sep 2018 08:29:52 +0100 Subject: [PATCH 3/9] fix: allowed combobox, searchbox, spinbutton role on type input text --- lib/commons/aria/index.js | 48 +++++++++++++++---- .../aria-allowed-role/aria-allowed-role.html | 3 ++ .../aria-allowed-role/aria-allowed-role.json | 3 ++ 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/lib/commons/aria/index.js b/lib/commons/aria/index.js index d5cd6580b2..147b5599c8 100644 --- a/lib/commons/aria/index.js +++ b/lib/commons/aria/index.js @@ -420,7 +420,15 @@ lookupTable.role = { }, nameFrom: ['author'], context: null, - unsupported: false + unsupported: false, + allowedElements: [ + { + tagName: 'INPUT', + attributes: { + TYPE: 'TEXT' + } + } + ] }, command: { nameFrom: ['author'], @@ -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'], @@ -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', @@ -2148,13 +2172,13 @@ lookupTable.elementsAllowedNoRole = [ TYPE: 'TEL' } }, - { - tagName: 'INPUT', - condition: elementConditions.CANNOT_HAVE_LIST_ATTRIBUTE, - attributes: { - TYPE: 'TEXT' - } - }, + // { + // tagName: 'INPUT', + // condition: elementConditions.CANNOT_HAVE_LIST_ATTRIBUTE, + // attributes: { + // TYPE: 'TEXT' + // } + // }, { tagName: 'INPUT', attributes: { @@ -2368,6 +2392,10 @@ lookupTable.evaluateRoleForElement = { return out; case 'radio': return role === 'menuitemradio'; + case 'text': + return ( + role === 'combobox' || role === 'searchbox' || role === 'spinbutton' + ); default: return false; } diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index e350efdcdc..b3fe0efeb0 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -27,6 +27,9 @@

+ + +

diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index ebbd7db81f..bd64d75e11 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -31,6 +31,9 @@ ["#pass-footer-valid-role"], ["#pass-embed-valid-role"], ["#pass-input-text-redundant-role"], + ["#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"], From 401d41e60dd424b9a7bd2ddc25171e22de60846b Mon Sep 17 00:00:00 2001 From: Jey Date: Mon, 3 Sep 2018 08:34:14 +0100 Subject: [PATCH 4/9] test: aria allowed role unit tests --- test/checks/aria/aria-allowed-role.js | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/checks/aria/aria-allowed-role.js b/test/checks/aria/aria-allowed-role.js index 428a0cd724..34a5d37767 100644 --- a/test/checks/aria/aria-allowed-role.js +++ b/test/checks/aria/aria-allowed-role.js @@ -91,6 +91,36 @@ describe('aria-allowed-role', 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 false when MENU has type context', function() { var node = document.createElement('menu'); node.setAttribute('type', 'context'); From 2086d079a4a2b1059a38fc95aed518635d90ffb9 Mon Sep 17 00:00:00 2001 From: Jey Date: Mon, 3 Sep 2018 11:50:53 +0100 Subject: [PATCH 5/9] test: get role tests for segments --- lib/commons/aria/index.js | 7 ------- test/commons/aria/get-role.js | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/commons/aria/index.js b/lib/commons/aria/index.js index 147b5599c8..90abf48c8f 100644 --- a/lib/commons/aria/index.js +++ b/lib/commons/aria/index.js @@ -2172,13 +2172,6 @@ lookupTable.elementsAllowedNoRole = [ TYPE: 'TEL' } }, - // { - // tagName: 'INPUT', - // condition: elementConditions.CANNOT_HAVE_LIST_ATTRIBUTE, - // attributes: { - // TYPE: 'TEXT' - // } - // }, { tagName: 'INPUT', attributes: { diff --git a/test/commons/aria/get-role.js b/test/commons/aria/get-role.js index 68a32ed2bf..96b9cba29a 100644 --- a/test/commons/aria/get-role.js +++ b/test/commons/aria/get-role.js @@ -119,6 +119,21 @@ describe('aria.getRole', function() { }); }); + describe('segments', function() { + var options = { + fallback: true, + dpub: true, + segments: true + }; + + it('returns all roles from element', function() { + var node = document.createElement('input'); + node.setAttribute('role', 'textbox combobox'); + var actual = aria.getRole(node, options); + assert.deepEqual(actual, ['textbox', 'combobox']); + }); + }); + describe('fallback', function() { it('returns the first valid item in the list', function() { var node = document.createElement('div'); From dcacb517f520aa482bce7f552785ba83fa26ff85 Mon Sep 17 00:00:00 2001 From: Jey Date: Wed, 5 Sep 2018 09:51:03 +0100 Subject: [PATCH 6/9] refactor: review based code changes --- .../aria/get-element-unallowed-roles.js | 6 +-- lib/commons/aria/get-role-segments.js | 44 +++++++++++++++++ lib/commons/aria/get-role.js | 7 +-- lib/rules/aria-allowed-role-matches.js | 19 +------- test/checks/aria/aria-allowed-role.js | 2 +- test/commons/aria/get-role-matches.js | 47 +++++++++++++++++++ test/commons/aria/get-role.js | 15 ------ .../aria-allowed-role/aria-allowed-role.html | 1 + .../aria-allowed-role/aria-allowed-role.json | 1 + 9 files changed, 97 insertions(+), 45 deletions(-) create mode 100644 lib/commons/aria/get-role-segments.js create mode 100644 test/commons/aria/get-role-matches.js diff --git a/lib/commons/aria/get-element-unallowed-roles.js b/lib/commons/aria/get-element-unallowed-roles.js index 963280c87e..9f586e1c43 100644 --- a/lib/commons/aria/get-element-unallowed-roles.js +++ b/lib/commons/aria/get-element-unallowed-roles.js @@ -19,11 +19,7 @@ aria.getElementUnallowedRoles = function getElementUnallowedRoles( return []; } - const roleSegments = axe.commons.aria.getRole(node, { - segments: true, - fallback: true, - dpub: true - }); + const roleSegments = axe.commons.aria.getRoleSegments(node); const implicitRole = axe.commons.aria.implicitRole(node); // stores all roles that are not allowed for a specific element most often an element only has one explicit role diff --git a/lib/commons/aria/get-role-segments.js b/lib/commons/aria/get-role-segments.js new file mode 100644 index 0000000000..8e81c37aac --- /dev/null +++ b/lib/commons/aria/get-role-segments.js @@ -0,0 +1,44 @@ +/* global aria, axe */ + +/** + * Returns all roles applicable to element in a list + * + * @method getRoleSegments + * @memberof axe.commons.aria + * @instance + * @param {Element} node + * @returns {Array} Roles list or empty list + */ + +aria.getRoleSegments = 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 + return roles; +}; diff --git a/lib/commons/aria/get-role.js b/lib/commons/aria/get-role.js index e833265067..1ab0bae5d8 100644 --- a/lib/commons/aria/get-role.js +++ b/lib/commons/aria/get-role.js @@ -12,12 +12,11 @@ * @param {boolean} options.fallback Allow fallback roles * @param {boolean} options.abstracts Allow role to be abstract * @param {boolean} options.dpub Allow role to be any (valid) doc-* roles - * @param {boolean} options.segments Allows getting full list of roles applied * @returns {string|null} Role or null */ aria.getRole = function getRole( node, - { noImplicit, fallback, abstracts, dpub, segments } = {} + { noImplicit, fallback, abstracts, dpub } = {} ) { const roleAttr = (node.getAttribute('role') || '').trim().toLowerCase(); const roleList = fallback ? axe.utils.tokenList(roleAttr) : [roleAttr]; @@ -30,10 +29,6 @@ aria.getRole = function getRole( return aria.isValidRole(role, { allowAbstract: abstracts }); }); - if (segments) { - return validRoles; - } - const explicitRole = validRoles[0]; // Get the implicit role, if permitted diff --git a/lib/rules/aria-allowed-role-matches.js b/lib/rules/aria-allowed-role-matches.js index ac5292ddc8..fdc0040f8f 100644 --- a/lib/rules/aria-allowed-role-matches.js +++ b/lib/rules/aria-allowed-role-matches.js @@ -1,18 +1 @@ -const { isValidRole, getRole } = axe.commons.aria; - -// get role(s) as array(segments), with fallback and dpub -const roleSegments = getRole(node, { - segments: true, - fallback: true, - dpub: true -}); - -// filter invalid roles -const validRoles = roleSegments.filter(role => isValidRole(role)); - -// if no valid roles are applied, ignore -if (!validRoles.length) { - return false; -} - -return true; +return axe.commons.aria.getRoleSegments(node).length > 0; diff --git a/test/checks/aria/aria-allowed-role.js b/test/checks/aria/aria-allowed-role.js index 34a5d37767..d3a56ef325 100644 --- a/test/checks/aria/aria-allowed-role.js +++ b/test/checks/aria/aria-allowed-role.js @@ -121,7 +121,7 @@ describe('aria-allowed-role', function() { ); }); - it('returns false when MENU has type context', function() { + it('returns true when MENU has type context', function() { var node = document.createElement('menu'); node.setAttribute('type', 'context'); node.setAttribute('role', 'navigation'); diff --git a/test/commons/aria/get-role-matches.js b/test/commons/aria/get-role-matches.js new file mode 100644 index 0000000000..ee13a4cb6e --- /dev/null +++ b/test/commons/aria/get-role-matches.js @@ -0,0 +1,47 @@ +describe('aria.getRoleSegments', function() { + 'use strict'; + + var aria = axe.commons.aria; + + it('returns a list of roles', function() { + var node = document.createElement('div'); + var roles = ['presentation', 'button', 'contentinfo']; + node.setAttribute('role', roles.join(' ')); + var actual = aria.getRoleSegments(node); + assert.deepEqual(actual, roles); + }); + + it('returns a list of roles excluding invalid roles', function() { + var node = document.createElement('button'); + var roles = ['link', 'button', 'anhcor']; + var expected = ['link', 'button']; + node.setAttribute('role', roles.join(' ')); + var actual = aria.getRoleSegments(node); + assert.deepEqual(actual, expected); + }); + + it('handles case sensitivity and returns a list of roles', function() { + var node = document.createElement('div'); + var roles = ['PreSENTATION', 'button', 'ARTICLE']; + var expected = roles.map(function(r) { + return r.toLowerCase(); + }); + node.setAttribute('role', roles.join(' ')); + var actual = aria.getRoleSegments(node); + assert.deepEqual(actual, expected); + }); + + it('returns empty array when there is no role', function() { + var node = document.createElement('section'); + node.setAttribute('role', ''); + var actual = aria.getRoleSegments(node); + assert.deepEqual(actual, []); + }); + + it('handles white spaces in role(s) supplied', function() { + var node = document.createElement('section'); + node.setAttribute('role', ' contentinfo '); + var actual = aria.getRoleSegments(node); + assert.deepEqual(actual, ['contentinfo']); + }); +}); diff --git a/test/commons/aria/get-role.js b/test/commons/aria/get-role.js index 96b9cba29a..68a32ed2bf 100644 --- a/test/commons/aria/get-role.js +++ b/test/commons/aria/get-role.js @@ -119,21 +119,6 @@ describe('aria.getRole', function() { }); }); - describe('segments', function() { - var options = { - fallback: true, - dpub: true, - segments: true - }; - - it('returns all roles from element', function() { - var node = document.createElement('input'); - node.setAttribute('role', 'textbox combobox'); - var actual = aria.getRole(node, options); - assert.deepEqual(actual, ['textbox', 'combobox']); - }); - }); - describe('fallback', function() { it('returns the first valid item in the list', function() { var node = document.createElement('div'); diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index b3fe0efeb0..0fc7d94951 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -26,6 +26,7 @@

+ diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index bd64d75e11..f04018af4e 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -30,6 +30,7 @@ ["#pass-header-valid-role"], ["#pass-footer-valid-role"], ["#pass-embed-valid-role"], + ["#pass-div-has-any-role"], ["#pass-input-text-redundant-role"], ["#pass-input-text-role-searchbox"], ["#pass-input-text-role-combobox"], From 2170563302b70f3cbe5af93082b8ed16c3f544cc Mon Sep 17 00:00:00 2001 From: Jey Date: Wed, 5 Sep 2018 10:15:28 +0100 Subject: [PATCH 7/9] test: add more integration tests --- .../rules/aria-allowed-role/aria-allowed-role.html | 5 +++-- .../rules/aria-allowed-role/aria-allowed-role.json | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index 0fc7d94951..07316554ad 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -7,6 +7,7 @@
+
ok
    @@ -26,7 +27,6 @@

    - @@ -50,4 +50,5 @@

    - \ No newline at end of file + + diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index f04018af4e..4bc2460d83 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -56,6 +56,7 @@ ["#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"] ] } \ No newline at end of file From 39e77843270eca7a5d97348406a2ac39f1582fc6 Mon Sep 17 00:00:00 2001 From: Jey Date: Thu, 6 Sep 2018 09:07:33 +0100 Subject: [PATCH 8/9] refactor: based on review --- .../aria/get-element-unallowed-roles.js | 74 +++++++++++++++---- lib/rules/aria-allowed-role-matches.js | 8 +- test/commons/aria/get-role-matches.js | 47 ------------ .../aria-allowed-role/aria-allowed-role.html | 6 +- 4 files changed, 68 insertions(+), 67 deletions(-) delete mode 100644 test/commons/aria/get-role-matches.js diff --git a/lib/commons/aria/get-element-unallowed-roles.js b/lib/commons/aria/get-element-unallowed-roles.js index 9f586e1c43..d20bc0eba7 100644 --- a/lib/commons/aria/get-element-unallowed-roles.js +++ b/lib/commons/aria/get-element-unallowed-roles.js @@ -1,5 +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 @@ -19,30 +60,31 @@ aria.getElementUnallowedRoles = function getElementUnallowedRoles( return []; } - const roleSegments = axe.commons.aria.getRoleSegments(node); + const roleSegments = getRoleSegments(node); const implicitRole = axe.commons.aria.implicitRole(node); // 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 => { - // 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; - } + // if role and implicit role are same, when allowImplicit: true + // ignore as it is a redundant role + if (allowImplicit && role === implicitRole) { + return false; } - // if role and implicit role are same, it is redundant - ignore - if (role === implicitRole) { - return false; + // 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; } diff --git a/lib/rules/aria-allowed-role-matches.js b/lib/rules/aria-allowed-role-matches.js index fdc0040f8f..b3375967a3 100644 --- a/lib/rules/aria-allowed-role-matches.js +++ b/lib/rules/aria-allowed-role-matches.js @@ -1 +1,7 @@ -return axe.commons.aria.getRoleSegments(node).length > 0; +return ( + axe.commons.aria.getRole(node, { + noImplicit: true, + dpub: true, + fallback: true + }) !== null +); diff --git a/test/commons/aria/get-role-matches.js b/test/commons/aria/get-role-matches.js deleted file mode 100644 index ee13a4cb6e..0000000000 --- a/test/commons/aria/get-role-matches.js +++ /dev/null @@ -1,47 +0,0 @@ -describe('aria.getRoleSegments', function() { - 'use strict'; - - var aria = axe.commons.aria; - - it('returns a list of roles', function() { - var node = document.createElement('div'); - var roles = ['presentation', 'button', 'contentinfo']; - node.setAttribute('role', roles.join(' ')); - var actual = aria.getRoleSegments(node); - assert.deepEqual(actual, roles); - }); - - it('returns a list of roles excluding invalid roles', function() { - var node = document.createElement('button'); - var roles = ['link', 'button', 'anhcor']; - var expected = ['link', 'button']; - node.setAttribute('role', roles.join(' ')); - var actual = aria.getRoleSegments(node); - assert.deepEqual(actual, expected); - }); - - it('handles case sensitivity and returns a list of roles', function() { - var node = document.createElement('div'); - var roles = ['PreSENTATION', 'button', 'ARTICLE']; - var expected = roles.map(function(r) { - return r.toLowerCase(); - }); - node.setAttribute('role', roles.join(' ')); - var actual = aria.getRoleSegments(node); - assert.deepEqual(actual, expected); - }); - - it('returns empty array when there is no role', function() { - var node = document.createElement('section'); - node.setAttribute('role', ''); - var actual = aria.getRoleSegments(node); - assert.deepEqual(actual, []); - }); - - it('handles white spaces in role(s) supplied', function() { - var node = document.createElement('section'); - node.setAttribute('role', ' contentinfo '); - var actual = aria.getRoleSegments(node); - assert.deepEqual(actual, ['contentinfo']); - }); -}); diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index 07316554ad..36fcd844b7 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -28,9 +28,9 @@

    - - - + + +

    From a54843ee6ad61ce61aed6e36d74b80d65b2d1a52 Mon Sep 17 00:00:00 2001 From: Jey Date: Thu, 6 Sep 2018 09:40:29 +0100 Subject: [PATCH 9/9] test: add tests for aria-allowed-role --- .../aria/get-element-unallowed-roles.js | 2 +- lib/commons/aria/get-role-segments.js | 44 ------------------- .../aria/get-element-unallowed-roles.js | 18 ++++++++ .../aria-allowed-role/aria-allowed-role.html | 5 ++- .../aria-allowed-role/aria-allowed-role.json | 5 ++- 5 files changed, 27 insertions(+), 47 deletions(-) delete mode 100644 lib/commons/aria/get-role-segments.js diff --git a/lib/commons/aria/get-element-unallowed-roles.js b/lib/commons/aria/get-element-unallowed-roles.js index d20bc0eba7..b371dee71a 100644 --- a/lib/commons/aria/get-element-unallowed-roles.js +++ b/lib/commons/aria/get-element-unallowed-roles.js @@ -51,7 +51,7 @@ function getRoleSegments(node) { */ aria.getElementUnallowedRoles = function getElementUnallowedRoles( node, - allowImplicit + allowImplicit = true ) { const tagName = node.nodeName.toUpperCase(); diff --git a/lib/commons/aria/get-role-segments.js b/lib/commons/aria/get-role-segments.js deleted file mode 100644 index 8e81c37aac..0000000000 --- a/lib/commons/aria/get-role-segments.js +++ /dev/null @@ -1,44 +0,0 @@ -/* global aria, axe */ - -/** - * Returns all roles applicable to element in a list - * - * @method getRoleSegments - * @memberof axe.commons.aria - * @instance - * @param {Element} node - * @returns {Array} Roles list or empty list - */ - -aria.getRoleSegments = 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 - return roles; -}; diff --git a/test/commons/aria/get-element-unallowed-roles.js b/test/commons/aria/get-element-unallowed-roles.js index 318bd455e2..b01344243d 100644 --- a/test/commons/aria/get-element-unallowed-roles.js +++ b/test/commons/aria/get-element-unallowed-roles.js @@ -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'); + }); }); diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index 36fcd844b7..b27daccdfb 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -28,6 +28,8 @@

    + + @@ -51,4 +53,5 @@

    - + + diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index 4bc2460d83..a92c112926 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -32,6 +32,8 @@ ["#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"], @@ -57,6 +59,7 @@ ["#fail-button-role-cell"], ["#fail-aside-doc-foreword"], ["#fail-aside-role-tab"], - ["#fail-button-role-gridcell"] + ["#fail-button-role-gridcell"], + ["#fail-input-role-gridcell-multiple-role"] ] } \ No newline at end of file