Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker
authorNicolas Chevobbe <chevobbe.nicolas@gmail.com>
Mon, 28 Mar 2016 21:01:32 +0200
changeset 291354 6d91363a7eac7343ded2da66d19b3ca01b6d0776
parent 291353 712a70746d183ee33e00bc06efd48de25b7332a7
child 291355 ad16491227f9cdf08fda7a3c1973562497d0f4ef
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1259559
milestone48.0a1
Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker Add an 'unit-change' event fired when shift+click on color and angle swatches. Add a listener on this event in text-property-editor.js to call the same function that's called when tooltip edit is commited to persist the new unit. Edit some tests to adapt to this new behaviour and create some tests to make sure the value obtained via shift+click are actually persisted. MozReview-Commit-ID: CcF4oiBPEzT
devtools/client/inspector/computed/test/browser_computed_cycle_color.js
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_cycle-angle.js
devtools/client/inspector/rules/test/browser_rules_cycle-color.js
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/shared/output-parser.js
devtools/client/shared/test/browser_css_angle.js
devtools/shared/css-angle.js
devtools/shared/css-color.js
--- a/devtools/client/inspector/computed/test/browser_computed_cycle_color.js
+++ b/devtools/client/inspector/computed/test/browser_computed_cycle_color.js
@@ -21,49 +21,50 @@ add_task(function*() {
   yield selectNode("#matches", inspector);
 
   info("Checking the property itself");
   let container = getComputedViewPropertyView(view, "color").valueNode;
   checkColorCycling(container, view);
 
   info("Checking matched selectors");
   container = yield getComputedViewMatchedRules(view, "color");
-  checkColorCycling(container, view);
+  yield checkColorCycling(container, view);
 });
 
-function checkColorCycling(container, view) {
-  let swatch = container.querySelector(".computedview-colorswatch");
+function* checkColorCycling(container, view) {
   let valueNode = container.querySelector(".computedview-color");
   let win = view.styleWindow;
 
   // "Authored" (default; currently the computed value)
   is(valueNode.textContent, "rgb(255, 0, 0)",
                             "Color displayed as an RGB value.");
 
-  // Hex
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "#f00", "Color displayed as a hex value.");
-
-  // HSL
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "hsl(0, 100%, 50%)",
-                            "Color displayed as an HSL value.");
+  let tests = [{
+    value: "red",
+    comment: "Color displayed as a color name."
+  }, {
+    value: "#f00",
+    comment: "Color displayed as an authored value."
+  }, {
+    value: "hsl(0, 100%, 50%)",
+    comment: "Color displayed as an HSL value again."
+  }, {
+    value: "rgb(255, 0, 0)",
+    comment: "Color displayed as an RGB value again."
+  }];
 
-  // RGB
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "rgb(255, 0, 0)",
-                            "Color displayed as an RGB value.");
+  for (let test of tests) {
+    yield checkSwatchShiftClick(container, win, test.value, test.comment);
+  }
+}
+
+function* checkSwatchShiftClick(container, win, expectedValue, comment) {
+  let swatch = container.querySelector(".computedview-colorswatch");
+  let valueNode = container.querySelector(".computedview-color");
 
-  // Color name
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "red",
-                            "Color displayed as a color name.");
-
-  // Back to "Authored"
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "rgb(255, 0, 0)",
-                            "Color displayed as an RGB value.");
+  let onUnitChange = swatch.once("unit-change");
+  EventUtils.synthesizeMouseAtCenter(swatch, {
+    type: "mousedown",
+    shiftKey: true
+  }, win);
+  yield onUnitChange;
+  is(valueNode.textContent, expectedValue, comment);
 }
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -77,16 +77,17 @@ skip-if = e10s && debug # Bug 1250058 - 
 [browser_rules_context-menu-show-mdn-docs-02.js]
 [browser_rules_context-menu-show-mdn-docs-03.js]
 [browser_rules_copy_styles.js]
 [browser_rules_cssom.js]
 [browser_rules_cubicbezier-appears-on-swatch-click.js]
 [browser_rules_cubicbezier-commit-on-ENTER.js]
 [browser_rules_cubicbezier-revert-on-ESC.js]
 [browser_rules_custom.js]
