Bug 940500 - Colorpicker tooltip now edits css gradients correctly. r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 03 Dec 2013 08:45:29 -0500
changeset 173151 bac5ac014c63754ff362d96905091337524aefeb
parent 173150 78e12d1f3aa23128910a5a32e69808f3a669a126
child 173152 51cb8e90b8194e13a8736972d5b156633633f6ed
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs940500
milestone28.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 940500 - Colorpicker tooltip now edits css gradients correctly. r=miker
browser/devtools/shared/test/browser_outputparser.js
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser.ini
browser/devtools/styleinspector/test/browser_bug940500_rule_view_pick_gradient_color.js
toolkit/devtools/output-parser.js
--- a/browser/devtools/shared/test/browser_outputparser.js
+++ b/browser/devtools/shared/test/browser_outputparser.js
@@ -32,36 +32,49 @@ function testParseCssProperty() {
     colorSwatchClass: "test-colorswatch"
   });
 
   let target = doc.querySelector("div");
   ok(target, "captain, we have the div");
   target.appendChild(frag);
 
   is(target.innerHTML,
-     '1px solid <span style="background-color:red" class="test-colorswatch"></span>#F00',
+     '1px solid <span style="background-color:red" class="test-colorswatch"></span><span>#F00</span>',
      "CSS property correctly parsed");
 
   target.innerHTML = "";
 
+  let frag = parser.parseCssProperty("background-image", "linear-gradient(to right, #F60 10%, rgba(0,0,0,1))", {
+    colorSwatchClass: "test-colorswatch",
+    colorClass: "test-color"
+  });
+  target.appendChild(frag);
+  is(target.innerHTML,
+     'linear-gradient(to right, <span style="background-color:#F60" class="test-colorswatch"></span><span class="test-color">#F60</span> 10%, ' +
+     '<span style="background-color:rgba(0,0,0,1)" class="test-colorswatch"></span><span class="test-color">#000</span>)',
+     "Gradient CSS property correctly parsed");
+
+  target.innerHTML = "";
+
   testParseHTMLAttribute();
 }
 
 function testParseHTMLAttribute() {
   let attrib = "color:red; font-size: 12px; background-image: " +
                "url(chrome://branding/content/about-logo.png)";
   let frag = parser.parseHTMLAttribute(attrib, {
-    urlClass: "theme-link"
+    urlClass: "theme-link",
+    colorClass: "theme-color"
   });
 
   let target = doc.querySelector("div");
   ok(target, "captain, we have the div");
   target.appendChild(frag);
 
-  let expected = 'color:#F00; font-size: 12px; ' +
+  let expected = 'color:<span class="theme-color">#F00</span>; font-size: 12px; ' +
                  'background-image: url(\'<a href="chrome://branding/content/about-logo.png" ' +
                  'class="theme-link" ' +
                  'target="_blank">chrome://branding/content/about-logo.png</a>\')';
 
   is(target.innerHTML, expected, "HTML Attribute correctly parsed");
   target.innerHTML = "";
   testParseNonCssHTMLAttribute();
 }
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -2018,16 +2018,17 @@ TextPropertyEditor.prototype = {
     } else {
       this.element.removeAttribute("dirty");
     }
 
     let swatchClass = "ruleview-colorswatch";
     let outputParser = this.ruleEditor.ruleView._outputParser;
     let frag = outputParser.parseCssProperty(name, val, {
       colorSwatchClass: swatchClass,
+      colorClass: "ruleview-color",
       defaultColorType: !propDirty,
       urlClass: "theme-link",
       baseURI: this.sheetURI
     });
     this.valueSpan.innerHTML = "";
     this.valueSpan.appendChild(frag);
 
     // Attach the color picker tooltip to the color swatches
