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

Color fixes: #607, #556, #596 #610

Merged
merged 7 commits into from
Dec 12, 2017
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions lib/checks/color/color-contrast.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"bgOverlap": "Element's background color could not be determined because it is overlapped by another element",
"fgAlpha" : "Element's foreground color could not be determined because of alpha transparency",
"elmPartiallyObscured": "Element's background color could not be determined because it's partially obscured by another element",
"elmPartiallyObscuring": "Element's background color could not be determined because it partially overlaps other elements",
"outsideViewport": "Element's background color could not be determined because it's outside the viewport",
"equalRatio": "Element has a 1:1 contrast ratio with the background",
"default": "Unable to determine contrast ratio"
}
Expand Down
102 changes: 94 additions & 8 deletions lib/commons/color/get-background-color.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,15 @@ function sortPageBackground(elmStack) {
}
return bgNodes;
}

/**
* Get all elements rendered underneath the current element, In the order they are displayed (front to back)
* @method getBackgroundStack
* Get coordinates for an element's client rects or bounding client rect
* @method getCoords
* @memberof axe.commons.color
* @instance
* @param {Element} elm
* @return {Array}
* @param {DOMRect} rect
* @return {Object}
*/
color.getBackgroundStack = function(elm) {
let rect = elm.getBoundingClientRect();
color.getCoords = function(rect) {
let x, y;
if (rect.left > window.innerWidth) {
return;
Expand All @@ -203,7 +201,95 @@ color.getBackgroundStack = function(elm) {
Math.ceil(rect.top + (rect.height / 2)),
window.innerHeight - 1);

let elmStack = Array.from(document.elementsFromPoint(x, y));
return {x, y};
};
/**
* Get elements from point for block and inline elements, excluding line breaks
* @method getRectStack
* @memberof axe.commons.color
* @instance
* @param {Element} elm
* @return {Array}
*/
color.getRectStack = function(elm) {
let boundingCoords = color.getCoords(elm.getBoundingClientRect());
if (boundingCoords) {
// allows inline elements spanning multiple lines to be evaluated
let rects = Array.from(elm.getClientRects());
let boundingStack = Array.from(document.elementsFromPoint(boundingCoords.x, boundingCoords.y));
if (rects && rects.length > 1) {
let filteredArr = rects.filter((rect) => {
// exclude manual line breaks in Chrome/Safari
return rect.width && rect.width > 0;
})
.map((rect) => {
let coords = color.getCoords(rect);
if (coords) {
return Array.from(document.elementsFromPoint(coords.x, coords.y));
}
});
// add bounding client rect stack for comparison later
filteredArr.splice(0, 0, boundingStack);
return filteredArr;
} else {
return [boundingStack];
}
}
return null;
};
/**
* Get filtered stack of block and inline elements, excluding line breaks
* @method filteredRectStack
* @memberof axe.commons.color
* @instance
* @param {Element} elm
* @return {Array}
*/
color.filteredRectStack = function(elm) {
let rectStack = color.getRectStack(elm);
if (rectStack && rectStack.length === 1) {
// default case, elm.getBoundingClientRect()
return rectStack[0];
}
else if (rectStack && rectStack.length > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor formatting issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

let boundingStack = rectStack.shift();
let isSame;
// iterating over arrays of DOMRects
rectStack.forEach((rectList, index) => {
if (index === 0) { return; }
// if the stacks are the same, use the first one. otherwise, return null.
let rectA = rectStack[index - 1],
rectB = rectStack[index];

// if elements in clientRects are the same
// or the boundingClientRect contains the differing element, pass it
isSame = rectA.every(function(element, elementIndex) {
return element === rectB[elementIndex];
}) || boundingStack.includes(elm);
});
if (!isSame) {
axe.commons.color.incompleteData.set('bgColor', 'elmPartiallyObscuring');
return null;
}
// pass the first stack if it wasn't partially covered
return rectStack[0];
} else {
// rect outside of viewport
axe.commons.color.incompleteData.set('bgColor', 'outsideViewport');
return null;
}
};
/**
* Get all elements rendered underneath the current element, In the order they are displayed (front to back)
* @method getBackgroundStack
* @memberof axe.commons.color
* @instance
* @param {Element} elm
* @return {Array}
*/
color.getBackgroundStack = function(elm) {
let elmStack = color.filteredRectStack(elm);
if (elmStack === null) { return null; }
elmStack = includeMissingElements(elmStack, elm);
elmStack = dom.reduceToElementsBelowFloating(elmStack, elm);
elmStack = sortPageBackground(elmStack);
Expand Down
6 changes: 5 additions & 1 deletion lib/rules/color-contrast-matches.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ if (nodeName === 'FIELDSET' && node.disabled || axe.commons.dom.findUp(node, 'fi
var nodeParentLabel = axe.commons.dom.findUp(node, 'label');
if (nodeName === 'LABEL' || nodeParentLabel) {
var relevantNode = node;
var relevantVirtualNode = virtualNode;

if (nodeParentLabel) {
relevantNode = nodeParentLabel;
// we need an input candidate from a parent to account for label children
relevantVirtualNode = axe.utils.getNodeFromTree(axe._tree[0], nodeParentLabel);
}
// explicit label of disabled input
let doc = axe.commons.dom.getRootNode(relevantNode);
Expand All @@ -45,7 +49,7 @@ if (nodeName === 'LABEL' || nodeParentLabel) {
return false;
}

var candidate = axe.utils.querySelectorAll(virtualNode, 'input:not([type="hidden"]):not([type="image"])' +
var candidate = axe.utils.querySelectorAll(relevantVirtualNode, 'input:not([type="hidden"]):not([type="image"])' +
':not([type="button"]):not([type="submit"]):not([type="reset"]), select, textarea');
if (candidate.length && candidate[0].actualNode.disabled) {
return false;
Expand Down
23 changes: 23 additions & 0 deletions test/checks/color/color-contrast.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,29 @@ describe('color-contrast', function () {
assert.deepEqual(checkContext._relatedNodes, []);
});

it('should return true for inline elements with sufficient contrast spanning multiple lines', function () {
fixture.innerHTML = '<p>Text oh heyyyy <a href="#" id="target">and here\'s <br>a link</a></p>';
var target = fixture.querySelector('#target');
assert.isTrue(checks['color-contrast'].evaluate.call(checkContext, target));
assert.deepEqual(checkContext._relatedNodes, []);
});

it('should return undefined for inline elements spanning multiple lines that are overlapped', function () {
fixture.innerHTML = '<div style="position:relative;"><div style="background-color:rgba(0,0,0,1);position:absolute;width:300px;height:200px;"></div>' +
'<p>Text oh heyyyy <a href="#" id="target">and here\'s <br>a link</a></p></div>';
var target = fixture.querySelector('#target');
assert.isUndefined(checks['color-contrast'].evaluate.call(checkContext, target));
assert.deepEqual(checkContext._relatedNodes, []);
});

it('should return true for inline elements with sufficient contrast', function () {
fixture.innerHTML = '<p>Text oh heyyyy <b id="target">and here\'s bold text</b></p>';
var target = fixture.querySelector('#target');
var result = checks['color-contrast'].evaluate.call(checkContext, target);
assert.isTrue(result);
assert.deepEqual(checkContext._relatedNodes, []);
});

it('should return false when there is not sufficient contrast', function () {
fixture.innerHTML = '<div style="color: yellow; background-color: white;" id="target">' +
'My text</div>';
Expand Down
33 changes: 33 additions & 0 deletions test/commons/color/get-background-color.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,39 @@ describe('color.getBackgroundColor', function () {
assert.deepEqual(bgNodes, [target]);
});

it('should return a bgcolor for a multiline inline element fully covering the background', function () {
fixture.innerHTML = '<div style="position:relative;">' +
'<div style="background-color:rgba(0,0,0,1);position:absolute;width:300px;height:200px;"></div>' +
'<p style="position: relative;z-index:1;">Text oh heyyyy <a href="#" id="target">and here\'s <br>a link</a></p>' +
'</div>';
var actual = axe.commons.color.getBackgroundColor(document.getElementById('target'), []);
assert.isNotNull(actual);
assert.equal(Math.round(actual.blue), 0);
assert.equal(Math.round(actual.red), 0);
assert.equal(Math.round(actual.green), 0);
});

it('should return null for a multiline block element not fully covering the background', function () {
Copy link
Contributor Author

@marcysutton marcysutton Dec 7, 2017

Choose a reason for hiding this comment

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

This test fails in Firefox AND Phantom, which is super irritating. Essentially, this test returns an actual color even though half of the element isn't covering its parent–but in Chrome it returns null. Rather than spending more time trying to fix this case, I'm going to capture the problem in #621 and remove the test from this PR.

fixture.innerHTML = '<div style="position:relative;">' +
'<div id="background" style="background-color:rgba(0,0,0,1); position:absolute; width:300px; height:20px;"></div>' +
'<p style="position:relative; z-index:1; width:300px;" id="target">Text content Text content Text content '+
'Text content Text content Text content</p>' +
'</div>';
var actual = axe.commons.color.getBackgroundColor(document.getElementById('target'), []);
assert.isNull(actual);
assert.equal(axe.commons.color.incompleteData.get('bgColor'), 'elmPartiallyObscured');
});

it('should return null if a multiline inline element does not fully cover background', function () {
fixture.innerHTML = '<div style="position:relative;">' +
'<div style="background-color:rgba(0,0,0,1);position:absolute;width:300px;height:20px;"></div>' +
'<p style="position: relative;z-index:1;">Text oh heyyyy <a href="#" id="target">and here\'s <br>a link</a></p>' +
'</div>';
var actual = axe.commons.color.getBackgroundColor(document.getElementById('target'), []);
assert.isNull(actual);
assert.equal(axe.commons.color.incompleteData.get('bgColor'), 'elmPartiallyObscuring');
});

it('should count a TR as a background element for TD', function () {
fixture.innerHTML = '<div style="background-color:#007acc;">' +
'<table style="width:100%">' +
Expand Down
11 changes: 11 additions & 0 deletions test/rule-matches/color-contrast-matches.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,18 @@ describe('color-contrast-matches', function () {
var target = fixture.querySelector('input');
var tree = axe._tree = axe.utils.getFlattenedTree(fixture);
assert.isFalse(rule.matches(target, axe.utils.getNodeFromTree(tree[0], target)));
});

it('should not match a disabled implicit label child', function () {
fixture.innerHTML = '<label>' +
'<input type="checkbox" style="position: absolute;display: inline-block;width: 1.5rem;height: 1.5rem;opacity: 0;" disabled checked>' +
'<span style="background-color:rgba(0, 0, 0, 0.26);display:inline-block;width: 1.5rem;height: 1.5rem;" aria-hidden="true"></span>' +
'<span style="color:rgba(0, 0, 0, 0.38);" id="target">Baseball</span>' +
'</label>';
var target = fixture.querySelector('#target');
var tree = axe._tree = axe.utils.getFlattenedTree(fixture);
var result = rule.matches(target, axe.utils.getNodeFromTree(tree[0], target));
assert.isFalse(result);
});

it('should not match <textarea disabled>', function () {
Expand Down