+[browser_rules_cycle-angle.js]
 [browser_rules_cycle-color.js]
 [browser_rules_edit-property-cancel.js]
 [browser_rules_edit-property-click.js]
 [browser_rules_edit-property-commit.js]
 [browser_rules_edit-property-computed.js]
 [browser_rules_edit-property-increments.js]
 [browser_rules_edit-property-order.js]
 [browser_rules_edit-property-remove_01.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_cycle-angle.js
@@ -0,0 +1,93 @@
+/* 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 cycling angle units in the rule view.
+
+const TEST_URI = `
+  <style type="text/css">
+    body {
+      image-orientation: 1turn;
+    }
+    div {
+      image-orientation: 180deg;
+    }
+  </style>
+  <body><div>Test</div>cycling angle units in the rule view!</body>
+`;
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  let container = getRuleViewProperty(
+    view, "body", "image-orientation").valueSpan;
+  yield checkAngleCycling(container, view);
+  yield checkAngleCyclingPersist(inspector, view);
+});
+
+function* checkAngleCycling(container, view) {
+  let valueNode = container.querySelector(".ruleview-angle");
+  let win = view.styleWindow;
+
+  // turn
+  is(valueNode.textContent, "1turn", "Angle displayed as a turn value.");
+
+  let tests = [{
+    value: "360deg",
+    comment: "Angle displayed as a degree value."
+  }, {
+    value: `${Math.round(Math.PI * 2 * 10000) / 10000}rad`,
+    comment: "Angle displayed as a radian value."
+  }, {
+    value: "400grad",
+    comment: "Angle displayed as a gradian value."
+  }, {
+    value: "1turn",
+    comment: "Angle displayed as a turn value again."
+  }];
+
+  for (let test of tests) {
+    yield checkSwatchShiftClick(container, win, test.value, test.comment);
+  }
+}
+
+function* checkAngleCyclingPersist(inspector, view) {
+  yield selectNode("div", inspector);
+  let container = getRuleViewProperty(
+    view, "div", "image-orientation").valueSpan;
+  let valueNode = container.querySelector(".ruleview-angle");
+  let win = view.styleWindow;
+
+  is(valueNode.textContent, "180deg", "Angle displayed as a degree value.");
+
+  yield checkSwatchShiftClick(container, win,
+    `${Math.round(Math.PI * 10000) / 10000}rad`,
+    "Angle displayed as a radian value.");
+
+  // Select the body and reselect the div to see
+  // if the new angle unit persisted
+  yield selectNode("body", inspector);
+  yield selectNode("div", inspector);
+
+  // We have to query for the container and the swatch because
+  // they've been re-generated
+  container = getRuleViewProperty(view, "div", "image-orientation").valueSpan;
+  valueNode = container.querySelector(".ruleview-angle");
+  is(valueNode.textContent, `${Math.round(Math.PI * 10000) / 10000}rad`,
+    "Angle still displayed as a radian value.");
+}
+
+function* checkSwatchShiftClick(container, win, expectedValue, comment) {
+  let swatch = container.querySelector(".ruleview-angleswatch");
+  let valueNode = container.querySelector(".ruleview-angle");
+
+  let onUnitChange = swatch.once("unit-change");
+  EventUtils.synthesizeMouseAtCenter(swatch, {
+    type: "mousedown",
+    shiftKey: true
+  }, win);
+  yield onUnitChange;
+  is(valueNode.textContent, expectedValue, comment);
+}
--- a/devtools/client/inspector/rules/test/browser_rules_cycle-color.js
+++ b/devtools/client/inspector/rules/test/browser_rules_cycle-color.js
@@ -6,58 +6,88 @@
 
 // Test cycling color types in the rule view.
 
 const TEST_URI = `
   <style type="text/css">
     body {
       color: #f00;
     }
+    span {
+      color: blue;
+    }
   </style>
-  Test cycling color types in the rule view!
+  <body><span>Test</span> cycling color types in the rule view!</body>
 `;
 
 add_task(function*() {
   yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
-  let {view} = yield openRuleView();
+  let {inspector, view} = yield openRuleView();
   let container = getRuleViewProperty(view, "body", "color").valueSpan;
-  checkColorCycling(container, view);
+  yield checkColorCycling(container, view);
+  yield checkColorCyclingPersist(inspector, view);
 });
 
-function checkColorCycling(container, view) {
-  let swatch = container.querySelector(".ruleview-colorswatch");
+function* checkColorCycling(container, view) {
   let valueNode = container.querySelector(".ruleview-color");
   let win = view.styleWindow;
 
   // Hex
   is(valueNode.textContent, "#f00", "Color displayed as a hex value.");
 
-  // HSL
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "hsl(0, 100%, 50%)",
-                            "Color displayed as an HSL value.");
+  let tests = [{
+    value: "hsl(0, 100%, 50%)",
+    comment: "Color displayed as an HSL value."
+  }, {
+    value: "rgb(255, 0, 0)",
+    comment: "Color displayed as an RGB value."
+  }, {
+    value: "red",
+    comment: "Color displayed as a color name."
+  }, {
+    value: "#f00",
+    comment: "Color displayed as an authored value."
+  }, {
+    value: "hsl(0, 100%, 50%)",
+    comment: "Color displayed as an HSL value again."
+  }];
 
-  // RGB
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "rgb(255, 0, 0)",
-                            "Color displayed as an RGB value.");
+  for (let test of tests) {
+    yield checkSwatchShiftClick(container, win, test.value, test.comment);
+  }
+}
+
+function* checkColorCyclingPersist(inspector, view) {
+  yield selectNode("span", inspector);
+  let container = getRuleViewProperty(view, "span", "color").valueSpan;
+  let valueNode = container.querySelector(".ruleview-color");
+  let win = view.styleWindow;
 
-  // Color name
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "red",
-                            "Color displayed as a color name.");
+  is(valueNode.textContent, "blue", "Color displayed as a color name.");
+
+  yield checkSwatchShiftClick(container, win, "#00f",
+    "Color displayed as a hex value.");
+
+  // Select the body and reselect the span to see
+  // if the new color unit persisted
+  yield selectNode("body", inspector);
+  yield selectNode("span", inspector);
 
-  // "Authored"
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "#f00",
-                            "Color displayed as an authored value.");
+  // We have to query for the container and the swatch because
+  // they've been re-generated
+  container = getRuleViewProperty(view, "span", "color").valueSpan;
+  valueNode = container.querySelector(".ruleview-color");
+  is(valueNode.textContent, "#00f",
+    "Color  is still displayed as a hex value.");
+}
 
-  // One more click skips hex, because it is the same as authored, and
-  // instead goes back to HSL.
-  EventUtils.synthesizeMouseAtCenter(swatch,
-                                     {type: "mousedown", shiftKey: true}, win);
-  is(valueNode.textContent, "hsl(0, 100%, 50%)",
-                            "Color displayed as an HSL value again.");
+function* checkSwatchShiftClick(container, win, expectedValue, comment) {
+  let swatch = container.querySelector(".ruleview-colorswatch");
+  let valueNode = container.querySelector(".ruleview-color");
+
+  let onUnitChange = swatch.once("unit-change");
+  EventUtils.synthesizeMouseAtCenter(swatch, {
+    type: "mousedown",
+    shiftKey: true
+  }, win);
+  yield onUnitChange;
+  is(valueNode.textContent, expectedValue, comment);
 }
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -365,16 +365,17 @@ TextPropertyEditor.prototype = {
         // Adding this swatch to the list of swatches our colorpicker
         // knows about
         this.ruleView.tooltips.colorPicker.addSwatch(span, {
           onShow: this._onStartEditing,
           onPreview: this._onSwatchPreview,
           onCommit: this._onSwatchCommit,
           onRevert: this._onSwatchRevert
         });
+        span.on("unit-change", this._onSwatchCommit);
       }
     }
 
     // Attach the cubic-bezier tooltip to the bezier swatches
     this._bezierSwatchSpans =
       this.valueSpan.querySelectorAll("." + bezierSwatchClass);
     if (this.ruleEditor.isEditable) {
       for (let span of this._bezierSwatchSpans) {
@@ -399,16 +400,24 @@ TextPropertyEditor.prototype = {
           onShow: this._onStartEditing,
           onPreview: this._onSwatchPreview,
           onCommit: this._onSwatchCommit,
           onRevert: this._onSwatchRevert
         }, outputParser, parserOptions);
       }
     }
 
+    this.angleSwatchSpans =
+      this.valueSpan.querySelectorAll("." + angleSwatchClass);
+    if (this.ruleEditor.isEditable) {
+      for (let angleSpan of this.angleSwatchSpans) {
+        angleSpan.on("unit-change", this._onSwatchCommit);
+      }
+    }
+
     // Populate the computed styles.
     this._updateComputed();
 
     // Update the rule property highlight.
     this.ruleView._updatePropertyHighlight(this);
   },
 
   _onStartEditing: function() {
@@ -606,16 +615,23 @@ TextPropertyEditor.prototype = {
    *
    * @param {Number} direction
    *        The move focus direction number.
    */
   remove: function(direction) {
     if (this._colorSwatchSpans && this._colorSwatchSpans.length) {
       for (let span of this._colorSwatchSpans) {
         this.ruleView.tooltips.colorPicker.removeSwatch(span);
+        span.off("unit-change", this._onSwatchCommit);
+      }
+    }
+
+    if (this.angleSwatchSpans && this.angleSwatchSpans.length) {
+      for (let span of this.angleSwatchSpans) {
+        span.off("unit-change", this._onSwatchCommit);
       }
     }
 
     this.element.parentNode.removeChild(this.element);
     this.ruleEditor.rule.editClosestTextProperty(this.prop, direction);
     this.nameSpan.textProperty = null;
     this.valueSpan.textProperty = null;
     this.prop.remove();
--- a/devtools/client/shared/output-parser.js
+++ b/devtools/client/shared/output-parser.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {Cc, Ci, Cu} = require("chrome");
 const {angleUtils} = require("devtools/shared/css-angle");
 const {colorUtils} = require("devtools/shared/css-color");
 const Services = require("Services");
+const EventEmitter = require("devtools/shared/event-emitter");
 
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 
 const BEZIER_KEYWORDS = ["linear", "ease-in-out", "ease-in", "ease-out",
                          "ease"];
 
 // Functions that accept a color argument.
 const COLOR_TAKING_FUNCTIONS = ["linear-gradient",
@@ -334,16 +335,17 @@ OutputParser.prototype = {
       // in order to prevent the value input to be focused.
       // Bug 711942 will add a tooltip to edit angle values and we should
       // be able to move this listener to Tooltip.js when it'll be implemented.
       swatch.addEventListener("click", function(event) {
         if (event.shiftKey) {
           event.stopPropagation();
         }
       }, false);
+      EventEmitter.decorate(swatch);
       container.appendChild(swatch);
     }
 
     let value = this._createNode("span", {
       class: options.angleClass
     }, angle);
 
     container.appendChild(value);
@@ -391,16 +393,17 @@ OutputParser.prototype = {
 
       if (options.colorSwatchClass) {
         let swatch = this._createNode("span", {
           class: options.colorSwatchClass,
           style: "background-color:" + color
         });
         this.colorSwatches.set(swatch, colorObj);
         swatch.addEventListener("mousedown", this._onColorSwatchMouseDown, false);
+        EventEmitter.decorate(swatch);
         container.appendChild(swatch);
       }
 
       if (options.defaultColorType) {
         color = colorObj.toString();
         container.dataset.colorĀ = color;
       }
 
@@ -457,31 +460,33 @@ OutputParser.prototype = {
       return;
     }
 
     let swatch = event.target;
     let color = this.colorSwatches.get(swatch);
     let val = color.nextColorUnit();
 
     swatch.nextElementSibling.textContent = val;
+    swatch.emit("unit-change", val);
   },
 
   _onAngleSwatchMouseDown: function(event) {
     // Prevent text selection in the case of shift-click or double-click.
     event.preventDefault();
 
     if (!event.shiftKey) {
       return;
     }
 
     let swatch = event.target;
     let angle = this.angleSwatches.get(swatch);
     let val = angle.nextAngleUnit();
 
     swatch.nextElementSibling.textContent = val;
+    swatch.emit("unit-change", val);
   },
 
   /**
    * A helper function that sanitizes a possibly-unterminated URL.
    */
   _sanitizeURL: function(url) {
     // Re-lex the URL and add any needed termination characters.
     let urlTokenizer = DOMUtils.getCSSLexer(url);
--- a/devtools/client/shared/test/browser_css_angle.js
+++ b/devtools/client/shared/test/browser_css_angle.js
@@ -54,23 +54,23 @@ function getTestData() {
     authored: "0deg",
     deg: "0deg",
     rad: "0rad",
     grad: "0grad",
     turn: "0turn"
   }, {
     authored: "180deg",
     deg: "180deg",
-    rad: "3.14rad",
+    rad: `${Math.round(Math.PI * 10000) / 10000}rad`,
     grad: "200grad",
     turn: "0.5turn"
   }, {
     authored: "180DEG",
     deg: "180DEG",
-    rad: "3.14RAD",
+    rad: `${Math.round(Math.PI * 10000) / 10000}RAD`,
     grad: "200GRAD",
     turn: "0.5TURN"
   }, {
     authored: `-${Math.PI}rad`,
     deg: "-180deg",
     rad: `-${Math.PI}rad`,
     grad: "-200grad",
     turn: "-0.5turn"
@@ -78,35 +78,35 @@ function getTestData() {
     authored: `-${Math.PI}RAD`,
     deg: "-180DEG",
     rad: `-${Math.PI}RAD`,
     grad: "-200GRAD",
     turn: "-0.5TURN"
   }, {
     authored: "100grad",
     deg: "90deg",
-    rad: "1.57rad",
+    rad: `${Math.round(Math.PI / 2 * 10000) / 10000}rad`,
     grad: "100grad",
     turn: "0.25turn"
   }, {
     authored: "100GRAD",
     deg: "90DEG",
-    rad: "1.57RAD",
+    rad: `${Math.round(Math.PI / 2 * 10000) / 10000}RAD`,
     grad: "100GRAD",
     turn: "0.25TURN"
   }, {
     authored: "-1turn",
     deg: "-360deg",
-    rad: "-6.28rad",
+    rad: `${-1 * Math.round(Math.PI * 2 * 10000) / 10000}rad`,
     grad: "-400grad",
     turn: "-1turn"
   }, {
     authored: "-10TURN",
     deg: "-3600DEG",
-    rad: "-62.83RAD",
+    rad: `${-1 * Math.round(Math.PI * 2 * 10 * 10000) / 10000}RAD`,
     grad: "-4000GRAD",
     turn: "-10TURN"
   }, {
     authored: "inherit",
     deg: "inherit",
     rad: "inherit",
     grad: "inherit",
     turn: "inherit"
--- a/devtools/shared/css-angle.js
+++ b/devtools/shared/css-angle.js
@@ -137,17 +137,17 @@ CssAngle.prototype = {
       // The angle is valid and is in turn.
       radValue = this.authoredAngleValue * 360 * (Math.PI / 180);
     }
 
     let unitStr = CssAngle.ANGLEUNIT.rad;
     if (this._angleUnitUppercase === true) {
       unitStr = unitStr.toUpperCase();
     }
-    return `${Math.round(radValue * 100) / 100}${unitStr}`;
+    return `${Math.round(radValue * 10000) / 10000}${unitStr}`;
   },
 
   get grad() {
     let invalidOrSpecialValue = this._getInvalidOrSpecialValue();
     if (invalidOrSpecialValue !== false) {
       return invalidOrSpecialValue;
     }
 
--- a/devtools/shared/css-color.js
+++ b/devtools/shared/css-color.js
@@ -284,18 +284,19 @@ CssColor.prototype = {
     this.lowerCased = color.toLowerCase();
     this.authored = color;
     return this;
   },
 
   nextColorUnit: function() {
     // Reorder the formats array to have the current format at the
     // front so we can cycle through.
-    let formats = ["authored", "hex", "hsl", "rgb", "name"];
-    let putOnEnd = formats.splice(0, formats.indexOf(this.colorUnit));
+    let formats = ["hex", "hsl", "rgb", "name"];
+    let currentFormat = classifyColor(this.toString());
+    let putOnEnd = formats.splice(0, formats.indexOf(currentFormat));
     formats = formats.concat(putOnEnd);
     let currentDisplayedColor = this[formats[0]];
 
     for (let format of formats) {
       if (this[format].toLowerCase() !== currentDisplayedColor.toLowerCase()) {
         this.colorUnit = CssColor.COLORUNIT[format];
         break;
       }