--- a/browser/devtools/styleinspector/test/browser.ini
+++ b/browser/devtools/styleinspector/test/browser.ini
@@ -48,8 +48,9 @@ support-files =
 [browser_bug894376_css_value_completion_existing_property_value_pair.js]
 [browser_ruleview_bug_902966_revert_value_on_ESC.js]
 [browser_ruleview_pseudoelement.js]
 support-files = browser_ruleview_pseudoelement.html
 [browser_computedview_bug835808_keyboard_nav.js]
 [browser_bug913014_matched_expand.js]
 [browser_bug765105_background_image_tooltip.js]
 [browser_bug889638_rule_view_color_picker.js]
+[browser_bug940500_rule_view_pick_gradient_color.js]
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleinspector/test/browser_bug940500_rule_view_pick_gradient_color.js
@@ -0,0 +1,135 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that changing a color in a gradient css declaration using the tooltip
+// color picker works
+
+let {gDevTools} = Cu.import("resource:///modules/devtools/gDevTools.jsm", {});
+
+let contentDoc;
+let contentWin;
+let inspector;
+let ruleView;
+
+const PAGE_CONTENT = [
+  '<style type="text/css">',
+  '  body {',
+  '    background-image: linear-gradient(to left, #f06 25%, #333 95%, #000 100%);',
+  '  }',
+  '</style>',
+  'Updating a gradient declaration with the color picker tooltip'
+].join("\n");
+
+function test() {
+  waitForExplicitFinish();
+
+  gBrowser.selectedTab = gBrowser.addTab();
+  gBrowser.selectedBrowser.addEventListener("load", function(evt) {
+    gBrowser.selectedBrowser.removeEventListener(evt.type, arguments.callee, true);
+    contentDoc = content.document;
+    contentWin = contentDoc.defaultView;
+    waitForFocus(createDocument, content);
+  }, true);
+
+  content.location = "data:text/html,rule view color picker tooltip test";
+}
+
+function createDocument() {
+  contentDoc.body.innerHTML = PAGE_CONTENT;
+
+  openRuleView((aInspector, aRuleView) => {
+    inspector = aInspector;
+    ruleView = aRuleView;
+    startTests();
+  });
+}
+
+function startTests() {
+  inspector.selection.setNode(contentDoc.body);
+  inspector.once("inspector-updated", testColorParsing);
+}
+
+function endTests() {
+  executeSoon(function() {
+    gDevTools.once("toolbox-destroyed", () => {
+      contentDoc = contentWin = inspector = ruleView = null;
+      gBrowser.removeCurrentTab();
+      finish();
+    });
+    inspector._toolbox.destroy();
+  });
+}
+
+function testColorParsing() {
+  let ruleEl = getRuleViewProperty("background-image");
+  ok(ruleEl, "The background-image gradient declaration was found");
+
+  let swatchEls = ruleEl.valueSpan.querySelectorAll(".ruleview-colorswatch");
+  ok(swatchEls, "The color swatch elements were found");
+  is(swatchEls.length, 3, "There are 3 color swatches");
+
+  let colorEls = ruleEl.valueSpan.querySelectorAll(".ruleview-color");
+  ok(colorEls, "The color elements were found");
+  is(colorEls.length, 3, "There are 3 color values");
+
+  let colors = ["#F06", "#333", "#000"];
+  for (let i = 0; i < colors.length; i ++) {
+    is(colorEls[i].textContent, colors[i], "The right color value was found");
+  }
+
+  testPickingNewColor();
+}
+
+function testPickingNewColor() {
+  // Grab the first color swatch and color in the gradient
+  let ruleEl = getRuleViewProperty("background-image");
+  let swatchEl = ruleEl.valueSpan.querySelector(".ruleview-colorswatch");
+  let colorEl = ruleEl.valueSpan.querySelector(".ruleview-color");
+
+  // Get the color picker tooltip
+  let cPicker = ruleView.colorPicker;
+
+  cPicker.tooltip.once("shown", () => {
+    simulateColorChange(cPicker, [1, 1, 1, 1]);
+
+    executeSoon(() => {
+      is(swatchEl.style.backgroundColor, "rgb(1, 1, 1)",
+        "The color swatch's background was updated");
+      is(colorEl.textContent, "rgba(1, 1, 1, 1)",
+        "The color text was updated");
+      is(content.getComputedStyle(content.document.body).backgroundImage,
+        "linear-gradient(to left, rgb(255, 0, 102) 25%, rgb(51, 51, 51) 95%, rgb(0, 0, 0) 100%)",
+        "The gradient has been updated correctly");
+
+      cPicker.hide();
+      endTests();
+    });
+  });
+  swatchEl.click();
+}
+
+function simulateColorChange(colorPicker, newRgba) {
+  // Note that this test isn't concerned with simulating events to test how the
+  // spectrum color picker reacts, see browser_spectrum.js for this.
+  // This test only cares about the color swatch <-> color picker <-> rule view
+  // interactions. That's why there's no event simulations here
+  colorPicker.spectrum.then(spectrum => {
+    spectrum.rgb = newRgba;
+    spectrum.updateUI();
+    spectrum.onChange();
+  });
+}
+
+function getRuleViewProperty(name) {
+  let prop = null;
+  [].forEach.call(ruleView.doc.querySelectorAll(".ruleview-property"), property => {
+    let nameSpan = property.querySelector(".ruleview-propertyname");
+    let valueSpan = property.querySelector(".ruleview-propertyvalue");
+
+    if (nameSpan.textContent === name) {
+      prop = {nameSpan: nameSpan, valueSpan: valueSpan};
+    }
+  });
+  return prop;
+}
--- a/toolkit/devtools/output-parser.js
+++ b/toolkit/devtools/output-parser.js
@@ -299,17 +299,19 @@ OutputParser.prototype = {
         this._appendNode("span", {
           class: options.colorSwatchClass,
           style: "background-color:" + color
         });
       }
       if (options.defaultColorType) {
         color = colorObj.toString();
       }
