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

@@ -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) ? <use black> : <use 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];
+  }
+}

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;
   },
 
   /**

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");
 }