Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better unsupported attribute support for aria-roledescription #1382

Merged
merged 11 commits into from
Feb 28, 2019
42 changes: 39 additions & 3 deletions build/tasks/aria-supported.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module.exports = function(grunt) {
return [...out, ...Object.keys(rule.props)];
}, []);
const aQaria = new Set(axe.utils.uniqueArray(roleAriaKeys, ariaKeys));
let footnotes;

/**
* Given a `base` Map and `subject` Map object,
Expand Down Expand Up @@ -60,6 +61,33 @@ module.exports = function(grunt) {
!subject.hasOwnProperty(key)
) {
out.push([`${key}`, 'No']);
} else if (
straker marked this conversation as resolved.
Show resolved Hide resolved
subject[key] &&
subject[key].unsupported &&
subject[key].unsupported.exceptions
) {
out.push([`${key}`, `Mixed[^${footnotes.length + 1}]`]);

let supportedElements = subject[
key
].unsupported.exceptions.map(element => {
if (typeof element === 'string') {
return `\`<${element}>\``;
} else if (element.nodeName && element.properties) {
return Object.keys(element.properties).map(prop => {
const value = element.properties[prop];
if (typeof value === 'string') {
return `\`<${element.nodeName} ${prop}="${value}">\``;
} else {
let values = value.map(v => `"${v}"`).join(' | ');
return `\`<${element.nodeName} ${prop}=${values}>\``;
}
});
}
});
footnotes.push(
'Supported on elements: ' + supportedElements.join(', ')
);
}
break;
case 'all':
Expand All @@ -77,11 +105,18 @@ module.exports = function(grunt) {
}, []);
};

const getMdContent = (heading, rolesTable, attributesTable) => {
return `${heading}\n\n## Roles\n\n${rolesTable}\n\n## Attributes\n\n${attributesTable}`;
const getMdContent = (
heading,
rolesTable,
attributesTable,
footnotes
) => {
return `${heading}\n\n## Roles\n\n${rolesTable}\n\n## Attributes\n\n${attributesTable}\n\n${footnotes}`;
};

const generateDoc = () => {
footnotes = [];

const content = getMdContent(
`# ARIA Roles and Attributes ${
listType === 'all' ? 'available' : listType
Expand All @@ -102,7 +137,8 @@ module.exports = function(grunt) {
mdTable([
['aria-attribute', 'axe-core support'],
...getDiff(aQaria, axe.commons.aria.lookupTable.attributes)
])
]),
footnotes.map((footnote, index) => `[^${index + 1}]: ${footnote}`)
);

// Format the content so Prettier doesn't create a diff after running.
Expand Down
4 changes: 3 additions & 1 deletion doc/aria-supported.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ For a detailed description about how accessibility support is decided, see [How
| -------------------- | ---------------- |
| aria-describedat | No |
| aria-details | No |
| aria-roledescription | No |
straker marked this conversation as resolved.
Show resolved Hide resolved
| aria-roledescription | Mixed[^1] |

[^1]: Supported on elements: `<button>`, `<input type="button" | "checkbox" | "image" | "radio" | "reset" | "submit">`, `<img>`, `<select>`, `<summary>`
39 changes: 28 additions & 11 deletions lib/checks/aria/unsupportedattr.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
let unsupported = Array.from(node.attributes)
.filter(candidate => {
// filter out unsupported attributes
return axe.commons.aria.validateAttr(candidate.name, {
flagUnsupported: true
});
const nodeName = node.nodeName.toUpperCase();
const lookupTable = axe.commons.aria.lookupTable;
const role = axe.commons.aria.getRole(node);

const unsupportedAttrs = Array.from(node.attributes)
.filter(({ name }) => {
const attribute = lookupTable.attributes[name];
if (!axe.commons.aria.validateAttr(name) || !attribute.unsupported) {
return false;
}

const unsupported = attribute.unsupported;

if (typeof unsupported === 'boolean' || !unsupported.exceptions) {
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, rather than checking if unsupported is falsey on line 8, I think this code would be a little easier to read if you check on line 14 if unsupported is not an object, and on line 14 do return !!unsupported.

}

// validate attributes and conditions (if any) from allowedElement to given node
let out = axe.commons.matches(node, unsupported.exceptions);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let out = -> const isException =


// if given node type has complex condition to evaluate a given aria-role, execute the same
if (Object.keys(lookupTable.evaluateRoleForElement).includes(nodeName)) {
return !lookupTable.evaluateRoleForElement[nodeName]({ node, role, out });
}
return !out;
})
.map(candidate => {
return candidate.name.toString();
});
.map(candidate => candidate.name.toString());

if (unsupported.length) {
this.data(unsupported);
if (unsupportedAttrs.length) {
this.data(unsupportedAttrs);
return true;
}
return false;
6 changes: 1 addition & 5 deletions lib/commons/aria/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,9 @@ aria.allowedAttr = function(role) {
* @memberof axe.commons.aria
* @instance
* @param {String} att The attribute name
* @param {Object} options Use `flagUnsupported: true` to report unsupported attributes
* @return {Boolean}
*/
aria.validateAttr = function(att, { flagUnsupported = false } = {}) {
aria.validateAttr = function(att) {
straker marked this conversation as resolved.
Show resolved Hide resolved
const attrDefinition = aria.lookupTable.attributes[att];
if (flagUnsupported && attrDefinition) {
return !!attrDefinition.unsupported;
}
return !!attrDefinition;
};
17 changes: 16 additions & 1 deletion lib/commons/aria/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,22 @@ lookupTable.attributes = {
unsupported: false
},
'aria-roledescription': {
unsupported: true
type: 'string',
allowEmpty: true,
unsupported: {
exceptions: [
'button',
{
nodeName: 'input',
properties: {
type: ['button', 'checkbox', 'image', 'radio', 'reset', 'submit']
}
},
'img',
'select',
'summary'
]
}
},
'aria-rowcount': {
type: 'int',
Expand Down
62 changes: 62 additions & 0 deletions test/checks/aria/unsupportedattr.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,66 @@ describe('unsupportedattr', function() {
);
assert.isFalse(check.evaluate.apply(checkContext, params));
});

it('should return false if applied to an element that matches the unsupported "exceptions" list', function() {
axe.commons.aria.lookupTable.attributes['aria-mccheddarton'] = {
unsupported: {
exceptions: ['button']
}
};
var params = checkSetup(
'<button id="target" aria-mccheddarton="true">Contents</div>'
);
assert.isFalse(check.evaluate.apply(checkContext, params));
});

it('should return false if applied to an element that matches the unsupported "exceptions" list using complex conditions', function() {
axe.commons.aria.lookupTable.attributes['aria-mccheddarton'] = {
unsupported: {
exceptions: [
{
nodeName: 'input',
properties: {
type: 'checkbox'
}
}
]
}
};
var params = checkSetup(
'<input type="checkbox" id="target" aria-mccheddarton="true">Contents</div>'
);
assert.isFalse(check.evaluate.apply(checkContext, params));
});

it('should return true if applied to an element that does not match the unsupported "exceptions" list', function() {
axe.commons.aria.lookupTable.attributes['aria-mccheddarton'] = {
unsupported: {
exceptions: ['button']
}
};
var params = checkSetup(
'<div id="target" aria-mccheddarton="true">Contents</div>'
);
assert.isTrue(check.evaluate.apply(checkContext, params));
});

it('should return true if applied to an element that does not match the unsupported "exceptions" list using complex conditions', function() {
axe.commons.aria.lookupTable.attributes['aria-mccheddarton'] = {
unsupported: {
exceptions: [
{
nodeName: 'input',
properties: {
type: 'checkbox'
}
}
]
}
};
var params = checkSetup(
'<input type="radio" id="target" aria-mccheddarton="true">Contents</div>'
);
assert.isTrue(check.evaluate.apply(checkContext, params));
});
});
17 changes: 16 additions & 1 deletion test/checks/aria/valid-attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('aria-valid-attr', function() {
assert.deepEqual(checkContext._data, ['aria-cats', 'aria-dogs']);
});

it('should return false if no invalid ARIA attributes are found', function() {
it('should return true if no invalid ARIA attributes are found', function() {
var node = document.createElement('div');
node.id = 'test';
node.tabIndex = 1;
Expand Down Expand Up @@ -54,6 +54,21 @@ describe('aria-valid-attr', function() {
axe.commons.aria.validateAttr = orig;
});

it('should return true for unsupported ARIA attributes', function() {
axe.commons.aria.lookupTable.attributes['aria-mccheddarton'] = {
unsupported: true
};

var node = document.createElement('div');
node.id = 'test';
node.tabIndex = 1;
node.setAttribute('aria-mccheddarton', 'true');
fixture.appendChild(node);

assert.isTrue(checks['aria-valid-attr'].evaluate.call(checkContext, node));
assert.isNull(checkContext._data);
});

describe('options', function() {
it('should exclude provided attribute names', function() {
fixture.innerHTML =
Expand Down