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

feat(label,select-name): allow placeholder to pass label rule, add select-name rule #2448

Merged
merged 4 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
| [object-alt](https://dequeuniversity.com/rules/axe/4.0/object-alt?application=RuleDescription) | Ensures <object> elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review |
| [role-img-alt](https://dequeuniversity.com/rules/axe/4.0/role-img-alt?application=RuleDescription) | Ensures [role='img'] elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review |
| [scrollable-region-focusable](https://dequeuniversity.com/rules/axe/4.0/scrollable-region-focusable?application=RuleDescription) | Elements that have scrollable content should be accessible by keyboard | Moderate | wcag2a, wcag211 | failure |
| [select-name](https://dequeuniversity.com/rules/axe/4.0/select-name?application=RuleDescription) | Ensures select element has an accessible name | Minor, Critical | cat.forms, wcag2a, wcag412, wcag131, section508, section508.22.n | failure, needs review |
| [server-side-image-map](https://dequeuniversity.com/rules/axe/4.0/server-side-image-map?application=RuleDescription) | Ensures that server-side image maps are not used | Minor | cat.text-alternatives, wcag2a, wcag211, section508, section508.22.f | needs review |
| [svg-img-alt](https://dequeuniversity.com/rules/axe/4.0/svg-img-alt?application=RuleDescription) | Ensures svg elements with an img, graphics-document or graphics-symbol role have an accessible text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review |
| [td-headers-attr](https://dequeuniversity.com/rules/axe/4.0/td-headers-attr?application=RuleDescription) | Ensure that each cell in a table using the headers refers to another cell in that table | Serious | cat.tables, wcag2a, wcag131, section508, section508.22.g | failure, needs review |
Expand Down
14 changes: 14 additions & 0 deletions lib/checks/shared/non-empty-placeholder.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"id": "non-empty-placeholder",
"evaluate": "attr-non-space-content-evaluate",
"options": {
"attribute": "placeholder"
},
"metadata": {
"impact": "serious",
"messages": {
"pass": "Element has a placeholder attribute",
"fail": "Element has no placeholder attribute or the placeholder attribute is empty"
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets distinguish between these two. I realise this is also true for non-empty-title, I'll open a tech debt ticket for that.

Copy link
Contributor Author

@straker straker Aug 14, 2020

Choose a reason for hiding this comment

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

Doing this would make the generic check attr-non-space-content change, which in turn affects the 3 other checks that rely on it: none-empty-alt, none-empty-value, non-empty-title (the tests and messages would need to be updated in each of those to handle this.data being called in the check).

As such, this change would be outside the scope of the PR and I vote we do this in a separate PR (the ticket you created).

}
}
}
13 changes: 10 additions & 3 deletions lib/commons/text/native-text-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,18 @@ const nativeTextMethods = {
*/
singleSpace: function singleSpace() {
return ' ';
}
},

/**
* Return the placeholder text
* @param {VirtualNode} element
* @return {String} placeholder text
*/
placeholderText: attrText.bind(null, 'placeholder')
};

function attrText(attr, { actualNode }) {
return actualNode.getAttribute(attr) || '';
function attrText(attr, vNode) {
return vNode.attr(attr) || '';
}

/**
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/label.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"id": "label",
"selector": "input, select, textarea",
"selector": "input, textarea",
"matches": "label-matches",
"tags": [
"cat.forms",
Expand All @@ -21,6 +21,7 @@
"implicit-label",
"explicit-label",
"non-empty-title",
"non-empty-placeholder",
"role-none",
"role-presentation"
],
Expand Down
27 changes: 27 additions & 0 deletions lib/rules/select-name.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"id": "select-name",
"selector": "select",
"tags": [
"cat.forms",
"wcag2a",
"wcag412",
"wcag131",
"section508",
"section508.22.n"
],
"metadata": {
"description": "Ensures select element has an accessible name",
"help": "Select element must have and accessible name"
},
"all": [],
"any": [
straker marked this conversation as resolved.
Show resolved Hide resolved
"aria-label",
"aria-labelledby",
"implicit-label",
"explicit-label",
"non-empty-title",
"role-none",
"role-presentation"
],
"none": ["help-same-as-label", "hidden-explicit-label"]
}
7 changes: 4 additions & 3 deletions lib/standards/html-elms.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,9 @@ const htmlElms = {
implicitAttrs: {
'aria-valuenow': ''
},
// 5.1 input type="text", input type="password", input type="search", input type="tel", input type="url" and textarea Element
// 5.1 input type="text", input type="password", input type="search", input type="tel", input type="url"
// 5.7 Other Form Elements
namingMethods: ['labelText']
namingMethods: ['labelText', 'placeholderText']
}
}
},
Expand Down Expand Up @@ -840,7 +840,8 @@ const htmlElms = {
'aria-valuenow': '',
'aria-multiline': 'true'
},
namingMethods: ['labelText']
// 5.1 textarea
namingMethods: ['labelText', 'placeholderText']
},
tfoot: {
allowedRoles: true
Expand Down
37 changes: 37 additions & 0 deletions test/checks/shared/non-empty-placeholder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
describe('non-empty-placeholder', function() {
'use strict';

var fixture = document.getElementById('fixture');
var checkSetup = axe.testUtils.checkSetup;
var checkEvaluate = axe.testUtils.getCheckEvaluate('non-empty-placeholder');

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

it('should return true if a placeholder is present', function() {
var params = checkSetup('<input id="target" placeholder="woohoo" />');

assert.isTrue(checkEvaluate.apply(null, params));
});

it('should return false if a placeholder is not present', function() {
var params = checkSetup('<input id="target" />');

assert.isFalse(checkEvaluate.apply(null, params));
});

it('should return false if a placeholder is present, but empty', function() {
var params = checkSetup('<input id="target" placeholder=" " />');

assert.isFalse(checkEvaluate.apply(null, params));
});

it('should collapse whitespace', function() {
var params = checkSetup(
'<input id="target" placeholder=" \t \n \r \t \t\r\n " />'
);

assert.isFalse(checkEvaluate.apply(null, params));
});
});
6 changes: 2 additions & 4 deletions test/commons/text/accessible-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -1026,8 +1026,7 @@ describe('text.accessibleTextVirtual', function() {
});
});

// not implemented yet, doesn't work accross ATs
it.skip('should find a placeholder attribute', function() {
it('should find a placeholder attribute', function() {
types.forEach(function(type) {
var t = type ? ' type="' + type + '"' : '';
fixture.innerHTML = '<input' + t + ' placeholder="Hello World">';
Expand Down Expand Up @@ -1118,8 +1117,7 @@ describe('text.accessibleTextVirtual', function() {
);
});

// not implemented yet, doesn't work accross ATs
it.skip('should find a placeholder attribute', function() {
it('should find a placeholder attribute', function() {
fixture.innerHTML = '<textarea placeholder="Hello World"></textarea>';
axe.testUtils.flatTreeSetup(fixture);

Expand Down
15 changes: 15 additions & 0 deletions test/commons/text/native-text-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,19 @@ describe('text.nativeTextMethods', function() {
assert.equal(singleSpace(), ' ');
});
});

describe('placeholderText', function() {
var placeholderText = nativeTextMethods.placeholderText;
it('returns the placeholder attribute of actualNode', function() {
fixtureSetup('<input placeholder="foo" />');
var input = axe.utils.querySelectorAll(axe._tree[0], 'input')[0];
assert.equal(placeholderText(input), 'foo');
});

it('returns `` when there is no placeholder', function() {
fixtureSetup('<input />');
var input = axe.utils.querySelectorAll(axe._tree[0], 'input')[0];
assert.equal(placeholderText(input), '');
});
});
});
22 changes: 3 additions & 19 deletions test/integration/rules/label/label.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,26 @@
<input type="submit" id="na4" />
<input type="reset" id="na5" />
<input type="text" id="fail1" />
<select id="fail2"></select>
<textarea id="fail3"></textarea>
<input type="text" aria-label="label" id="pass1" />
<select aria-label="label" id="pass2"></select>
<textarea aria-label="label" id="pass3"></textarea>
<input type="text" aria-labelledby="label" id="pass4" />
<select aria-labelledby="label" id="pass5"></select>
<textarea aria-labelledby="label" id="pass6"></textarea>
<div id="label">Label</div>
<label>Label <input type="text" id="pass7"/></label>
<label
>Label
<select id="pass8"></select
></label>
<label>Label <textarea id="pass9"></textarea></label>

<label><input type="text" id="fail4"/></label>
<label><select id="fail5"></select></label>
<label><textarea id="fail6"></textarea></label>
<label for="fail7"></label><input type="text" id="fail7" />
<label for="fail8"></label
><select id="fail8"></select>
<label for="fail9"></label><textarea id="fail9"></textarea>
<label for="fail10"
><select id="fail10">
<option>Thing</option>
</select></label
>

<label for="fail11" style="display: none;">Text</label
><input type="text" id="fail11" /> <label for="pass10">Label</label
><input type="text" id="pass10" /> <label for="pass11">Label</label
><select id="pass11"></select>
<label for="pass12">Label</label><textarea id="pass12"></textarea>
><input type="text" id="pass10" /> <label for="pass12">Label</label
><textarea id="pass12"></textarea>

<input id="pass13" title="Label" />
<select id="pass14" title="Label"></select>
<textarea id="pass15" title="Label"></textarea>

<label
Expand Down
11 changes: 0 additions & 11 deletions test/integration/rules/label/label.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,26 @@
"rule": "label",
"violations": [
["#fail1"],
["#fail2"],
["#fail3"],
["#fail4"],
["#fail5"],
["#fail6"],
["#fail7"],
["#fail8"],
["#fail9"],
["#fail10"],
["#fail11"],
["#fail22"],
["#fail24"],
["#fail25"],
["#fail26"],
["#fail27"]
],
"passes": [
["#pass1"],
["#pass2"],
["#pass3"],
["#pass4"],
["#pass5"],
["#pass6"],
["#pass7"],
["#pass8"],
["#pass9"],
["#pass10"],
["#pass11"],
["#pass12"],
["#pass13"],
["#pass14"],
["#pass15"],
["#pass16"],
["#pass17"],
Expand Down
33 changes: 33 additions & 0 deletions test/integration/rules/select-name/select-name.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<form action="#">
<select id="fail1"></select>
<select aria-label="label" id="pass1"></select>
<select aria-labelledby="label" id="pass2"></select>
<div id="label">Label</div>
<label>
Label
<select id="pass3"></select>
</label>

<label><select id="fail2"></select></label>
<label for="fail3">
<select id="fail3">
<option>Thing</option>
</select>
</label>
<label for="pass4">Label</label>
<select id="pass4"></select>

<select id="pass5" title="Label"></select>

<select id="pass6" role="presentation"></select>
<select id="pass7" role="none"></select>

<div>
<label>
<select id="fail4">
<option selected="selected">Chosen</option>
<option>Not Selected</option>
</select>
</label>
</div>
</form>
14 changes: 14 additions & 0 deletions test/integration/rules/select-name/select-name.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"description": "select-name test",
"rule": "select-name",
"violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"]],
"passes": [
["#pass1"],
["#pass2"],
["#pass3"],
["#pass4"],
["#pass5"],
["#pass6"],
["#pass7"]
]
}
3 changes: 2 additions & 1 deletion test/integration/virtual-rules/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
</head>
<body>
<div id="mocha"></div>
<script src="autocompelte-valid.js"></script>
<script src="autocomplete-valid.js"></script>
<script src="image-alt.js"></script>
<script src="frame-title.js"></script>
<script src="input-image-alt.js"></script>
Expand All @@ -35,6 +35,7 @@
<script src="svg-img-alt.js"></script>
<script src="role-img-alt.js"></script>
<script src="link-name.js"></script>
<script src="select-name.js"></script>
<script src="/test/integration/adapter.js"></script>
</body>
</html>
28 changes: 27 additions & 1 deletion test/integration/virtual-rules/label.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('label', function() {

it('should incomplete for aria-labelledby', function() {
var results = axe.runVirtualRule('label', {
nodeName: 'select',
nodeName: 'input',
attributes: {
'aria-labelledby': 'foobar'
}
Expand Down Expand Up @@ -102,6 +102,32 @@ describe('label', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should pass for role=presentation', function() {
var results = axe.runVirtualRule('label', {
nodeName: 'input',
attributes: {
role: 'presentation'
}
});

assert.lengthOf(results.passes, 1);
assert.lengthOf(results.violations, 0);
assert.lengthOf(results.incomplete, 0);
});

it('should pass for role=none', function() {
var results = axe.runVirtualRule('label', {
nodeName: 'input',
attributes: {
role: 'none'
}
});

assert.lengthOf(results.passes, 1);
assert.lengthOf(results.violations, 0);
assert.lengthOf(results.incomplete, 0);
});

it('should incomplete for both missing aria-label and implicit label', function() {
var results = axe.runVirtualRule('label', {
nodeName: 'input',
Expand Down
Loading