Bug 1303826 - don't select upper-case color names by default; r?gregtatum draft
authorTom Tromey <tom@tromey.com>
Tue, 20 Sep 2016 14:46:39 -0600
changeset 415677 0e1418129c148c35512cb806cf94dcaa38fa1807
parent 415606 6396e3ab4695af0b3815bd09255c48d182f06540
child 531669 e345dfea18d32b2b71161c26ce42b9ddd7aebd9e
push id29934
push userbmo:ttromey@mozilla.com
push dateTue, 20 Sep 2016 20:47:58 +0000
reviewersgregtatum
bugs1303826
milestone52.0a1
Bug 1303826 - don't select upper-case color names by default; r?gregtatum MozReview-Commit-ID: 6XfK1aPhSxx
devtools/client/shared/test/unit/test_cssColor.js
devtools/shared/css/color.js
--- a/devtools/client/shared/test/unit/test_cssColor.js
+++ b/devtools/client/shared/test/unit/test_cssColor.js
@@ -57,9 +57,20 @@ function run_test() {
 
     // Check that our implementation matches DOMUtils.
     compareWithDomutils(test.input, true);
 
     // And check some obvious errors.
     compareWithDomutils("mumble" + test.input, false);
     compareWithDomutils(test.input + "trailingstuff", false);
   }
+
+  // Regression test for bug 1303826.
+  let black = new colorUtils.CssColor("#000");
+  black.colorUnit = "name";
+  equal(black.toString(), "black", "test non-upper-case color cycling");
+
+  let upper = new colorUtils.CssColor("BLACK");
+  upper.colorUnit = "hex";
+  equal(upper.toString(), "#000", "test upper-case color cycling");
+  upper.colorUnit = "name";
+  equal(upper.toString(), "BLACK", "test upper-case color preservation");
 }
--- a/devtools/shared/css/color.js
+++ b/devtools/shared/css/color.js
@@ -84,22 +84,29 @@ CssColor.prototype = {
   _colorUnit: null,
   _colorUnitUppercase: false,
 
   // The value as-authored.
   authored: null,
   // A lower-cased copy of |authored|.
   lowerCased: null,
 
+  _setColorUnitUppercase: function (color) {
+    // Specifically exclude the case where the color is
+    // case-insensitive.  This makes it so that "#000" isn't
+    // considered "upper case" for the purposes of color cycling.
+    this._colorUnitUppercase = (color === color.toUpperCase()) &&
+      (color !== color.toLowerCase());
+  },
+
   get colorUnit() {
     if (this._colorUnit === null) {
       let defaultUnit = Services.prefs.getCharPref(COLOR_UNIT_PREF);
       this._colorUnit = CssColor.COLORUNIT[defaultUnit];
-      this._colorUnitUppercase =
-        (this.authored === this.authored.toUpperCase());
+      this._setColorUnitUppercase(this.authored);
     }
     return this._colorUnit;
   },
 
   set colorUnit(unit) {
     this._colorUnit = unit;
   },
 
@@ -109,17 +116,17 @@ CssColor.prototype = {
    * color unit untouched.
    *
    * @param {String} color The color to use
    */
   setAuthoredUnitFromColor: function (color) {
     if (Services.prefs.getCharPref(COLOR_UNIT_PREF) ===
         CssColor.COLORUNIT.authored) {
       this._colorUnit = classifyColor(color);
-      this._colorUnitUppercase = (color === color.toUpperCase());
+      this._setColorUnitUppercase(color);
     }
   },
 
   get hasAlpha() {
     if (!this.valid) {
       return false;
     }
     return this._getRGBATuple().a !== 1;
@@ -316,16 +323,17 @@ CssColor.prototype = {
    *         Any valid color string
    */
   newColor: function (color) {
     // Store a lower-cased version of the color to help with format
     // testing.  The original text is kept as well so it can be
     // returned when needed.
     this.lowerCased = color.toLowerCase();
     this.authored = color;
+    this._setColorUnitUppercase(color);
     return this;
   },
 
   nextColorUnit: function () {
     // Reorder the formats array to have the current format at the
     // front so we can cycle through.
     let formats = ["hex", "hsl", "rgb", "name"];
     let currentFormat = classifyColor(this.toString());