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
61 changes: 58 additions & 3 deletions build/tasks/aria-supported.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,46 @@ module.exports = function(grunt) {
return [...out, ...Object.keys(rule.props)];
}, []);
const aQaria = new Set(axe.utils.uniqueArray(roleAriaKeys, ariaKeys));
let footnotes;

/**
* Parse a list of unsupported exception elements and add a footnote
* detailing which HTML elements are supported.
* @param {Array<String|Object>} elements List of supported elements
*/
const generateSupportFootnote = elements => {
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
let supportedElements = elements.map(element => {
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
if (typeof element === 'string') {
return `\`<${element}>\``;
}

/**
* if element is not a string it will be an object with structure:
{
nodeName: string,
properties: {
type: {string|string[]}
}
}
*/
return Object.keys(element.properties).map(prop => {
const value = element.properties[prop];

// the 'type' property can be a string or an array
if (typeof value === 'string') {
return `\`<${element.nodeName} ${prop}="${value}">\``;
}

// output format for an array of types:
// <input type="button" | "checkbox">
let values = value.map(v => `"${v}"`).join(' | ');
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
return `\`<${element.nodeName} ${prop}=${values}>\``;
});
});
footnotes.push(
'Supported on elements: ' + supportedElements.join(', ')
);
};

/**
* Given a `base` Map and `subject` Map object,
Expand Down Expand Up @@ -60,6 +100,13 @@ 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}]`]);
generateSupportFootnote(subject[key].unsupported.exceptions);
}
break;
case 'all':
Expand All @@ -77,11 +124,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 +156,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 validateAttr(att) {
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