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: Add aria-dialog-name #2609

Merged
merged 6 commits into from
Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -15,6 +15,7 @@
| [area-alt](https://dequeuniversity.com/rules/axe/4.0/area-alt?application=RuleDescription) | Ensures <area> elements of image maps have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, wcag244, wcag412, section508, section508.22.a | failure, needs review |
| [aria-allowed-attr](https://dequeuniversity.com/rules/axe/4.0/aria-allowed-attr?application=RuleDescription) | Ensures ARIA attributes are allowed for an element's role | Critical | cat.aria, wcag2a, wcag412 | failure |
| [aria-command-name](https://dequeuniversity.com/rules/axe/4.0/aria-command-name?application=RuleDescription) | Ensures every ARIA button, link and menuitem has an accessible name | Serious | cat.aria, wcag2a, wcag412 | failure, needs review |
| [aria-dialog-name](https://dequeuniversity.com/rules/axe/4.0/aria-dialog-name?application=RuleDescription) | Ensures every ARIA dialog and alertdialog node has an accessible name | Serious | cat.aria, wcag2a, wcag412 | failure, needs review |
| [aria-hidden-body](https://dequeuniversity.com/rules/axe/4.0/aria-hidden-body?application=RuleDescription) | Ensures aria-hidden='true' is not present on the document body. | Critical | cat.aria, wcag2a, wcag412 | failure |
| [aria-hidden-focus](https://dequeuniversity.com/rules/axe/4.0/aria-hidden-focus?application=RuleDescription) | Ensures aria-hidden elements do not contain focusable elements | Serious | cat.name-role-value, wcag2a, wcag412, wcag131 | failure, needs review |
| [aria-input-field-name](https://dequeuniversity.com/rules/axe/4.0/aria-input-field-name?application=RuleDescription) | Ensures every ARIA input field has an accessible name | Moderate, Serious | cat.aria, wcag2a, wcag412 | failure, needs review |
Expand Down
13 changes: 13 additions & 0 deletions lib/rules/aria-dialog-name.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"id": "aria-dialog-name",
"selector": "[role=\"dialog\"], [role=\"alertdialog\"]",
"matches": "no-naming-method-matches",
"tags": ["cat.aria", "wcag2a", "wcag412"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking good, glad all the tests are passing now. Just one more thing before we can merge. The rule should be a best practice rule and not linked to wcag success criterion

Suggested change
"tags": ["cat.aria", "wcag2a", "wcag412"],
"tags": ["cat.aria", "best-practice"],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I've been meaning to ask how this is decided. @WilcoFiers added tags in the umbrella issue but some of the newly implemented rules don't match those tags (or just partially):

  • aria-command-name has "cat.aria", "wcag2a", "wcag412" instead of just "wcag2a"
  • aria-progressbar-name has "cat.aria", "wcag2a", "wcag111" instead of just "sc111"
  • aria-tooltip-name has "cat.aria", "wcag2a", "wcag412" instead of "best-practice"

Specifically here: Why is not naming a dialog not a 4.12 failure and "just" best practice?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is based on our interpretation of WCAG 2. Every rule needs a category tag. That's the "cat.*" stuff. Additionally, a rule is either a WCAG success criterion, which are those "wcag123" type tags or a "best-practice". If it's a wcag tag, it'll also include a level, which is just the level of that success criterion (SC) in WCAG. So SC 1.1.1 has the tag "cat.aria", "wcag2a", "wcag111". This best practice would get "cat.aria", "best-practice".

As for why I made tooltip a WCAG rule and think dialog should be a best practice, that is based on some discussions I've had internally at Deque. A tooltip doesn't make much sense when it is empty, but more importantly a tooltip is a piece of a user interface that users will understand to be a single "thing". Success criterion 4.1.2 applies only to "user interface components". It's a little more difficult to argue that a dialog is a user interface component, since it is just a region of the screen with other content in it. User interface components are the smallest things users will understand as its own control. For example a select is a user interface component, but a list of radio buttons is not, each radio button would be though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see how this makes sense.

Slightly going off-topic here: why does a tree not fall under "user interface components"? It was was scratched due to aria-structure-name being scratched but I would consider this more of a widget and not just some region. Happy to continue this in #2421 (comment)

"metadata": {
"description": "Ensures every ARIA dialog and alertdialog node has an accessible name",
"help": "ARIA dialog and alertdialog nodes must have an accessible name"
},
"all": [],
"any": ["aria-label", "aria-labelledby", "non-empty-title"],
"none": []
}
24 changes: 24 additions & 0 deletions test/integration/rules/aria-dialog-name/aria-dialog-name.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!-- PASS -->
<div id="pass1" role="dialog" aria-label="Cookies"></div>
<div id="pass2" role="dialog" title="Cookies"></div>
<div id="pass3" role="dialog" aria-labelledby="dialog-div-heading">
<h2 id="dialog-div-heading">Cookies</h2>
</div>
<dialog id="pass4" open role="alertdialog" aria-label="Cookies"></dialog>
<dialog id="pass5" open role="alertdialog" title="Cookies"></dialog>
<dialog id="pass6" open role="alertdialog" aria-labelledby="alertdialog-heading">
<h2 id="alertdialog-heading">Cookies</h2>
</dialog>
<div id="pass7" role="alertdialog" aria-label="Cookies"></div>
<div id="pass8" role="alertdialog" title="Cookies"></div>
<div id="pass9" role="alertdialog" aria-labelledby="alertdialog-div-heading">
<h2 id="alertdialog-div-heading">Cookies</h2>
</div>

<!-- FAIL -->
<div id="fail1" role="dialog"></div>
<div id="fail2" role="dialog" aria-labelledby="non-existent"></div>
<dialog id="fail3" open role="alertdialog"></dialog>
<dialog id="fail4" open role="alertdialog" aria-labelledby="non-existent"></dialog>
<div id="fail5" role="alertdialog"></div>
<div id="fail6" role="alertdialog" aria-labelledby="non-existent"></div>
23 changes: 23 additions & 0 deletions test/integration/rules/aria-dialog-name/aria-dialog-name.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"description": "aria-dialog-name test",
"rule": "aria-dialog-name",
"passes": [
["#pass1"],
["#pass2"],
["#pass3"],
["#pass4"],
["#pass5"],
["#pass6"],
["#pass7"],
["#pass8"],
["#pass9"]
],
"violations": [
["#fail1"],
["#fail2"],
["#fail3"],
["#fail4"],
["#fail5"],
["#fail6"]
]
}
89 changes: 89 additions & 0 deletions test/integration/virtual-rules/aria-dialog-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
describe('aria-dialog-name', function() {
it('should pass for aria-label', function() {
var node = new axe.SerialVirtualNode({
nodeName: 'div',
attributes: {
role: 'dialog',
'aria-label': 'foobar'
}
});
node.parent = null;

var results = axe.runVirtualRule('aria-dialog-name', node);

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

it('should incomplete for aria-labelledby', function() {
var node = new axe.SerialVirtualNode({
nodeName: 'div',
attributes: {
role: 'dialog',
'aria-labelledby': 'foobar'
}
});
node.parent = null;

var results = axe.runVirtualRule('aria-dialog-name', node);

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

it('should pass for title', function() {
var node = new axe.SerialVirtualNode({
nodeName: 'div',
attributes: {
role: 'alertdialog',
title: 'foobar'
}
});
// children are required since titleText comes after subtree text
// in accessible name calculation
node.children = [];
node.parent = null;

var results = axe.runVirtualRule('aria-dialog-name', node);

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

it('should fail when aria-label contains only whitespace', function() {
var node = new axe.SerialVirtualNode({
nodeName: 'div',
attributes: {
role: 'alertdialog',
'aria-label': ' \t \n '
}
});
node.children = [];

var results = axe.runVirtualRule('aria-dialog-name', node);

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

it('should fail when title is empty', function() {
var node = new axe.SerialVirtualNode({
nodeName: 'div',
attributes: {
role: 'dialog',
title: ''
}
});
node.children = [];

var results = axe.runVirtualRule('aria-dialog-name', node);

assert.lengthOf(results.passes, 0);
assert.lengthOf(results.violations, 1);
assert.lengthOf(results.incomplete, 0);
});
});
1 change: 1 addition & 0 deletions test/integration/virtual-rules/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<div id="mocha"></div>
<script src="area-alt.js"></script>
<script src="aria-command-name.js"></script>
<script src="aria-dialog-name.js"></script>
<script src="aria-input-field-name.js"></script>
<script src="aria-progressbar-name.js"></script>
<script src="aria-toggle-field-name.js"></script>
Expand Down