Skip to content

Commit

Permalink
WIP: migrate aria-errormessage to its own check
Browse files Browse the repository at this point in the history
Integration tests are failing, but I couldn't figure out why. The union of the two checks seems to be causing problems
  • Loading branch information
Marcy Sutton committed Oct 12, 2017
1 parent cb8e39c commit ae6d887
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 39 deletions.
25 changes: 25 additions & 0 deletions lib/checks/aria/errormessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
options = Array.isArray(options) ? options : [];

var attr = node.getAttribute('aria-errormessage'),
hasAttr = node.hasAttribute('aria-errormessage');

var doc = axe.commons.dom.getRootNode(node);

function validateAttrValue() {
var idref = attr && doc.getElementById(attr);
if (idref) {
return idref.getAttribute('role') === 'alert' ||
idref.getAttribute('aria-live') === 'assertive' ||
axe.utils.tokenList(node.getAttribute('aria-describedby') || '').indexOf(attr) > -1;
}
}

// limit results to elements that actually have this attribute
if (options.indexOf(attr) === -1 && hasAttr) {
if (!validateAttrValue()) {
this.data(attr);
return false;
}

return true;
}
11 changes: 11 additions & 0 deletions lib/checks/aria/errormessage.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"id": "aria-errormessage",
"evaluate": "errormessage.js",
"metadata": {
"impact": "critical",
"messages": {
"pass": "Uses a supported aria-errormessage technique",
"fail": "aria-errormessage value{{=it.data && it.data.length > 1 ? 's' : ''}} {{~it.data:value}} `{{=value}}{{~}}` must use a technique to announce the message (e.g., aria-live, aria-describedby, role=alert, etc.)"
}
}
}
11 changes: 8 additions & 3 deletions lib/checks/aria/valid-attr-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ var invalid = [],
var attr, attrName,
attrs = node.attributes;

var skipAttrs = ['aria-errormessage'];

for (var i = 0, l = attrs.length; i < l; i++) {
attr = attrs[i];
attrName = attr.name;
if (options.indexOf(attrName) === -1 && aria.test(attrName) &&
!axe.commons.aria.validateAttrValue(node, attrName)) {
// skip any attributes handled elsewhere
if (!skipAttrs.includes(attrName)) {
if (options.indexOf(attrName) === -1 && aria.test(attrName) &&
!axe.commons.aria.validateAttrValue(node, attrName)) {

invalid.push(attrName + '="' + attr.nodeValue + '"');
invalid.push(attrName + '="' + attr.nodeValue + '"');
}
}
}

Expand Down
20 changes: 1 addition & 19 deletions lib/commons/aria/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,6 @@ aria.validateAttr = function (att) {
return !!lookupTables.attributes[att];
};

/**
* Validate the value of an ARIA attribute with an idref
* @param {HTMLElement} doc Root element
* @param {HTMLElement} node The element to check
* @param {String} attr The name of the attribute
* @param {String} value The value of the attribute
* @return {Boolean}
*/
aria.validateIdrefType = function (doc, node, attr, value) {
var idref = value && doc.getElementById(value);
if (idref && attr === 'aria-errormessage') {
return idref.getAttribute('role') === 'alert' ||
idref.getAttribute('aria-live') === 'assertive' ||
axe.utils.tokenList(node.getAttribute('aria-describedby') || '').indexOf(value) > -1;
}
return !!idref;
};

/**
* Validate the value of an ARIA attribute
* @param {HTMLElement} node The element to check
Expand Down Expand Up @@ -87,7 +69,7 @@ aria.validateAttrValue = function (node, attr) {
}, list.length !== 0);

case 'idref':
return aria.validateIdrefType(doc, node, attr, value);
return !!(value && doc.getElementById(value));

case 'idrefs':
list = axe.utils.tokenList(value);
Expand Down
1 change: 1 addition & 0 deletions lib/rules/aria-valid-attr-value.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"all": [],
"any": [
"aria-errormessage",

This comment has been minimized.

Copy link
@AutoSponge

AutoSponge Oct 13, 2017

Contributor

move both of these to the all array. Currently, one of them can short-circuit the testing logic and it will mess up the integration tests.

"aria-valid-attr-value"
],
"none": []
Expand Down
49 changes: 49 additions & 0 deletions test/checks/aria/errormessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
describe('aria-errormessage', function () {
'use strict';

var fixture = document.getElementById('fixture');

var checkContext = axe.testUtils.MockCheckContext();

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

it('should return false if aria-errormessage value is invalid', function () {
var testHTML = '<div></div>';
testHTML += '<div id="plain"></div>';
fixture.innerHTML = testHTML;
var target = fixture.children[0];
target.setAttribute('aria-errormessage', 'plain');
assert.isFalse(checks['aria-errormessage'].evaluate.call(checkContext, target));
});

it('should return true if aria-errormessage id is alert', function () {
var testHTML = '<div></div>';
testHTML += '<div id="alert" role="alert"></div>';
fixture.innerHTML = testHTML;
var target = fixture.children[0];
target.setAttribute('aria-errormessage', 'alert');
assert.isTrue(checks['aria-errormessage'].evaluate.call(checkContext, target));
});

it('should return true if aria-errormessage id is aria-live=assertive', function () {
var testHTML = '<div></div>';
testHTML += '<div id="live" aria-live="assertive"></div>';
fixture.innerHTML = testHTML;
var target = fixture.children[0];
target.setAttribute('aria-errormessage', 'live');
assert.isTrue(checks['aria-errormessage'].evaluate.call(checkContext, target));
});

it('should return true if aria-errormessage id is aria-describedby', function () {
var testHTML = '<div></div>';
testHTML += '<div id="plain"></div>';
fixture.innerHTML = testHTML;
var target = fixture.children[0];
target.setAttribute('aria-errormessage', 'plain');
target.setAttribute('aria-describedby', 'plain');
assert.isTrue(checks['aria-errormessage'].evaluate.call(checkContext, target));
});
});
17 changes: 0 additions & 17 deletions test/checks/aria/valid-attr-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,6 @@ describe('aria-valid-attr-value', function () {
assert.isNull(checkContext._data);
});

it('should return true if aria-errormessage id is alert or aria-describedby', function () {
var testHTML = '<div></div>';
testHTML += '<div id="plain"></div>';
testHTML += '<div id="live" aria-live="assertive"></div>';
testHTML += '<div id="alert" role="alert"></div>';
fixture.innerHTML = testHTML;
var target = fixture.children[0];
target.setAttribute('aria-errormessage', 'plain');
assert.isFalse(checks['aria-valid-attr-value'].evaluate.call(checkContext, target));
target.setAttribute('aria-errormessage', 'live');
assert.isTrue(checks['aria-valid-attr-value'].evaluate.call(checkContext, target));
target.setAttribute('aria-errormessage', 'alert');
assert.isTrue(checks['aria-valid-attr-value'].evaluate.call(checkContext, target));
target.setAttribute('aria-describedby', 'plain');
assert.isTrue(checks['aria-valid-attr-value'].evaluate.call(checkContext, target));
});

it('should return false if any values are invalid', function () {
var node = document.createElement('div');
node.id = 'test';
Expand Down

0 comments on commit ae6d887

Please sign in to comment.