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.

Sign up for GitHub

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

Jump to bottom

fix(multiple-label): no longer raises issue when aria-labelledby overrides how AT views multiple labels #1538

Merged
merged 1 commit into from
May 17, 2019
Merged

fix(multiple-label): no longer raises issue when aria-labelledby overrides how AT views multiple labels #1538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 19 additions & 11 deletions lib/checks/label/multiple-label.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,8 @@ let labels = Array.from(document.querySelectorAll(`label[for="${id}"]`));
let parent = node.parentNode;

if (labels.length) {
// filter out hidden labels because they're fine
// except: fail first label if hidden because of VO
labels = labels.filter(function(label, index) {
if (
(index === 0 && !axe.commons.dom.isVisible(label, true)) ||
axe.commons.dom.isVisible(label, true)
) {
return label;
}
});
// filter out CSS hidden labels because they're fine
labels = labels.filter(label => axe.commons.dom.isVisible(label));
}

while (parent) {
Expand All @@ -26,4 +18,20 @@ while (parent) {
}

this.relatedNodes(labels);
return labels.length > 1;

// more than 1 CSS visible label
if (labels.length > 1) {
const ATVisibleLabels = labels.filter(label =>
axe.commons.dom.isVisible(label, true)
);
// more than 1 AT visible label will fail IOS/Safari/VO even with aria-labelledby
if (ATVisibleLabels.length > 1) {
return true;
}
// make sure the ONE AT visible label is in the list of idRefs of aria-labelledby
const labelledby = axe.commons.dom.idrefs(node, 'aria-labelledby');
return !labelledby.includes(ATVisibleLabels[0]);
}

// only 1 CSS visible label
return false;
90 changes: 77 additions & 13 deletions test/checks/label/multiple-label.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,7 @@ describe('multiple-label', function() {
assert.deepEqual(checkContext._relatedNodes, [l1]);
});

it('should return true if there are multiple explicit labels but the first one is hidden', function() {
fixture.innerHTML =
'<label style="display:none" for="test-input2" id="l1">label one</label>' +
'<label for="test-input2" id="l2">label two</label>' +
'<input id="test-input2" type="text">';
var target = fixture.querySelector('#test-input2');
var l1 = fixture.querySelector('#l1');
var l2 = fixture.querySelector('#l2');
assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
assert.deepEqual(checkContext._relatedNodes, [l1, l2]);
});

it('should return true if there are multiple explicit labels but the first one is hidden', function() {
it('should return true if there are multiple explicit labels but some are hidden', function() {
fixture.innerHTML =
'<label for="me" id="l1">visible</label>' +
'<label for="me" style="display:none;" id="l2">hidden</label>' +
Expand Down Expand Up @@ -112,4 +100,80 @@ describe('multiple-label', function() {
checks['multiple-label'].evaluate.call(checkContext, target)
);
});

it('should return true given multiple labels and no aria-labelledby', function() {
fixture.innerHTML = '<input type="checkbox" id="A">';
fixture.innerHTML += '<label for="A">Please</label>';
fixture.innerHTML += '<label for="A">Excuse</label>';
fixture.innerHTML += '<label for="A">My</label>';
fixture.innerHTML += '<label for="A">Dear</label>';
fixture.innerHTML += '<label for="A">Aunt</label>';
fixture.innerHTML += '<label for="A">Sally</label>';
var target = fixture.querySelector('#A');
assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
});

it('should return true given multiple labels, one label AT visible, and no aria-labelledby', function() {
fixture.innerHTML = '<input type="checkbox" id="B">';
fixture.innerHTML += '<label for="B">Please</label>';
fixture.innerHTML += '<label for="B" aria-hidden="true">Excuse</label>';
fixture.innerHTML += '<label for="B" aria-hidden="true">My</label>';
fixture.innerHTML += '<label for="B" aria-hidden="true">Dear</label>';
fixture.innerHTML += '<label for="B" aria-hidden="true">Aunt</label>';
fixture.innerHTML += '<label for="B" aria-hidden="true">Sally</label>';
var target = fixture.querySelector('#B');
assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
});

it('should return false given multiple labels, one label AT visible, and aria-labelledby for AT visible', function() {
fixture.innerHTML = '<input type="checkbox" id="D" aria-labelledby="E"/>';
fixture.innerHTML += '<label for="D" aria-hidden="true">Please</label>';
fixture.innerHTML += '<label for="D" id="E">Excuse</label>';
var target = fixture.querySelector('#D');
assert.isFalse(
checks['multiple-label'].evaluate.call(checkContext, target)
);
});

it('should return false given multiple labels, one label AT visible, and aria-labelledby for all', function() {
fixture.innerHTML = '<input type="checkbox" id="F" aria-labelledby="G H"/>';
fixture.innerHTML +=
'<label for="F" id="G" aria-hidden="true">Please</label>';
fixture.innerHTML += '<label for="F" id="H">Excuse</label>';
var target = fixture.querySelector('#F');
assert.isFalse(
checks['multiple-label'].evaluate.call(checkContext, target)
);
});

it('should return false given multiple labels, one label visible, and no aria-labelledby', function() {
fixture.innerHTML = '<input type="checkbox" id="I"/>';
fixture.innerHTML += '<label for="I" style="display:none">Please</label>';
fixture.innerHTML += '<label for="I" >Excuse</label>';
var target = fixture.querySelector('#I');
assert.isFalse(
checks['multiple-label'].evaluate.call(checkContext, target)
);
});

it('should return true given multiple labels, all visible, aria-labelledby for all', function() {
fixture.innerHTML =
'<input type="checkbox" id="J" aria-labelledby="K L M N O P">';
fixture.innerHTML += '<label for="J" id="K">Please</label>';
fixture.innerHTML += '<label for="J" id="L">Excuse</label>';
fixture.innerHTML += '<label for="J" id="M">My</label>';
fixture.innerHTML += '<label for="J" id="N">Dear</label>';
fixture.innerHTML += '<label for="J" id="O">Aunt</label>';
fixture.innerHTML += '<label for="J" id="P">Sally</label>';
var target = fixture.querySelector('#J');
assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
});

it('should return true given multiple labels, one AT visible, no aria-labelledby', function() {
fixture.innerHTML = '<input type="checkbox" id="Q"/>';
fixture.innerHTML += '<label for="Q" aria-hidden="true"></label>';
fixture.innerHTML += '<label for="Q" >Excuse</label>';
var target = fixture.querySelector('#Q');
assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
});
});