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

Commit 581e575

Browse files
committed
Addressing alice's feedback on PR.
1 parent 00d8606 commit 581e575

4 files changed

Lines changed: 52 additions & 67 deletions

File tree

src/audits/AriaRoleNotScoped.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ axs.AuditRules.addRule({
3737
var elementRole = axs.utils.getRoles(element);
3838
if (!elementRole || !elementRole.applied)
3939
return false;
40-
elementRole = elementRole.applied;
41-
var ariaRole = elementRole.details;
40+
var appliedRole = elementRole.applied;
41+
var ariaRole = appliedRole.details;
4242
var requiredScope = ariaRole['scope'];
4343
if (!requiredScope || requiredScope.length === 0) {
4444
return false;
@@ -47,8 +47,8 @@ axs.AuditRules.addRule({
4747
while ((parent = parent.parentNode)) {
4848
var parentRole = axs.utils.getRoles(parent, true);
4949
if (parentRole && parentRole.applied) {
50-
parentRole = parentRole.applied;
51-
if (requiredScope.indexOf(parentRole.name) >= 0) // if this ancestor role is one of the required roles
50+
var appliedParentRole = parentRole.applied;
51+
if (requiredScope.indexOf(appliedParentRole.name) >= 0) // if this ancestor role is one of the required roles
5252
return false;
5353
}
5454
}

src/audits/RequiredOwnedAriaRoleMissing.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ goog.require('axs.utils');
5454
var ownedElement = ownedElements[i];
5555
var ownedElementRole = axs.utils.getRoles(ownedElement, true);
5656
if (ownedElementRole && ownedElementRole.applied) {
57-
ownedElementRole = ownedElementRole.applied;
57+
var appliedRole = ownedElementRole.applied;
5858
for (var j = required.length - 1; j >= 0; j--) {
59-
if (ownedElementRole.name === required[j]) { // if this explicitly owned element has a required role
59+
if (appliedRole.name === required[j]) { // if this explicitly owned element has a required role
6060
return false;
6161
}
6262
}
@@ -76,10 +76,10 @@ goog.require('axs.utils');
7676
var elementRole = axs.utils.getRoles(element);
7777
if (!elementRole || !elementRole.applied)
7878
return [];
79-
elementRole = elementRole.applied;
80-
if (!elementRole.valid)
79+
var appliedRole = elementRole.applied;
80+
if (!appliedRole.valid)
8181
return [];
82-
return elementRole.details['mustcontain'] || [];
82+
return appliedRole.details['mustcontain'] || [];
8383
}
8484
axs.AuditRules.addRule(spec);
8585
})();

src/js/AccessibilityUtils.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,8 +1055,8 @@ axs.utils.getRoles = function(element, implicit) {
10551055
for (var i = 0; i < roleNames.length; i++) {
10561056
var role = roleNames[i];
10571057
var ariaRole = axs.constants.ARIA_ROLES[role];
1058-
var roleObject = {'name': role};
1059-
if (ariaRole && !ariaRole['abstract']) {
1058+
var roleObject = { 'name': role };
1059+
if (ariaRole && !ariaRole.abstract) {
10601060
roleObject.details = ariaRole;
10611061
if (!result.applied) {
10621062
result.applied = roleObject;

test/js/utils-test.js

Lines changed: 41 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ test("returns the aria owners for a given element", function() {
173173
var actual = axs.utils.getIdReferrers("aria-owns", owned);
174174
equal(expected.length, ownerCount); // sanity check the test itself
175175
equal(actual.length, ownerCount);
176-
var allFound = Array.prototype.every.call(expected, function(element){
176+
var allFound = Array.prototype.every.call(expected, function(element) {
177177
return (Array.prototype.indexOf.call(actual, element) >= 0);
178178
});
179179
equal(allFound, true);
@@ -193,7 +193,7 @@ test("returns the elements this element labels", function() {
193193
var actual = axs.utils.getIdReferrers("aria-labelledby", label);
194194
equal(expected.length, labelledCount); // sanity check the test itself
195195
equal(actual.length, labelledCount);
196-
var allFound = Array.prototype.every.call(expected, function(element){
196+
var allFound = Array.prototype.every.call(expected, function(element) {
197197
return (Array.prototype.indexOf.call(actual, element) >= 0);
198198
});
199199
equal(allFound, true);
@@ -311,65 +311,50 @@ test("getRoles on element with abstract role.", function() {
311311
}
312312
}
313313
});
314-
315-
test("getRoles on element with multiple valid roles.", function() {
316-
var roles = ['checkbox', 'button', 'radio'];
317-
var expectedRoles = roles.map(function(role){
314+
(function() {
315+
/**
316+
* Creates a 'role detail' object which can be used for comparison in the assertions below.
317+
* @param {!string} role A potential ARIA role.
318+
* @return The 'role detail' object.
319+
*/
320+
function createExpectedRoleObject(role) {
318321
var valid = (axs.constants.ARIA_ROLES.hasOwnProperty(role) && !axs.constants.ARIA_ROLES[role].abstract);
319322
var result = { name: role, valid: valid };
320323
if (valid) {
321324
result.details = axs.constants.ARIA_ROLES[role];
322325
}
323326
return result;
324-
});
325-
var expected = {
326-
valid: true,
327-
applied: expectedRoles[0],
328-
roles: expectedRoles
329-
};
330-
var element = document.createElement('div');
331-
element.setAttribute('role', roles.join(' '));
332-
var actual = axs.utils.getRoles(element);
333-
deepEqual(actual, expected);
334-
});
327+
}
335328

336-
test("getRoles on element with invalid and valid roles.", function() {
337-
var roles = ['foo', 'button', 'bar'];
338-
var expectedRoles = roles.map(function(role){
339-
var valid = (axs.constants.ARIA_ROLES.hasOwnProperty(role) && !axs.constants.ARIA_ROLES[role].abstract);
340-
var result = { name: role, valid: valid };
341-
if (valid) {
342-
result.details = axs.constants.ARIA_ROLES[role];
343-
}
344-
return result;
345-
});
346-
var expected = {
347-
valid: true,
348-
applied: expectedRoles[1],
349-
roles: expectedRoles
350-
};
351-
var element = document.createElement('div');
352-
element.setAttribute('role', roles.join(' '));
353-
var actual = axs.utils.getRoles(element);
354-
deepEqual(actual, expected);
355-
});
329+
/**
330+
* Helper for multiple role tests.
331+
* @param {!Array<string>} roles Strings to set in the 'role' attribute of the element under test.
332+
* @param {!number} validIdx The index of the expected applied (valid) ARIA role in the array above
333+
* or a negative number if there are no valid roles.
334+
* @return {Function} A test function for qunit.
335+
*/
336+
function multipleRoleTestHelper(roles, validIdx) {
337+
return function() {
338+
var expectedRoles = roles.map(createExpectedRoleObject);
339+
var expected = {
340+
roles: expectedRoles
341+
};
342+
if (validIdx >= 0) {
343+
expected.valid = true;
344+
expected.applied = expectedRoles[validIdx];
345+
}
346+
else {
347+
expected.valid = false;
348+
}
349+
var element = document.createElement('div');
350+
element.setAttribute('role', roles.join(' '));
351+
var actual = axs.utils.getRoles(element);
352+
deepEqual(actual, expected);
353+
};
354+
}
356355

357-
test("getRoles on element with multiple invalid roles.", function() {
358-
var roles = ['foo', 'fubar', 'bar'];
359-
var expectedRoles = roles.map(function(role){
360-
var valid = (axs.constants.ARIA_ROLES.hasOwnProperty(role) && !axs.constants.ARIA_ROLES[role].abstract);
361-
var result = { name: role, valid: valid };
362-
if (valid) {
363-
result.details = axs.constants.ARIA_ROLES[role];
364-
}
365-
return result;
366-
});
367-
var expected = {
368-
valid: false,
369-
roles: expectedRoles
370-
};
371-
var element = document.createElement('div');
372-
element.setAttribute('role', roles.join(' '));
373-
var actual = axs.utils.getRoles(element);
374-
deepEqual(actual, expected);
375-
});
356+
test("getRoles on element with multiple valid roles.", multipleRoleTestHelper(['checkbox', 'button', 'radio'], 0));
357+
test("getRoles on element with invalid and valid roles.", multipleRoleTestHelper(['foo', 'button', 'bar'], 1));
358+
test("getRoles on element with multiple invalid roles.", multipleRoleTestHelper(['foo', 'fubar', 'bar'], -1));
359+
360+
}());

0 commit comments

Comments
 (0)