Bug 965181 - respect default color unit when choosing a color. r=mratcliffe
authorTom Tromey <tromey@mozilla.com>
Wed, 07 Oct 2015 12:04:00 +0200
changeset 266914 cd09b70dd073e06259126900a1af6b9c6207c6ef
parent 266913 14574ff2009cb332070e3bf4297340076fdb0120
child 266915 ddc16c7d513d09f5b1e1bbea1abfac268105dc97
push id29503
push usercbook@mozilla.com
push dateFri, 09 Oct 2015 09:36:47 +0000
treeherdermozilla-central@462074ffada4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmratcliffe
bugs965181
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 965181 - respect default color unit when choosing a color. r=mratcliffe
devtools/client/shared/widgets/Tooltip.js
devtools/client/styleinspector/test/browser.ini
devtools/client/styleinspector/test/browser_ruleview_colorUnit.js
devtools/shared/css-color.js
devtools/shared/tests/unit/test_cssColor.js
--- a/devtools/client/shared/widgets/Tooltip.js
+++ b/devtools/client/shared/widgets/Tooltip.js
@@ -1140,18 +1140,17 @@ SwatchColorPickerTooltip.prototype = Her
    * color.
    */
   show: function() {
     // Call then parent class' show function
     SwatchBasedEditorTooltip.prototype.show.call(this);
     // Then set spectrum's color and listen to color changes to preview them
     if (this.activeSwatch) {
       this.currentSwatchColor = this.activeSwatch.nextSibling;
-      this._colorUnit =
-        colorUtils.classifyColor(this.currentSwatchColor.textContent);
+      this._originalColor = this.currentSwatchColor.textContent;
       let color = this.activeSwatch.style.backgroundColor;
       this.spectrum.then(spectrum => {
         spectrum.off("changed", this._onSpectrumColorChange);
         spectrum.rgb = this._colorToRgba(color);
         spectrum.on("changed", this._onSpectrumColorChange);
         spectrum.updateUI();
       });
     }
@@ -1219,17 +1218,17 @@ SwatchColorPickerTooltip.prototype = Her
   _colorToRgba: function(color) {
     color = new colorUtils.CssColor(color);
     let rgba = color._getRGBATuple();
     return [rgba.r, rgba.g, rgba.b, rgba.a];
   },
 
   _toDefaultType: function(color) {
     let colorObj = new colorUtils.CssColor(color);
-    colorObj.colorUnit = this._colorUnit;
+    colorObj.setAuthoredUnitFromColor(this._originalColor);
     return colorObj.toString();
   },
 
   destroy: function() {
     SwatchBasedEditorTooltip.prototype.destroy.call(this);
     this.currentSwatchColor = null;
     this.spectrum.then(spectrum => {
       spectrum.off("changed", this._onSpectrumColorChange);
--- a/devtools/client/styleinspector/test/browser.ini
+++ b/devtools/client/styleinspector/test/browser.ini
@@ -67,16 +67,17 @@ support-files =
 [browser_ruleview_colorpicker-appears-on-swatch-click.js]
 [browser_ruleview_colorpicker-commit-on-ENTER.js]
 [browser_ruleview_colorpicker-edit-gradient.js]
 [browser_ruleview_colorpicker-hides-on-tooltip.js]
 [browser_ruleview_colorpicker-multiple-changes.js]
 [browser_ruleview_colorpicker-release-outside-frame.js]
 [browser_ruleview_colorpicker-revert-on-ESC.js]
 [browser_ruleview_colorpicker-swatch-displayed.js]
+[browser_ruleview_colorUnit.js]
 [browser_ruleview_completion-existing-property_01.js]
 [browser_ruleview_completion-existing-property_02.js]
 [browser_ruleview_completion-new-property_01.js]
 [browser_ruleview_completion-new-property_02.js]
 [browser_ruleview_computed-lists_01.js]
 [browser_ruleview_computed-lists_02.js]
 [browser_ruleview_completion-popup-hidden-after-navigation.js]
 [browser_ruleview_content_01.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/styleinspector/test/browser_ruleview_colorUnit.js
@@ -0,0 +1,59 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that color selection respects the user pref.
+
+const TEST_URI = `
+  <style type='text/css'>
+    #testid {
+      color: blue;
+    }
+  </style>
+  <div id='testid' class='testclass'>Styled Node</div>
+`;
+
+add_task(function*() {
+  let TESTS = [
+    {name: "hex", result: "#0F0"},
+    {name: "rgb", result: "rgb(0, 255, 0)"}
+  ];
+
+  for (let {name, result} of TESTS) {
+    info("starting test for " + name);
+    Services.prefs.setCharPref("devtools.defaultColorUnit", name);
+
+    yield addTab("data:text/html;charset=utf-8," +
+                 encodeURIComponent(TEST_URI));
+    let {inspector, view} = yield openRuleView();
+    yield selectNode("#testid", inspector);
+    yield basicTest(view, name, result);
+  }
+});
+
+function* basicTest(view, name, result) {
+  let cPicker = view.tooltips.colorPicker;
+  let swatch = getRuleViewProperty(view, "#testid", "color").valueSpan
+      .querySelector(".ruleview-colorswatch");
+  let onShown = cPicker.tooltip.once("shown");
+  swatch.click();
+  yield onShown;
+
+  let testNode = yield getNode("#testid");
+
+  yield simulateColorPickerChange(view, cPicker, [0, 255, 0, 1], {
+    element: testNode,
+    name: "color",
+    value: "rgb(0, 255, 0)"
+  });
+
+  let spectrum = yield cPicker.spectrum;
+  let onHidden = cPicker.tooltip.once("hidden");
+  EventUtils.sendKey("RETURN", spectrum.element.ownerDocument.defaultView);
+  yield onHidden;
+
+  is(getRuleViewPropertyValue(view, "#testid", "color"), result,
+     "changing the color used the " + name + " unit");
+}
--- a/devtools/shared/css-color.js
+++ b/devtools/shared/css-color.js
@@ -87,16 +87,30 @@ CssColor.prototype = {
     }
     return this._colorUnit;
   },
 
   set colorUnit(unit) {
     this._colorUnit = unit;
   },
 
+  /**
+   * If the current color unit pref is "authored", then set the
+   * default color unit from the given color.  Otherwise, leave the
+   * 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);
+    }
+  },
+
   get hasAlpha() {
     if (!this.valid) {
       return false;
     }
     return this._getRGBATuple().a !== 1;
   },
 
   get valid() {
--- a/devtools/shared/tests/unit/test_cssColor.js
+++ b/devtools/shared/tests/unit/test_cssColor.js
@@ -20,10 +20,15 @@ const CLASSIFY_TESTS = [
   { input: "blue", output: "name" },
   { input: "orange", output: "name" }
 ];
 
 function run_test() {
   for (let test of CLASSIFY_TESTS) {
     let result = colorUtils.classifyColor(test.input);
     equal(result, test.output, "test classifyColor(" + test.input + ")");
+
+    let obj = new colorUtils.CssColor("purple");
+    obj.setAuthoredUnitFromColor(test.input);
+    equal(obj.colorUnit, test.output,
+          "test setAuthoredUnitFromColor(" + test.input + ")");
   }
 }