From 5dedb6076dd97471cb95e32faeeb5d03508f21e8 Mon Sep 17 00:00:00 2001 From: Mike de Boer Date: Thu, 4 Apr 2019 16:36:17 +0000 Subject: [PATCH] Bug 1541829 - Fixup the relative luminance implementation in the Color class and update its consumers with the correct API. r=jaws Since we rarely touch this code, I took the liberty of changing this to a JS class and fix the contrast ratio calculations to actually conform to the WCAG spec, instead of using arbitrary constants. I changed the `isBright` getter to `useBrightText`, because that is more apt; we're usually looking for an answer to 'should I use white text on this background?', instead of looking for an arbitrary threshold to classify a color as being bright. I updated the tests to cover more of this and clarified the assertion messages as well. Differential Revision: https://phabricator.services.mozilla.com/D26097 --HG-- extra : moz-landing-system : lando --- toolkit/modules/Color.jsm | 80 +++++++++++++------- toolkit/modules/FinderHighlighter.jsm | 2 +- toolkit/modules/tests/xpcshell/test_Color.js | 19 ++--- 3 files changed, 65 insertions(+), 36 deletions(-) diff --git a/toolkit/modules/Color.jsm b/toolkit/modules/Color.jsm index e56e2b9b5640..c394c321720c 100644 --- a/toolkit/modules/Color.jsm +++ b/toolkit/modules/Color.jsm @@ -6,6 +6,33 @@ var EXPORTED_SYMBOLS = ["Color"]; +/** + * A list of minimum contrast ratio's per WCAG 2.0 conformance level. + * Please refer to section 1.4.3 of the WCAG 2.0 spec at http://www.w3.org/TR/WCAG20/. + * Simply put: + * A = Large text only. + * AA = Regular sized text or large text in enhanced contrast mode. + * AAA = Regular sized text in enhanced contrast mode. + * + * @type {Object} + */ +const CONTRAST_RATIO_LEVELS = { + A: 3, + AA: 4.5, + AAA: 7, +}; + +/** + * For text legibility on any background color, you need to determine which text + * color - black or white - will yield the highest contrast ratio. + * Since you're always comparing `contrastRatio(bgcolor, black) > + * contrastRatio(bgcolor, white) ? : `, we can greatly + * simplify the calculation to the following constant. + * + * @type {Number} + */ +const CONTRAST_BRIGHTTEXT_THRESHOLD = Math.sqrt(1.05 * 0.05) - 0.05; + /** * Color class, which describes a color. * In the future, this object may be extended to allow for conversions between @@ -15,13 +42,13 @@ var EXPORTED_SYMBOLS = ["Color"]; * @param {Number} g Green color component * @param {Number} b Blue color component */ -function Color(r, g, b) { - this.r = r; - this.g = g; - this.b = b; -} +class Color { + constructor(r, g, b) { + this.r = r; + this.g = g; + this.b = b; + } -Color.prototype = { /** * Formula from W3C's WCAG 2.0 spec's relative luminance, section 1.4.1, * http://www.w3.org/TR/WCAG20/. @@ -29,25 +56,25 @@ Color.prototype = { * @return {Number} Relative luminance, represented as number between 0 and 1. */ get relativeLuminance() { - let colorArr = [this.r, this.b, this.g].map(color => { + let colorArr = [this.r, this.g, this.b].map(color => { color = parseInt(color, 10); - if (color <= 10) + if (color <= 10) { return color / 255 / 12.92; + } return Math.pow(((color / 255) + 0.055) / 1.055, 2.4); }); return colorArr[0] * 0.2126 + colorArr[1] * 0.7152 + colorArr[2] * 0.0722; - }, + } /** - * @return {Boolean} TRUE if the color value can be considered bright. + * @return {Boolean} TRUE if you need to use a bright color (e.g. 'white'), when + * this color is set as the background. */ - get isBright() { - // Note: this is a high enough value to be considered as 'bright', but was - // decided upon empirically. - return this.relativeLuminance > 0.7; - }, + get useBrightText() { + return this.relativeLuminance <= CONTRAST_BRIGHTTEXT_THRESHOLD; + } /** * Get the contrast ratio between the current color and a second other color. @@ -61,25 +88,26 @@ Color.prototype = { * as 1:1 to 21:1. */ contrastRatio(otherColor) { - if (!(otherColor instanceof Color)) + if (!(otherColor instanceof Color)) { throw new TypeError("The first argument should be an instance of Color"); + } let luminance = this.relativeLuminance; let otherLuminance = otherColor.relativeLuminance; return (Math.max(luminance, otherLuminance) + 0.05) / (Math.min(luminance, otherLuminance) + 0.05); - }, + } /** - * Biased method to check if the contrast ratio between two colors is high - * enough to be discernable. + * Method to check if the contrast ratio between two colors is high enough to + * be discernable. * - * @param {Color} otherColor Color instance to calculate the contrast with + * @param {Color} otherColor Color instance to calculate the contrast with + * @param {String} [level] WCAG conformance level that maps to the minimum + * required contrast ratio. Defaults to 'AA' * @return {Boolean} */ - isContrastRatioAcceptable(otherColor) { - // Note: this is a high enough value to be considered as 'high contrast', - // but was decided upon empirically. - return this.contrastRatio(otherColor) > 3; - }, -}; + isContrastRatioAcceptable(otherColor, level = "AA") { + return this.contrastRatio(otherColor) > CONTRAST_RATIO_LEVELS[level]; + } +} diff --git a/toolkit/modules/FinderHighlighter.jsm b/toolkit/modules/FinderHighlighter.jsm index 0ccb8372bf67..31f508abcb76 100644 --- a/toolkit/modules/FinderHighlighter.jsm +++ b/toolkit/modules/FinderHighlighter.jsm @@ -734,7 +734,7 @@ FinderHighlighter.prototype = { if (!cssColor || !cssColor.length) return false; cssColor.shift(); - return new Color(...cssColor).isBright; + return !(new Color(...cssColor)).useBrightText; }, /** diff --git a/toolkit/modules/tests/xpcshell/test_Color.js b/toolkit/modules/tests/xpcshell/test_Color.js index 302065ca57f4..19e23948b271 100644 --- a/toolkit/modules/tests/xpcshell/test_Color.js +++ b/toolkit/modules/tests/xpcshell/test_Color.js @@ -4,7 +4,7 @@ const {Color} = ChromeUtils.import("resource://gre/modules/Color.jsm"); function run_test() { testRelativeLuminance(); - testIsBright(); + testUseBrightText(); testContrastRatio(); testIsContrastRatioAcceptable(); } @@ -17,16 +17,15 @@ function testRelativeLuminance() { Assert.equal(c.relativeLuminance, 1, "White is quite the luminant one"); c = new Color(142, 42, 142); - Assert.equal(c.relativeLuminance, 0.25263952353998204, - "This purple is not that luminant"); + Assert.equal(c.relativeLuminance, 0.09359705837110571, "This purple is not that luminant"); } -function testIsBright() { +function testUseBrightText() { let c = new Color(0, 0, 0); - Assert.equal(c.isBright, 0, "Black is bright"); + Assert.ok(c.useBrightText, "Black is bright, so bright text should be used here"); c = new Color(255, 255, 255); - Assert.equal(c.isBright, 1, "White is bright"); + Assert.ok(!c.useBrightText, "White is bright, so better not use bright colored text on it"); } function testContrastRatio() { @@ -36,8 +35,8 @@ function testContrastRatio() { Assert.equal(c.contrastRatio(c), 1, "Contrast between equals is min"); let c3 = new Color(142, 42, 142); - Assert.equal(c.contrastRatio(c3), 6.05279047079964, "Contrast between black and purple"); - Assert.equal(c2.contrastRatio(c3), 3.469474137806338, "Contrast between white and purple"); + Assert.equal(c.contrastRatio(c3), 2.871941167422114, "Contrast between black and purple shouldn't be very high"); + Assert.equal(c2.contrastRatio(c3), 7.312127503938331, "Contrast between white and purple should be high"); } function testIsContrastRatioAcceptable() { @@ -48,6 +47,8 @@ function testIsContrastRatioAcceptable() { Assert.equal(c.g, 156, "Greens should match"); Assert.equal(c.b, 152, "Blues should match"); Assert.ok(c.isContrastRatioAcceptable(c2), "The blue is high contrast enough"); - c = new Color(...[35, 65, 100]); + c2 = new Color(...[35, 65, 100]); Assert.ok(!c.isContrastRatioAcceptable(c2), "The blue is not high contrast enough"); + // But would it be high contrast enough at a lower conformance level? + Assert.ok(c.isContrastRatioAcceptable(c2, "A"), "The blue is high contrast enough when used as large text"); }