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: corrects color-contrast tests with overlapping elements #378

Closed
wants to merge 1 commit into from
Closed

fix: corrects color-contrast tests with overlapping elements #378

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
83 changes: 41 additions & 42 deletions lib/commons/color/get-background-color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ const graphicNodes = [
/**
* Reports if an element has a background image or gradient
* @private
* @param {Element} elm
* @param {Element} node
* @param {Object|null} style
* @return {Boolean}
*/
function elmHasImage(elm, style) {
var nodeName = elm.nodeName.toUpperCase();
function elmHasImage(node, style) {
var nodeName = node.nodeName.toUpperCase();
if (graphicNodes.includes(nodeName)) {
axe.commons.color.incompleteData.set('bgColor', 'imgNode');
return true;
}

style = style || window.getComputedStyle(elm);
style = style || window.getComputedStyle(node);
var bgImageStyle = style.getPropertyValue('background-image');
var hasBgImage = bgImageStyle !== 'none';
if (hasBgImage) {
Expand All @@ -30,11 +30,11 @@ function elmHasImage(elm, style) {
/**
* Returns the non-alpha-blended background color of an element
* @private
* @param {Element} elm
* @param {Element} node
* @return {Color}
*/
function getBgColor(elm, elmStyle) {
elmStyle = elmStyle || window.getComputedStyle(elm);
function getBgColor(node, elmStyle) {
elmStyle = elmStyle || window.getComputedStyle(node);

let bgColor = new color.Color();
bgColor.parseRgbString( elmStyle.getPropertyValue('background-color'));
Expand All @@ -53,17 +53,17 @@ function getBgColor(elm, elmStyle) {
* @param {Element} bgNode
* @return {Boolean}
*/
function contentOverlapping(targetElement, bgNode) {
function contentOverlapping(targetElement, fgNode) {
// get content box of target element
// check to see if the current bgNode is overlapping
var targetRect = targetElement.getClientRects()[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: I used getClientRects because it would target the diff in coverage for an inline label wrapping an input, as opposed to the outer bounding box of the label. I guess with this change, the fgNode is the input instead of the label in that case?

Copy link
Contributor Author

@downzer0 downzer0 Jun 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The fgNode in this case would be the foreground "whatever". The function receives two element parameters - one that's the "background" and one that's potentially overlapping (generally a child element). The only way you'd have overlapping would be with absolute positioning, but you'd still need a parent/child combo, right? Are there any scenarios that might not be covered here?

Copy link
Contributor

@marcysutton marcysutton Jun 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

When the input was under audit and its parent was an implicit, inline label, the label would be skipped and not considered a background unless it was added to the stack manually. (also the label itself as a node under audit, which contains the actual text content). I couldn't guarantee which element was in the front of the element stack since the node context would be either the child or the parent. If you look at the examples in this MDN doc, you can see getClientRects can be used to check inline content overlap. https://developer.mozilla.org/en-US/docs/Web/API/Element/getClientRects

But if this works, awesome. I want to run it through the aXe extension to be sure.

var obscuringElements = document.elementsFromPoint(targetRect.left, targetRect.top);
if (obscuringElements) {
for(var i = 0; i < obscuringElements.length; i++) {
if (obscuringElements[i] !== targetElement && obscuringElements[i] === bgNode) {
return true;
}
}
// check to see if the current fgNode is overlapping
var targetRect = targetElement.getBoundingClientRect();
var fgNodeRect = fgNode.getBoundingClientRect();
var overlappingNodes = !(targetRect.right < fgNodeRect.left ||
targetRect.left > fgNodeRect.right ||
targetRect.bottom < fgNodeRect.top ||
targetRect.top > fgNodeRect.bottom);
if (overlappingNodes) {
return true;
}
return false;
}
Expand Down Expand Up @@ -97,13 +97,13 @@ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) {
/**
* Determine if element is partially overlapped, triggering a Can't Tell result
* @private
* @param {Element} elm
* @param {Element} node
* @param {Element} bgElm
* @param {Object} bgColor
* @return {Boolean}
*/
function elmPartiallyObscured(elm, bgElm, bgColor) {
var obscured = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0);
function elmPartiallyObscured(node, bgElm, bgColor) {
var obscured = (node !== bgElm && !dom.visuallyContains(node, bgElm) && bgColor.alpha !== 0);
if (obscured) {
axe.commons.color.incompleteData.set('bgColor', 'elmPartiallyObscured');
}
Expand All @@ -118,31 +118,31 @@ function elmPartiallyObscured(elm, bgElm, bgColor) {
*
* @private
* @param {Array} elmStack
* @param {Element} elm
* @param {Element} node
*/
function includeMissingElements(elmStack, elm) {
function includeMissingElements(elmStack, node) {
const elementMap = {'TD': 'TR', 'INPUT': 'LABEL'};
const tagArray = elmStack.map((elm) => {
return elm.tagName;
const tagArray = elmStack.map((node) => {
return node.tagName;
});
let bgNodes = elmStack;
for (let candidate in elementMap) {
if (elementMap.hasOwnProperty(candidate)) {
// tagName matches key
if (elm.tagName === candidate) {
let ancestorMatch = axe.commons.dom.findUp(elm, elementMap[candidate]);
if (node.tagName === candidate) {
let ancestorMatch = axe.commons.dom.findUp(node, elementMap[candidate]);
if (ancestorMatch && elmStack.indexOf(ancestorMatch) === -1) {
// found an ancestor not in elmStack, and it overlaps
let overlaps = axe.commons.dom.visuallyOverlaps(elm.getBoundingClientRect(), ancestorMatch);
let overlaps = axe.commons.dom.visuallyOverlaps(node.getBoundingClientRect(), ancestorMatch);
if (overlaps) {
bgNodes.splice(elmStack.indexOf(elm) + 1, 0, ancestorMatch);
bgNodes.splice(elmStack.indexOf(node) + 1, 0, ancestorMatch);
}
}
}
// tagName matches value
// (such as LABEL, when matching itself. It should be in the list, but Phantom skips it)
if (elm.tagName === elementMap[candidate] && tagArray.indexOf(elm.tagName) === -1) {
bgNodes.splice(tagArray.indexOf(candidate) + 1, 0, elm);
if (node.tagName === elementMap[candidate] && tagArray.indexOf(node.tagName) === -1) {
bgNodes.splice(tagArray.indexOf(candidate) + 1, 0, node);
}
}
}
Expand Down Expand Up @@ -178,8 +178,8 @@ function sortPageBackground(elmStack) {
* Get all elements rendered underneath the current element,
* in the order in which it is rendered
*/
color.getBackgroundStack = function(elm) {
let rect = elm.getBoundingClientRect();
color.getBackgroundStack = function(node) {
let rect = node.getBoundingClientRect();
let x, y;
if (rect.left > window.innerWidth) {
return;
Expand All @@ -195,27 +195,26 @@ color.getBackgroundStack = function(elm) {
window.innerHeight - 1);

let elmStack = document.elementsFromPoint(x, y);
elmStack = includeMissingElements(elmStack, elm);
elmStack = dom.reduceToElementsBelowFloating(elmStack, elm);
elmStack = includeMissingElements(elmStack, node);
elmStack = dom.reduceToElementsBelowFloating(elmStack, node);
elmStack = sortPageBackground(elmStack);

// Return all elements BELOW the current element, null if the element is undefined
let elmIndex = elmStack.indexOf(elm);
if (calculateObscuringAlpha(elmIndex, elmStack, elm) >= 0.99) {
let elmIndex = elmStack.indexOf(node);
if (calculateObscuringAlpha(elmIndex, elmStack, node) >= 0.99) {
// if the total of the elements above our element results in total obscuring, return null
axe.commons.color.incompleteData.set('bgColor', 'bgOverlap');
return null;
}
return elmIndex !== -1 ? elmStack : null;
};

color.getBackgroundColor = function(elm, bgElms = [], noScroll = false) {
color.getBackgroundColor = function(node, bgElms = [], noScroll = false) {
if(noScroll !== true) {
elm.scrollIntoView();
node.scrollIntoView();
}
let bgColors = [];
let elmStack = color.getBackgroundStack(elm);

let elmStack = color.getBackgroundStack(node);
// Search the stack until we have an alpha === 1 background
(elmStack || []).some((bgElm) => {
let bgElmStyle = window.getComputedStyle(bgElm);
Expand All @@ -224,8 +223,8 @@ color.getBackgroundColor = function(elm, bgElms = [], noScroll = false) {
let bgColor = getBgColor(bgElm, bgElmStyle);

if (// abort if a node is partially obscured and obscuring element has a background
elmPartiallyObscured(elm, bgElm, bgColor) ||
// OR if the background elm is a graphic
elmPartiallyObscured(node, bgElm, bgColor) ||
// OR if the background node is a graphic
elmHasImage(bgElm, bgElmStyle)
) {
bgColors = null;
Expand Down
Loading