From ffae2c00946751d4d1c009a5f905bd33388485e8 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Thu, 18 Jan 2018 13:45:10 +0100 Subject: [PATCH] feat: allow options in aria-allowed-attr, aria-required-attr (#673) * chore: rename lut in tests * feat: allow options in aria-allowed-attr * chore: rename options integ. tests for clarity * feat: add required-attr options, integration tests * chore: PR feedback for aria-allowed-attr * feat: use object for ARIA check options e.g. {separator: ['aria-valuenow', 'aria-valuemin', aria-valuemax']} * chore: remove duplicate to-array util The exact same code existed in axe.utils and axe.commons (as axe.utils.toArray) * chore: simplify ARIA options usage --- lib/checks/aria/allowed-attr.js | 10 +++- lib/checks/aria/required-attr.js | 5 ++ lib/commons/utils/to-array.js | 14 ------ lib/core/utils/to-array.js | 15 +++++- test/checks/aria/allowed-attr.js | 47 +++++++++++++++++ test/checks/aria/required-attr.js | 21 ++++++++ test/commons/aria/roles.js | 6 +-- test/commons/utils/to-array.js | 26 ---------- test/core/utils/to-array.js | 15 +++++- .../configure-options/configure-options.html | 25 ++++++++++ .../configure-options/configure-options.js | 50 +++++++++++++++++++ .../frames/frame.html | 0 .../options-parameter.html} | 2 +- .../options-parameter.js} | 2 +- 14 files changed, 190 insertions(+), 48 deletions(-) delete mode 100644 lib/commons/utils/to-array.js delete mode 100644 test/commons/utils/to-array.js create mode 100644 test/integration/full/configure-options/configure-options.html create mode 100644 test/integration/full/configure-options/configure-options.js rename test/integration/full/{options => options-parameter}/frames/frame.html (100%) rename test/integration/full/{options/options.html => options-parameter/options-parameter.html} (95%) rename test/integration/full/{options/options.js => options-parameter/options-parameter.js} (99%) diff --git a/lib/checks/aria/allowed-attr.js b/lib/checks/aria/allowed-attr.js index 3a9f614610..63d889c5f3 100644 --- a/lib/checks/aria/allowed-attr.js +++ b/lib/checks/aria/allowed-attr.js @@ -1,3 +1,5 @@ +options = options || {}; + var invalid = []; var attr, attrName, allowed, @@ -7,12 +9,18 @@ var attr, attrName, allowed, if (!role) { role = axe.commons.aria.implicitRole(node); } + allowed = axe.commons.aria.allowedAttr(role); + +if (Array.isArray(options[role])) { + allowed = axe.utils.uniqueArray(options[role].concat(allowed)); +} + if (role && allowed) { for (var i = 0, l = attrs.length; i < l; i++) { attr = attrs[i]; attrName = attr.name; - if (axe.commons.aria.validateAttr(attrName) && allowed.indexOf(attrName) === -1) { + if (axe.commons.aria.validateAttr(attrName) && !allowed.includes(attrName)) { invalid.push(attrName + '="' + attr.nodeValue + '"'); } } diff --git a/lib/checks/aria/required-attr.js b/lib/checks/aria/required-attr.js index 23bfe0a3df..74bce8d9ff 100644 --- a/lib/checks/aria/required-attr.js +++ b/lib/checks/aria/required-attr.js @@ -1,3 +1,5 @@ +options = options || {}; + var missing = []; if (node.hasAttributes()) { @@ -5,6 +7,9 @@ if (node.hasAttributes()) { role = node.getAttribute('role'), required = axe.commons.aria.requiredAttr(role); + if (Array.isArray(options[role])) { + required = axe.utils.uniqueArray(options[role], required); + } if (role && required) { for (var i = 0, l = required.length; i < l; i++) { attr = required[i]; diff --git a/lib/commons/utils/to-array.js b/lib/commons/utils/to-array.js deleted file mode 100644 index 839b267706..0000000000 --- a/lib/commons/utils/to-array.js +++ /dev/null @@ -1,14 +0,0 @@ -/* global axe */ - -/** - * Converts thing to an Array - * @method toArray - * @memberof axe.commons.utils - * @instance - * @param {NodeList|HTMLCollection|String} thing - * @return {Array} - */ -axe.utils.toArray = function (thing) { - 'use strict'; - return Array.prototype.slice.call(thing); -}; diff --git a/lib/core/utils/to-array.js b/lib/core/utils/to-array.js index 5d04fc2adc..394972cd88 100644 --- a/lib/core/utils/to-array.js +++ b/lib/core/utils/to-array.js @@ -7,4 +7,17 @@ axe.utils.toArray = function (thing) { 'use strict'; return Array.prototype.slice.call(thing); -}; \ No newline at end of file +}; + + +/** + * Creates an array without duplicate values from 2 array inputs + * @param {Array} arr1 First array + * @param {Array} arr2 Second array + * @return {Array} + */ +axe.utils.uniqueArray = (arr1, arr2) => { + return arr1.concat(arr2).filter((elem, pos, arr) => { + return arr.indexOf(elem) === pos; + }); +}; diff --git a/test/checks/aria/allowed-attr.js b/test/checks/aria/allowed-attr.js index 7ff80fd54e..5ce187f9f9 100644 --- a/test/checks/aria/allowed-attr.js +++ b/test/checks/aria/allowed-attr.js @@ -119,4 +119,51 @@ describe('aria-allowed-attr', function () { assert.isNull(checkContext._data); }); + describe('options', function () { + it('should allow provided attribute names for a role', function () { + axe.commons.aria.lookupTable.role.mcheddarton = { + type: 'widget', + attributes: { + allowed: ['aria-checked'] + }, + owned: null, + nameFrom: ['author'], + context: null + }; + fixture.innerHTML = '
'; + var target = fixture.children[0]; + assert.isTrue(checks['aria-allowed-attr'].evaluate.call(checkContext, target, {'mccheddarton': ['aria-checked', 'aria-snuggles']})); + delete axe.commons.aria.lookupTable.role.mccheddarton; + }); + + it('should handle multiple roles provided in options', function () { + axe.commons.aria.lookupTable.role.mcheddarton = { + type: 'widget', + attributes: { + allowed: ['aria-checked'] + }, + owned: null, + nameFrom: ['author'], + context: null + }; + axe.commons.aria.lookupTable.role.bagley = { + type: 'widget', + attributes: { + allowed: ['aria-checked'] + }, + owned: null, + nameFrom: ['author'], + context: null + }; + fixture.innerHTML = '
'; + var target = fixture.children[0]; + var options = { + 'mccheddarton': ['aria-snuggles'], + 'bagley': ['aria-snuggles2'] + }; + assert.isTrue(checks['aria-allowed-attr'].evaluate.call(checkContext, target, options)); + delete axe.commons.aria.lookupTable.role.mccheddarton; + delete axe.commons.aria.lookupTable.role.bagley; + }); + }); }); diff --git a/test/checks/aria/required-attr.js b/test/checks/aria/required-attr.js index 8d410221da..79ec2890e2 100644 --- a/test/checks/aria/required-attr.js +++ b/test/checks/aria/required-attr.js @@ -57,4 +57,25 @@ describe('aria-required-attr', function () { axe.commons.aria.requiredAttr = orig; }); + describe('options', function () { + it('should require provided attribute names for a role', function () { + axe.commons.aria.lookupTable.role.mccheddarton = { + type: 'widget', + attributes: { + required: ['aria-valuemax'] + }, + owned: null, + nameFrom: ['author'], + context: null + }; + fixture.innerHTML = '
'; + var target = fixture.children[0]; + var options = { + 'mccheddarton': ['aria-snuggles'] + }; + assert.isFalse(checks['aria-required-attr'].evaluate.call(checkContext, target, options)); + assert.deepEqual(checkContext._data, ['aria-snuggles', 'aria-valuemax']); + delete axe.commons.aria.lookupTable.role.mccheddarton; + }); + }); }); \ No newline at end of file diff --git a/test/commons/aria/roles.js b/test/commons/aria/roles.js index 4b3f686dfc..a481eb73b5 100644 --- a/test/commons/aria/roles.js +++ b/test/commons/aria/roles.js @@ -12,7 +12,7 @@ describe('aria.isValidRole', function () { }); - it('should return false if role is not found in the lut', function () { + it('should return false if role is not found in the lookup table', function () { assert.isFalse(axe.commons.aria.isValidRole('cats')); }); @@ -57,7 +57,7 @@ describe('aria.getRolesByType', function () { }); - it('should return empty array if role is not found in the lut', function () { + it('should return empty array if role is not found in the lookup table', function () { assert.deepEqual(axe.commons.aria.getRolesByType('blahblahblah'), []); }); }); @@ -77,7 +77,7 @@ describe('aria.getRoleType', function () { }); - it('should return null if role is not found in the lut', function () { + it('should return null if role is not found in the lookup table', function () { assert.isNull(axe.commons.aria.getRoleType('cats')); }); }); diff --git a/test/commons/utils/to-array.js b/test/commons/utils/to-array.js deleted file mode 100644 index aa8dac324a..0000000000 --- a/test/commons/utils/to-array.js +++ /dev/null @@ -1,26 +0,0 @@ -describe('utils.toArray', function () { - 'use strict'; - - it('should call Array.prototype.slice', function () { - var orig = Array.prototype.slice, - called = false, - arrayLike = {'0': 'cats', length: 1}; - - Array.prototype.slice = function () { - called = true; - assert.equal(this, arrayLike); - }; - - axe.commons.utils.toArray(arrayLike); - - Array.prototype.slice = orig; - - assert.isTrue(called); - }); - it('should return an array', function () { - var arrayLike = {'0': 'cats', length: 1}; - - var result = axe.commons.utils.toArray(arrayLike); - assert.isArray(result); - }); -}); \ No newline at end of file diff --git a/test/core/utils/to-array.js b/test/core/utils/to-array.js index 315331b750..133d432a8e 100644 --- a/test/core/utils/to-array.js +++ b/test/core/utils/to-array.js @@ -1,6 +1,5 @@ describe('axe.utils.toArray', function () { 'use strict'; - it('should call Array.prototype.slice', function () { var orig = Array.prototype.slice, called = false, @@ -24,4 +23,18 @@ describe('axe.utils.toArray', function () { var result = axe.utils.toArray(arrayLike); assert.isArray(result); }); + +}); + +describe('axe.utils.uniqueArray', function () { + 'use strict'; + + it('should filter duplicate values', function () { + var array1 = [1, 2, 3, 4, 5]; + var array2 = [1, 3, 7]; + + var result = axe.utils.uniqueArray(array1, array2); + assert.isArray(result); + assert.includeMembers(result, [1, 2, 3, 4, 5, 7]); + }); }); \ No newline at end of file diff --git a/test/integration/full/configure-options/configure-options.html b/test/integration/full/configure-options/configure-options.html new file mode 100644 index 0000000000..c6564d1201 --- /dev/null +++ b/test/integration/full/configure-options/configure-options.html @@ -0,0 +1,25 @@ + + + + + + + + + + + + +
+ +
+ + + + diff --git a/test/integration/full/configure-options/configure-options.js b/test/integration/full/configure-options/configure-options.js new file mode 100644 index 0000000000..9850846462 --- /dev/null +++ b/test/integration/full/configure-options/configure-options.js @@ -0,0 +1,50 @@ +describe('Check Configure Options', function() { + 'use strict'; + + var target = document.querySelector('#target'); + + describe('aria-allowed-attr', function() { + it('should allow an attribute supplied in options', function(done) { + target.setAttribute('role', 'separator'); + target.setAttribute('aria-valuenow', '0'); + + axe.configure({ + checks: [{ + id: 'aria-allowed-attr', + options: {'separator': ['aria-valuenow']} + }] + }); + axe.run(target, { + runOnly: { + type: 'rule', + values: [ 'aria-allowed-attr' ] + } + }, function(error, results) { + assert.lengthOf(results.violations, 0, 'violations'); + done(); + }); + }); + }); + + describe('aria-required-attr', function() { + it('should report unique attributes when supplied from options', function(done) { + target.setAttribute('role', 'slider'); + axe.configure({ + checks: [{ + id: 'aria-required-attr', + options: {slider: ['aria-snuggles']} + }] + }); + axe.run('#target', { + runOnly: { + type: 'rule', + values: [ 'aria-required-attr' ] + } + }, function(error, results) { + assert.lengthOf(results.violations, 1, 'violations'); + assert.sameMembers(results.violations[0].nodes[0].any[0].data, ['aria-valuemax', 'aria-valuemin', 'aria-snuggles']); + done(); + }); + }); + }); +}); diff --git a/test/integration/full/options/frames/frame.html b/test/integration/full/options-parameter/frames/frame.html similarity index 100% rename from test/integration/full/options/frames/frame.html rename to test/integration/full/options-parameter/frames/frame.html diff --git a/test/integration/full/options/options.html b/test/integration/full/options-parameter/options-parameter.html similarity index 95% rename from test/integration/full/options/options.html rename to test/integration/full/options-parameter/options-parameter.html index b4ff840854..b7db01a71d 100644 --- a/test/integration/full/options/options.html +++ b/test/integration/full/options-parameter/options-parameter.html @@ -38,7 +38,7 @@
- + diff --git a/test/integration/full/options/options.js b/test/integration/full/options-parameter/options-parameter.js similarity index 99% rename from test/integration/full/options/options.js rename to test/integration/full/options-parameter/options-parameter.js index 7e62278a4d..0e9a7eb632 100644 --- a/test/integration/full/options/options.js +++ b/test/integration/full/options-parameter/options-parameter.js @@ -1,4 +1,4 @@ -describe('Options', function() { +describe('Options parameter', function() { 'use strict'; before(function (done) {