Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Fix ARIA tablist and ARIA tab scope #207

Merged
merged 4 commits into from
Jul 31, 2015
Merged
Show file tree
Hide file tree
Changes from 3 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 Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* Add a special case to handle color `"transparent"` to fix (#180).
* Fix `matchSelector` not working properly in browser environments without vendor prefixes (#189).
* Fix false positives on elements with no role for Unsupported ARIA Attribute rule (#178 and #199).
* Fix ARIA `tablist` and ARIA `tab` scope (#204)

## 2.8.0 - 2015-07-24

Expand Down
6 changes: 3 additions & 3 deletions src/js/Constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,14 +407,14 @@ axs.constants.ARIA_ROLES = {
"tab": {
"namefrom": [ "contents", "author" ],
"parent": [ "sectionhead", "widget" ],
"properties": [ "aria-selected" ]
"properties": [ "aria-selected" ],
"scope": [ "tablist" ]
},
"tablist": {
"mustcontain": [ "tab" ],
"namefrom": [ "author" ],
"parent": [ "composite", "directory" ],
"properties": [ "aria-level" ],
"scope": [ "tablist" ]
"properties": [ "aria-level" ]
},
"tabpanel": {
"namefrom": [ "author" ],
Expand Down
30 changes: 30 additions & 0 deletions test/audits/aria-role-not-scoped-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,33 @@ test('Scope role missing', function() {
equal(actual.result, axs.constants.AuditResult.FAIL);
deepEqual(actual.elements, expected);
});

test('Scope role present - tablist', function() {
var rule = axs.AuditRules.getRule('ariaRoleNotScoped');
var fixture = document.getElementById('qunit-fixture');
var container = fixture.appendChild(document.createElement('ul'));
container.setAttribute('role', 'tablist');
for (var i = 0; i < 4; i++) {
var item = container.appendChild(document.createElement('li'));
item.setAttribute('role', 'tab');
}

var actual = rule.run({ scope: fixture });
equal(actual.result, axs.constants.AuditResult.PASS);
deepEqual(actual.elements, []);
});

test('Scope role missing - tab', function() {
var rule = axs.AuditRules.getRule('ariaRoleNotScoped');
var fixture = document.getElementById('qunit-fixture');
var expected = [];
for (var i = 0; i < 4; i++) {
var item = fixture.appendChild(document.createElement('span'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this test use spans where the previous uses a list? Would this test pass if a list was used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was just to save one line - this test doesn't need a container, the spans are added directly to the fixture. It didn't feel right to add a bunch of li elements without a ul or ol.

I can change it to list if you prefer - the test will still work.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you can be bothered, I think it would help avoid future confusion, but 👍 either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok no problem - will make that change then merge it.

item.setAttribute('role', 'tab');
expected.push(item);
}

var actual = rule.run({ scope: fixture });
equal(actual.result, axs.constants.AuditResult.FAIL);
deepEqual(actual.elements, expected);
});