-      this._appendTextNode(color);
+      this._appendNode("span", {
+        class: options.colorClass
+      }, color);
       return true;
     }
     return false;
   },
 
    /**
     * Append a URL to the output.
     *
@@ -357,17 +359,19 @@ OutputParser.prototype = {
    */
   _appendNode: function(tagName, attributes, value="") {
     let win = Services.appShell.hiddenDOMWindow;
     let doc = win.document;
     let node = doc.createElementNS(HTML_NS, tagName);
     let attrs = Object.getOwnPropertyNames(attributes);
 
     for (let attr of attrs) {
-      node.setAttribute(attr, attributes[attr]);
+      if (attributes[attr]) {
+        node.setAttribute(attr, attributes[attr]);
+      }
     }
 
     if (value) {
       let textNode = doc.createTextNode(value);
       node.appendChild(textNode);
     }
 
     this.parsed.push(node);
@@ -417,16 +421,18 @@ OutputParser.prototype = {
    *
    * @param  {Object} overrides
    *         The option values to override e.g. _mergeOptions({colors: false})
    *
    *         Valid options are:
    *           - defaultColorType: true // Convert colors to the default type
    *                                    // selected in the options panel.
    *           - colorSwatchClass: ""   // The class to use for color swatches.
+   *           - colorClass: ""         // The class to use for the color value
+   *                                    // that follows the swatch.
    *           - isHTMLAttribute: false // This property indicates whether we
    *                                    // are parsing an HTML attribute value.
    *                                    // When the value is passed in from an
    *                                    // HTML attribute we need to check that
    *                                    // any CSS property values are supported
    *                                    // by the property name before
    *                                    // processing the property value.
    *           - urlClass: ""           // The class to be used for url() links.
@@ -434,16 +440,17 @@ OutputParser.prototype = {
    *                                    // relative links.
    * @return {Object}
    *         Overridden options object
    */
   _mergeOptions: function(overrides) {
     let defaults = {
       defaultColorType: true,
       colorSwatchClass: "",
+      colorClass: "",
       isHTMLAttribute: false,
       urlClass: "",
       baseURI: ""
     };
 
     if (typeof overrides.baseURI === "string") {
       overrides.baseURI = Services.io.newURI(overrides.baseURI, null, null);
     }