Skip to content

Commit

Permalink
fix: Include body as part of background color checks when element doe…
Browse files Browse the repository at this point in the history
…s not intersect (#1520)

* fix: include body as part of background color checks when element does not intersect

* add test coverage for html element

* logic fix + general comments
  • Loading branch information
scurker authored and WilcoFiers committed May 28, 2019
1 parent 2f2e590 commit 55820cf
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 13 deletions.
17 changes: 10 additions & 7 deletions build/tasks/test-webdriver.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,23 @@ module.exports = function(grunt) {
.get(url)
// Get results
.then(function() {
let driverBrowser = driver
.getCapabilities()
.then(capabilities => capabilities.get('browserName'));
return Promise.all([driverBrowser, collectTestResults(driver)]);
return Promise.all([
driver.getCapabilities(),
collectTestResults(driver)
]);
})
// And process them
.then(function([browser, result]) {
grunt.log.writeln(url + ' [' + browser + ']');
.then(function([capabilities, result]) {
let browserName =
capabilities.get('browserName') +
(capabilities.get('mobileEmulationEnabled') ? '-mobile' : '');
grunt.log.writeln(url + ' [' + browserName + ']');

// Remember the errors
(result.reports || []).forEach(function(err) {
grunt.log.error(err.message);
err.url = url;
err.browser = browser;
err.browser = browserName;
errors.push(err);
});

Expand Down
21 changes: 17 additions & 4 deletions lib/commons/color/get-background-color.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,29 @@ function sortPageBackground(elmStack) {
let bodyIndex = elmStack.indexOf(document.body);
let bgNodes = elmStack;

// Body can sometimes appear out of order in the stack:
// 1) Body is not the first element due to negative z-index elements
// 2) Elements are positioned outside of body's rect coordinates
// (see https://github.com/dequelabs/axe-core/issues/1456)
// In those instances we want to reinsert body back into the element stack
// when not using the root document element as the html canvas for bgcolor
// prettier-ignore
let sortBodyElement =
bodyIndex > 1 || // negative z-index elements
bodyIndex === -1; // element does not intersect with body

if (
// Check that the body background is the page's background
bodyIndex > 1 && // only if there are negative z-index elements
sortBodyElement &&
!color.elementHasImage(document.documentElement) &&
color.getOwnBackgroundColor(
window.getComputedStyle(document.documentElement)
).alpha === 0
) {
// Remove body and html from it's current place
bgNodes.splice(bodyIndex, 1);
// Only remove document.body if it was originally contained within the element stack
if (bodyIndex > 1) {
bgNodes.splice(bodyIndex, 1);
}
// Remove document element since body will be used for bgcolor
bgNodes.splice(elmStack.indexOf(document.documentElement), 1);

// Put the body background as the lowest element
Expand Down
47 changes: 47 additions & 0 deletions test/commons/color/get-background-color.js
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,53 @@ describe('color.getBackgroundColor', function() {
assert.closeTo(actual.alpha, 1, 0);
});

it('should return the html canvas inherited from body bgColor when element content does not overlap with body', function() {
fixture.innerHTML =
'<div id="target" style="position: relative; top: 2px;">Text</div>';

// size body element so that target element is positioned outside of background
var originalHeight = document.body.style.height;
var originalBg = document.body.style.background;
document.body.style.height = '1px';
document.body.style.background = '#000';

var target = fixture.querySelector('#target');
var actual = axe.commons.color.getBackgroundColor(target, []);

assert.closeTo(actual.red, 0, 0);
assert.closeTo(actual.green, 0, 0);
assert.closeTo(actual.blue, 0, 0);
assert.closeTo(actual.alpha, 1, 0);

document.body.style.height = originalHeight;
document.body.style.background = originalBg;
});

it('should return the html canvas bgColor when element content does not overlap with body', function() {
fixture.innerHTML =
'<div id="target" style="position: relative; top: 2px;">Text</div>';

// size body element so that target element is positioned outside of background
var originalHeight = document.body.style.height;
var originalBg = document.body.style.background;
var originalRootBg = document.documentElement.style.background;
document.body.style.height = '1px';
document.body.style.background = '#0f0';
document.documentElement.style.background = '#f00';

var target = fixture.querySelector('#target');
var actual = axe.commons.color.getBackgroundColor(target, []);

assert.closeTo(actual.red, 255, 0);
assert.closeTo(actual.green, 0, 0);
assert.closeTo(actual.blue, 0, 0);
assert.closeTo(actual.alpha, 1, 0);

document.body.style.height = originalHeight;
document.body.style.background = originalBg;
document.documentElement.style.background = originalRootBg;
});

(shadowSupported ? it : xit)('finds colors in shadow boundaries', function() {
fixture.innerHTML = '<div id="container"></div>';
var container = fixture.querySelector('#container');
Expand Down
3 changes: 2 additions & 1 deletion test/commons/dom/visually-overlaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ describe('dom.visuallyOverlaps', function() {
it('should return false when rect has no overlap', function() {
fixture.innerHTML =
'<div style="position: absolute; top: 0px; left: 0px; height: 40px;' +
' width: 30px; background-color: red;"></div>' +
' width: 30px; background-color: red;">' +
'<div id="target" style="position: absolute; top: 50px; left: 0px; height: 20px;' +
' width: 45px; background-color: green;">' +
'</div></div>';
var target = fixture.querySelector('#target');
var targetRect = target.getBoundingClientRect();

assert.isFalse(
axe.commons.dom.visuallyOverlaps(targetRect, target.parentNode)
);
Expand Down
7 changes: 6 additions & 1 deletion test/runner.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
word-wrap: break-word;
hyphens: auto;
}

/* mocha stats having a fixed position can interfere with some tests when intersecting with elements */
#mocha-stats {
position: static;
}
</style>
<script>
mocha.setup({
Expand All @@ -30,8 +35,8 @@
</head>

<body>
<div id="mocha"></div>
<div id="fixture"></div>
<div id="mocha"></div>
<script src="/test/testutils.js"></script>
<% tests.forEach(function (file) { %>
<script src="<%=file%>"></script>
Expand Down

0 comments on commit 55820cf

Please sign in to comment.