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.

Sign up for GitHub

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(prohibited-attr): always report incomplete if there is text in the subtree #3347

Merged
merged 3 commits into from
Jan 14, 2022
Merged

fix(prohibited-attr): always report incomplete if there is text in the subtree #3347

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8096c6c
fix(prohibited-attr): always report incomplete if there is text in th…
WilcoFiers Jan 12, 2022
49e32b5
more tests
WilcoFiers Jan 13, 2022
c5fb527
Merge branch 'develop' into aria-prohibited-fix
WilcoFiers Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions lib/checks/aria/aria-prohibited-attr-evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ import standards from '../../standards';
* @memberof checks
* @return {Boolean} True if the element uses any prohibited ARIA attributes. False otherwise.
*/
function ariaProhibitedAttrEvaluate(node, options = {}, virtualNode) {
const extraElementsAllowedAriaLabel = options.elementsAllowedAriaLabel || [];

const prohibitedList = listProhibitedAttrs(
virtualNode,
extraElementsAllowedAriaLabel
);
export default function ariaProhibitedAttrEvaluate(node, options = {}, virtualNode) {
const elementsAllowedAriaLabel = options?.elementsAllowedAriaLabel || [];
const { nodeName } = virtualNode.props;
const role = getRole(virtualNode, { chromium: true });

const prohibitedList = listProhibitedAttrs(role, nodeName, elementsAllowedAriaLabel);
const prohibited = prohibitedList.filter(attrName => {
if (!virtualNode.attrNames.includes(attrName)) {
return false;
Expand All @@ -45,24 +43,26 @@ function ariaProhibitedAttrEvaluate(node, options = {}, virtualNode) {
return false;
}

this.data(prohibited);
const hasTextContent = sanitize(subtreeText(virtualNode)) !== '';
// Don't fail if there is text content to announce
return hasTextContent ? undefined : true;
let messageKey = virtualNode.hasAttr('role') ? 'hasRole' : 'noRole';
messageKey += prohibited.length > 1 ? 'Plural' : 'Singular';
this.data({ role, nodeName, messageKey, prohibited });

// `subtreeDescendant` to override namedFromContents
const textContent = subtreeText(virtualNode, { subtreeDescendant: true });
if (sanitize(textContent) !== '') {
// Don't fail if there is text content to announce
return undefined;
}
return true;
}

function listProhibitedAttrs(virtualNode, elementsAllowedAriaLabel) {
const role = getRole(virtualNode, { chromium: true });
function listProhibitedAttrs(role, nodeName, elementsAllowedAriaLabel) {
const roleSpec = standards.ariaRoles[role];
if (roleSpec) {
return roleSpec.prohibitedAttrs || [];
}

const { nodeName } = virtualNode.props;
if (!!role || elementsAllowedAriaLabel.includes(nodeName)) {
return [];
}
return ['aria-label', 'aria-labelledby'];
}

export default ariaProhibitedAttrEvaluate;
14 changes: 12 additions & 2 deletions lib/checks/aria/aria-prohibited-attr.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,18 @@
"impact": "serious",
"messages": {
"pass": "ARIA attribute is allowed",
"fail": "ARIA attribute: ${data.values} is not allowed. Use a different role attribute or element.",
"incomplete": "ARIA attribute: ${data.values} is not well supported. Use a different role attribute or element."
"fail": {
"hasRolePlural": "${data.prohibited} attributes cannot be used with role \"${data.role}\".",
"hasRoleSingular": "${data.prohibited} attribute cannot be used with role \"${data.role}\".",
"noRolePlural": "${data.prohibited} attributes cannot be used on a ${data.nodeName} with no valid role attribute.",
"noRoleSingular": "${data.prohibited} attribute cannot be used on a ${data.nodeName} with no valid role attribute."
},
"incomplete": {
"hasRoleSingular": "${data.prohibited} attribute is not well supported with role \"${data.role}\".",
"hasRolePlural": "${data.prohibited} attributes are not well supported with role \"${data.role}\".",
"noRoleSingular": "${data.prohibited} attribute is not well supported on a ${data.nodeName} with no valid role attribute.",
"noRolePlural": "${data.prohibited} attributes are not well supported on a ${data.nodeName} with no valid role attribute."
}
}
}
}
88 changes: 77 additions & 11 deletions test/checks/aria/aria-prohibited-attr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,105 @@ describe('aria-prohibited-attr', function() {
checkContext.reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Need tests for checking when the messageKey is set to noRoleSingular when returned as false and undefined

});

it('should return true for prohibited attributes', function() {
it('should return true for prohibited attributes and no content', function() {
var params = checkSetup(
'<div id="target" role="code" aria-hidden="false" aria-label="foo">Contents</div>'
'<div id="target" role="code" aria-hidden="false" aria-label="foo"></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, ['aria-label']);
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: 'code',
messageKey: 'hasRoleSingular',
prohibited: ['aria-label']
});
});

it('should return undefined for prohibited attributes and content', function() {
var params = checkSetup(
'<div id="target" role="code" aria-hidden="false" aria-label="foo">Contents</div>'
);
assert.isUndefined(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: 'code',
messageKey: 'hasRoleSingular',
prohibited: ['aria-label']
});
});

it('should return true for multiple prohibited attributes', function() {
var params = checkSetup(
'<div id="target" role="code" aria-hidden="false" aria-label="foo" aria-labelledby="foo">Contents</div>'
'<div id="target" role="code" aria-hidden="false" aria-label="foo" aria-labelledby="foo"></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: 'code',
messageKey: 'hasRolePlural',
// attribute order not important
prohibited: [
'aria-label',
'aria-labelledby'
]
});
});

// attribute order not important
assert.sameDeepMembers(checkContext._data, [
'aria-label',
'aria-labelledby'
]);
it('should return undefined if element has no role and has text content (singular)', function() {
var params = checkSetup(
'<div id="target" aria-label="foo">Contents</div>'
);
assert.isUndefined(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: null,
messageKey: 'noRoleSingular',
prohibited: ['aria-label']
});
});

it('should return undefined if element has no role and has text content', function() {
it('should return undefined if element has no role and has text content (plural)', function() {
var params = checkSetup(
'<div id="target" aria-label="foo" aria-labelledby="foo">Contents</div>'
);
assert.isUndefined(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: null,
messageKey: 'noRolePlural',
prohibited: [
'aria-label',
'aria-labelledby'
]
});
});

it('should return true if element has no role and no text content (singular)', function() {
var params = checkSetup(
'<div id="target" aria-label="foo"></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: null,
messageKey: 'noRoleSingular',
prohibited: ['aria-label']
});
});

it('should return true if element has no role and no text content', function() {
it('should return true if element has no role and no text content (plural)', function() {
var params = checkSetup(
'<div id="target" aria-label="foo" aria-labelledby="foo"></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, params));
assert.deepEqual(checkContext._data, {
nodeName: 'div',
role: null,
messageKey: 'noRolePlural',
prohibited: [
'aria-label',
'aria-labelledby'
]
});
});

it('should return false if all attributes are allowed', function() {
Expand Down
52 changes: 26 additions & 26 deletions test/integration/rules/aria-allowed-attr/failures.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
<div role="alert" aria-selected="true" id="fail1">fail</div>
<div role="link" aria-selected="true" id="fail2">fail</div>
<div role="row" aria-colcount="value" id="fail3">fail</div>
<div role="row" aria-rowcount="value" id="fail4">fail</div>
<div role="caption" aria-label="value" id="fail5">fail</div>
<div role="caption" aria-labelledby="value" id="fail6">fail</div>
<div role="code" aria-label="value" id="fail7">fail</div>
<div role="code" aria-labelledby="value" id="fail8">fail</div>
<div role="deletion" aria-label="value" id="fail9">fail</div>
<div role="deletion" aria-labelledby="value" id="fail10">fail</div>
<div role="emphasis" aria-label="value" id="fail11">fail</div>
<div role="emphasis" aria-labelledby="value" id="fail12">fail</div>
<div role="insertion" aria-label="value" id="fail13">fail</div>
<div role="insertion" aria-labelledby="value" id="fail14">fail</div>
<div role="paragraph" aria-label="value" id="fail15">fail</div>
<div role="paragraph" aria-labelledby="value" id="fail16">fail</div>
<div role="strong" aria-label="value" id="fail17">fail</div>
<div role="strong" aria-labelledby="value" id="fail18">fail</div>
<div role="subscript" aria-label="value" id="fail19">fail</div>
<div role="subscript" aria-labelledby="value" id="fail20">fail</div>
<div role="superscript" aria-label="value" id="fail21">fail</div>
<div role="superscript" aria-labelledby="value" id="fail22">fail</div>
<div role="alert" aria-selected="true" id="fail1"></div>
<div role="link" aria-selected="true" id="fail2"></div>
<div role="row" aria-colcount="value" id="fail3"></div>
<div role="row" aria-rowcount="value" id="fail4"></div>
<div role="caption" aria-label="value" id="fail5"></div>
<div role="caption" aria-labelledby="value" id="fail6"></div>
<div role="code" aria-label="value" id="fail7"></div>
<div role="code" aria-labelledby="value" id="fail8"></div>
<div role="deletion" aria-label="value" id="fail9"></div>
<div role="deletion" aria-labelledby="value" id="fail10"></div>
<div role="emphasis" aria-label="value" id="fail11"></div>
<div role="emphasis" aria-labelledby="value" id="fail12"></div>
<div role="insertion" aria-label="value" id="fail13"></div>
<div role="insertion" aria-labelledby="value" id="fail14"></div>
<div role="paragraph" aria-label="value" id="fail15"></div>
<div role="paragraph" aria-labelledby="value" id="fail16"></div>
<div role="strong" aria-label="value" id="fail17"></div>
<div role="strong" aria-labelledby="value" id="fail18"></div>
<div role="subscript" aria-label="value" id="fail19"></div>
<div role="subscript" aria-labelledby="value" id="fail20"></div>
<div role="superscript" aria-label="value" id="fail21"></div>
<div role="superscript" aria-labelledby="value" id="fail22"></div>
<div aria-label="value" id="fail23"></div>
<div aria-labelledby="value" id="fail24"></div>
<!-- technically presentation and none roles do not allow aria-label and aria-labelledby, but since those are global attributes the presentation role conflict will not resolve the roles to none or presentation -->
Expand All @@ -34,10 +34,10 @@
aria-orientation="horizontal"
id="fail30"
></audio> -->
<div role="mark" aria-label="value" id="fail31">fail</div>
<div role="mark" aria-labelledby="value" id="fail32">fail</div>
<div role="suggestion" aria-label="value" id="fail33">fail</div>
<div role="suggestion" aria-labelledby="value" id="fail34">fail</div>
<div role="mark" aria-label="value" id="fail31"></div>
<div role="mark" aria-labelledby="value" id="fail32"></div>
<div role="suggestion" aria-label="value" id="fail33"></div>
<div role="suggestion" aria-labelledby="value" id="fail34"></div>

<div role="table">
<div role="row" aria-expanded="false" id="fail35"></div>
Expand Down
1 change: 1 addition & 0 deletions test/integration/rules/aria-allowed-attr/incomplete.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
<div id="incomplete0" aria-label="foo">Foo</div>
<div id="incomplete1" aria-labelledby="missing">Foo</div>
<div id="incomplete2" aria-label="foo" role="code">Foo</div>
2 changes: 1 addition & 1 deletion test/integration/rules/aria-allowed-attr/incomplete.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"description": "aria-allowed-attr incomplete tests",
"rule": "aria-allowed-attr",
"incomplete": [["#incomplete0"], ["#incomplete1"]]
"incomplete": [["#incomplete0"], ["#incomplete1"], ["#incomplete2"]]
}