Skip to content

Commit

Permalink
feat(label,select-name): allow placeholder to pass label rule, add se…
Browse files Browse the repository at this point in the history
…lect-name rule (#2448)

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

* add role none/presentation check

* fix
  • Loading branch information
straker authored Aug 20, 2020
1 parent 750a95b commit 1315f8e
Show file tree
Hide file tree
Showing 16 changed files with 355 additions and 43 deletions.
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"
}
}
}
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": [
"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

0 comments on commit 1315f8e

Please sign in to comment.