Skip to content

Commit

Permalink
fix(color-contrast): take into account parent opacity for foreground …
Browse files Browse the repository at this point in the history
…color (#1902)

* fix(color-contrast): take into account parent opacity for foreground color

* fix rounding errors

* proper closeTo params

* update

* test recursive
  • Loading branch information
straker authored Nov 18, 2019
1 parent 7d2b2a6 commit 8719700
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lib/checks/color/color-contrast.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ if (!dom.isVisible(node, false)) {
const noScroll = !!(options || {}).noScroll;
const bgNodes = [];
const bgColor = color.getBackgroundColor(node, bgNodes, noScroll);
const fgColor = color.getForegroundColor(node, noScroll);
const fgColor = color.getForegroundColor(node, noScroll, bgColor);

const nodeStyle = window.getComputedStyle(node);
const fontSize = parseFloat(nodeStyle.getPropertyValue('font-size'));
Expand Down
41 changes: 35 additions & 6 deletions lib/commons/color/get-foreground-color.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,30 @@
/* global axe, color */

function getOpacity(node) {
if (!node) {
return 1;
}

const vNode = axe.utils.getNodeFromTree(node);

if (vNode && vNode._opacity !== undefined && vNode._opacity !== null) {
return vNode._opacity;
}

const nodeStyle = window.getComputedStyle(node);
const opacity = nodeStyle.getPropertyValue('opacity');
const finalOpacity = opacity * getOpacity(node.parentElement);

// cache the results of the getOpacity check on the parent tree
// so we don't have to look at the parent tree again for all its
// descendants
if (vNode) {
vNode._opacity = finalOpacity;
}

return finalOpacity;
}

/**
* Returns the flattened foreground color of an element, or null if it can't be determined because
* of transparency
Expand All @@ -8,22 +33,26 @@
* @instance
* @param {Element} node
* @param {Boolean} noScroll (default false)
* @param {Color} bgColor
* @return {Color|null}
*/
color.getForegroundColor = function(node, noScroll) {
var nodeStyle = window.getComputedStyle(node);
color.getForegroundColor = function(node, noScroll, bgColor) {
const nodeStyle = window.getComputedStyle(node);

var fgColor = new color.Color();
const fgColor = new color.Color();
fgColor.parseRgbString(nodeStyle.getPropertyValue('color'));
var opacity = nodeStyle.getPropertyValue('opacity');
const opacity = getOpacity(node);
fgColor.alpha = fgColor.alpha * opacity;
if (fgColor.alpha === 1) {
return fgColor;
}

var bgColor = color.getBackgroundColor(node, [], noScroll);
if (!bgColor) {
bgColor = color.getBackgroundColor(node, [], noScroll);
}

if (bgColor === null) {
var reason = axe.commons.color.incompleteData.get('bgColor');
const reason = axe.commons.color.incompleteData.get('bgColor');
axe.commons.color.incompleteData.set('fgColor', reason);
return null;
}
Expand Down
44 changes: 44 additions & 0 deletions test/commons/color/get-foreground-color.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global sinon */

describe('color.getForegroundColor', function() {
'use strict';

Expand Down Expand Up @@ -42,6 +44,39 @@ describe('color.getForegroundColor', function() {
assert.equal(actual.alpha, expected.alpha);
});

it('should take into account parent opacity tree', function() {
fixture.innerHTML =
'<div style="background-color: #fafafa">' +
'<div style="height: 40px; width: 30px; opacity: 0.6">' +
'<div id="target" style="height: 20px; width: 15px; color: rgba(0, 0, 0, 0.87);">' +
'This is my text' +
'</div></div></div>';
var target = fixture.querySelector('#target');
var actual = axe.commons.color.getForegroundColor(target);
var expected = new axe.commons.color.Color(119.5, 119.5, 119.5, 1);
assert.closeTo(actual.red, expected.red, 0.8);
assert.closeTo(actual.green, expected.green, 0.8);
assert.closeTo(actual.blue, expected.blue, 0.8);
assert.closeTo(actual.alpha, expected.alpha, 0.1);
});

it('should take into account entire parent opacity tree', function() {
fixture.innerHTML =
'<div style="background-color: #fafafa">' +
'<div style="height: 40px; width: 30px; opacity: 0.75">' +
'<div style="height: 40px; width: 30px; opacity: 0.8">' +
'<div id="target" style="height: 20px; width: 15px; color: rgba(0, 0, 0, 0.87);">' +
'This is my text' +
'</div></div></div></div>';
var target = fixture.querySelector('#target');
var actual = axe.commons.color.getForegroundColor(target);
var expected = new axe.commons.color.Color(119.5, 119.5, 119.5, 1);
assert.closeTo(actual.red, expected.red, 0.8);
assert.closeTo(actual.green, expected.green, 0.8);
assert.closeTo(actual.blue, expected.blue, 0.8);
assert.closeTo(actual.alpha, expected.alpha, 0.1);
});

it('should return null if containing parent has a background image and is non-opaque', function() {
fixture.innerHTML =
'<div style="height: 40px; width: 30px;' +
Expand All @@ -68,6 +103,15 @@ describe('color.getForegroundColor', function() {
assert.equal(actual.alpha, expected.alpha);
});

it('should not recalculate bgColor if passed in', function() {
var spy = sinon.spy(axe.commons.color, 'getBackgroundColor');
var bgColor = new axe.commons.color.Color(255, 255, 255, 1);
var node = document.createElement('div');
axe.commons.color.getForegroundColor(node, false, bgColor);
assert.isFalse(spy.called);
spy.restore();
});

(shadowSupported ? it : xit)(
'should return the fgcolor from inside of Shadow DOM',
function() {
Expand Down
1 change: 1 addition & 0 deletions test/runner.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<link rel="stylesheet" type="text/css" href="/node_modules/mocha/mocha.css" />
<script src="/node_modules/mocha/mocha.js"></script>
<script src="/node_modules/chai/chai.js"></script>
<script src="/node_modules/sinon/pkg/sinon.js"></script>
<style>
/* This is essential to stop mocha reporter created anchors from making the document overflow, mainly along the x-axis */
#mocha h1 a {
Expand Down

0 comments on commit 8719700

Please sign in to comment.