Bug 1275939 - replace domUtils.getCSSValuesForProperty in inplace-editor.js r=tromey

MozReview-Commit-ID: KtuAWnNsnaq

--HG--
rename : devtools/server/tests/mochitest/test_css-properties.html => devtools/server/tests/mochitest/test_css-properties_01.html
extra : rebase_source : 3002ac705c2a99fa04796666aa40261abdc87646

devtools/client/inspector/layout/layout.js

@@ -199,6 +199,7 @@ function LayoutView(inspector, document) {
   this.sizeLabel = this.doc.querySelector(".layout-size > span");
   this.sizeHeadingLabel = this.doc.getElementById("layout-element-size");
   this._geometryEditorHighlighter = null;
+  this._cssProperties = getCssProperties(inspector.toolbox);
 
   this.init();
 }
@@ -419,7 +420,8 @@ LayoutView.prototype = {
             session.destroy();
           }, e => console.error(e));
         }
-      }
+      },
+      cssProperties: this._cssProperties
     }, event);
   },
 

devtools/client/inspector/markup/markup.js

@@ -59,6 +59,7 @@ const nodeFilterConstants = require("devtools/shared/dom-node-filter-constants")
 /* eslint-disable mozilla/reject-some-requires */
 const {XPCOMUtils} = require("resource://gre/modules/XPCOMUtils.jsm");
 /* eslint-enable mozilla/reject-some-requires */
+const {getCssProperties} = require("devtools/shared/fronts/css-properties");
 
 loader.lazyRequireGetter(this, "CSS", "CSS");
 loader.lazyGetter(this, "AutocompletePopup", () => {
@@ -2927,7 +2928,8 @@ function TextEditor(container, node, templateId) {
           });
         });
       });
-    }
+    },
+    cssProperties: getCssProperties(this.markup._inspector.toolbox)
   });
 
   this.update();
@@ -2981,6 +2983,7 @@ function ElementEditor(container, node) {
   this.markup = this.container.markup;
   this.template = this.markup.template.bind(this.markup);
   this.doc = this.markup.doc;
+  this._cssProperties = getCssProperties(this.markup._inspector.toolbox);
 
   this.attrElements = new Map();
   this.animationTimers = {};
@@ -3006,6 +3009,7 @@ function ElementEditor(container, node) {
       trigger: "dblclick",
       stopOnReturn: true,
       done: this.onTagEdit.bind(this),
+      cssProperties: this._cssProperties
     });
   }
 
@@ -3029,7 +3033,8 @@ function ElementEditor(container, node) {
       }, function () {
         undoMods.apply();
       });
-    }
+    },
+    cssProperties: this._cssProperties
   });
 
   let displayName = this.node.displayName;
@@ -3267,7 +3272,8 @@ ElementEditor.prototype = {
         }, () => {
           undoMods.apply();
         });
-      }
+      },
+      cssProperties: this._cssProperties
     });
 
     // Figure out where we should place the attribute.

devtools/client/inspector/rules/rules.js

@@ -25,7 +25,7 @@ const {PrefObserver, PREF_ORIG_SOURCES} = require("devtools/client/styleeditor/u
 const {ElementStyle} = require("devtools/client/inspector/rules/models/element-style");
 const {Rule} = require("devtools/client/inspector/rules/models/rule");
 const {RuleEditor} = require("devtools/client/inspector/rules/views/rule-editor");
-const {createChild, promiseWarn} = require("devtools/client/inspector/shared/utils");
+const {createChild, promiseWarn, throttle} = require("devtools/client/inspector/shared/utils");
 const {gDevTools} = require("devtools/client/framework/devtools");
 const {getCssProperties} = require("devtools/shared/fronts/css-properties");
 
@@ -163,6 +163,9 @@ function CssRuleView(inspector, document, store, pageStyle) {
   this.store = store || {};
   this.pageStyle = pageStyle;
 
+  // Allow tests to override throttling behavior, as this can cause intermittents.
+  this.throttle = throttle;
+
   this.cssProperties = getCssProperties(inspector.toolbox);
 
   this._outputParser = new OutputParser(document, this.cssProperties.supportsType);

devtools/client/inspector/rules/test/browser_rules_multiple-properties-unfinished_02.js

@@ -12,6 +12,11 @@ const TEST_URI = "<div>Test Element</div>";
 add_task(function* () {
   yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
   let {inspector, view} = yield openRuleView();
+
+  // Turn off throttling, which can cause intermittents. Throttling is used by
+  // the TextPropertyEditor.
+  view.throttle = () => {};
+
   yield selectNode("div", inspector);
 
   let ruleEditor = getRuleViewRuleEditor(view, 0);

devtools/client/inspector/rules/views/rule-editor.js

@@ -149,6 +149,7 @@ RuleEditor.prototype = {
       editableField({
         element: this.selectorText,
         done: this._onSelectorDone,
+        cssProperties: this.rule.cssProperties
       });
     }
 
@@ -449,7 +450,8 @@ RuleEditor.prototype = {
       destroy: this._newPropertyDestroy,
       advanceChars: ":",
       contentType: InplaceEditor.CONTENT_TYPES.CSS_PROPERTY,
-      popup: this.ruleView.popup
+      popup: this.ruleView.popup,
+      cssProperties: this.rule.cssProperties
     });
 
     // Auto-close the input if multiple rules get pasted into new property.

devtools/client/inspector/rules/views/text-property-editor.js

@@ -12,8 +12,7 @@ const {
   createChild,
   appendText,
   advanceValidate,
-  blurOnMultipleProperties,
-  throttle
+  blurOnMultipleProperties
 } = require("devtools/client/inspector/shared/utils");
 const {
   parseDeclarations,
@@ -76,7 +75,7 @@ function TextPropertyEditor(ruleEditor, property) {
   this._onSwatchCommit = this._onSwatchCommit.bind(this);
   this._onSwatchPreview = this._onSwatchPreview.bind(this);
   this._onSwatchRevert = this._onSwatchRevert.bind(this);
-  this._onValidate = throttle(this._previewValue, 10, this);
+  this._onValidate = this.ruleView.throttle(this._previewValue, 10, this);
   this.update = this.update.bind(this);
   this.updatePropertyState = this.updatePropertyState.bind(this);
 
@@ -219,7 +218,8 @@ TextPropertyEditor.prototype = {
         destroy: this.updatePropertyState,
         advanceChars: ":",
         contentType: InplaceEditor.CONTENT_TYPES.CSS_PROPERTY,
-        popup: this.popup
+        popup: this.popup,
+        cssProperties: this.cssProperties
       });
 
       // Auto blur name field on multiple CSS rules get pasted in.
@@ -288,7 +288,8 @@ TextPropertyEditor.prototype = {
         property: this.prop,
         popup: this.popup,
         multiline: true,
-        maxWidth: () => this.container.getBoundingClientRect().width
+        maxWidth: () => this.container.getBoundingClientRect().width,
+        cssProperties: this.cssProperties
       });
     }
   },

devtools/client/shared/inplace-editor.js

@@ -120,6 +120,7 @@ function isKeyIn(key, ...keys) {
  *    {Boolean} preserveTextStyles: If true, do not copy text-related styles
  *              from `element` to the new input.
  *      defaults to false
+ *    {Object} cssProperties: An instance of CSSProperties.
  */
 function editableField(options) {
   return editableItem(options, function (element, event) {
@@ -219,7 +220,7 @@ function InplaceEditor(options, event) {
   let doc = this.elt.ownerDocument;
   this.doc = doc;
   this.elt.inplaceEditor = this;
-
+  this.cssProperties = options.cssProperties;
   this.change = options.change;
   this.done = options.done;
   this.destroy = options.destroy;
@@ -1477,7 +1478,7 @@ InplaceEditor.prototype = {
    * @return {Array} array of CSS property values (Strings)
    */
   _getCSSValuesForPropertyName: function (propertyName) {
-    return domUtils.getCSSValuesForProperty(propertyName);
+    return this.cssProperties.getValues(propertyName);
   },
 };
 

devtools/server/actors/css-properties.js

@@ -10,10 +10,15 @@ loader.lazyGetter(this, "DOMUtils", () => {
   return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
 });
 
+loader.lazyGetter(this, "appInfo", () => {
+  return Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULAppInfo);
+});
+
 const protocol = require("devtools/shared/protocol");
 const { ActorClassWithSpec, Actor } = protocol;
 const { cssPropertiesSpec } = require("devtools/shared/specs/css-properties");
 const { CSS_PROPERTIES, CSS_TYPES } = require("devtools/shared/css-properties-db");
+const { cssColors } = require("devtools/shared/css-color-db");
 
 exports.CssPropertiesActor = ActorClassWithSpec(cssPropertiesSpec, {
   typeName: "cssProperties",
@@ -27,7 +32,15 @@ exports.CssPropertiesActor = ActorClassWithSpec(cssPropertiesSpec, {
     Actor.prototype.destroy.call(this);
   },
 
-  getCSSDatabase() {
+  getCSSDatabase(clientBrowserVersion) {
+    // If the client and server are both the same version of Firefox, do not return a
+    // database, use the client-side css-properties-db.js.
+    const serverBrowserVersion = appInfo.platformVersion.match(/^\d+/)[0];
+
+    if (clientBrowserVersion !== 0 && clientBrowserVersion === serverBrowserVersion) {
+      return {};
+    }
+
     const properties = generateCssProperties();
     const pseudoElements = DOMUtils.getCSSPseudoElementNames();
 
@@ -44,6 +57,7 @@ exports.CssPropertiesActor = ActorClassWithSpec(cssPropertiesSpec, {
 function generateCssProperties() {
   const properties = {};
   const propertyNames = DOMUtils.getCSSPropertyNames(DOMUtils.INCLUDE_ALIASES);
+  const colors = Object.keys(cssColors);
 
   propertyNames.forEach(name => {
     // Get the list of CSS types this property supports.
@@ -54,12 +68,20 @@ function generateCssProperties() {
       }
     }
 
+    // Don't send colors over RDP, these will be re-attached by the front.
+    let values = DOMUtils.getCSSValuesForProperty(name);
+    if (values.includes("aliceblue")) {
+      values = values.filter(x => !colors.includes(x));
+      values.unshift("COLOR");
+    }
+
     // In order to maintain any backwards compatible changes when debugging older
     // clients, take the definition from the static CSS properties database, and fill it
     // in with the most recent property definition from the server.
     const clientDefinition = CSS_PROPERTIES[name] || {};
     const serverDefinition = {
       isInherited: DOMUtils.isInheritedProperty(name),
+      values,
       supports
     };
     properties[name] = Object.assign(clientDefinition, serverDefinition);

devtools/server/tests/mochitest/chrome.ini

@@ -32,7 +32,8 @@ skip-if = buildapp == 'mulet'
 [test_css-logic.html]
 [test_css-logic-media-queries.html]
 [test_css-logic-specificity.html]
-[test_css-properties.html]
+[test_css-properties_01.html]
+[test_css-properties_02.html]
 [test_Debugger.Source.prototype.introductionScript.html]
 [test_Debugger.Source.prototype.introductionType.html]
 [test_Debugger.Source.prototype.element.html]

devtools/server/tests/mochitest/test_css-properties_01.html

@@ -25,8 +25,12 @@ window.onload = function() {
     })
   }
 
+  function toSortedString(array) {
+    return JSON.stringify(array.sort());
+  }
+
   const runCssPropertiesTests = Task.async(function* (url, useActor) {
-    info("Opening two tabs.");
+    info(`Opening two tabs ${useActor ? "with" : "without"} CssPropertiesActor support.`);
 
     let attachmentA = yield promiseAttachUrl(url);
     let attachmentB = yield promiseAttachUrl(url);
@@ -73,6 +77,20 @@ window.onload = function() {
       "A CSS variable unicode properly evaluates.");
     ok(!cssProperties.isKnown("--foo bar"),
       "A CSS variable with spaces fails");
+
+    is(toSortedString(cssProperties.getValues('margin')),
+       toSortedString(["-moz-calc","auto","calc","inherit","initial","unset"]),
+       "Can get values for the CSS margin.");
+    is(cssProperties.getValues('foobar').length, 0,
+      "Unknown values return an empty array.");
+
+    const bgColorValues = cssProperties.getValues('background-color');
+    ok(bgColorValues.includes("blanchedalmond"),
+      "A property with color values includes blanchedalmond.");
+    ok(bgColorValues.includes("papayawhip"),
+      "A property with color values includes papayawhip.");
+    ok(bgColorValues.includes("rgb"),
+      "A property with color values includes non-colors.");
   });
 
   addAsyncTest(function* setup() {

devtools/server/tests/mochitest/test_css-properties_02.html

@@ -0,0 +1,98 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Bug 1265798 - Replace inIDOMUtils.cssPropertyIsShorthand
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test CSS Properties Actor</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  <script type="application/javascript;version=1.8" src="inspector-helpers.js"></script>
+  <script type="application/javascript;version=1.8">
+window.onload = function() {
+  const { initCssProperties, getCssProperties } =
+    require("devtools/shared/fronts/css-properties");
+
+  const { CSS_PROPERTIES_DB } = require("devtools/shared/css-properties-db");
+
+  function promiseAttachUrl (url) {
+    return new Promise((resolve, reject) => {
+      attachURL(url, function(err, client, tab, doc) {
+        if (err) {
+          return reject(err);
+        }
+        resolve({client, tab, doc});
+      });
+    })
+  }
+
+  addAsyncTest(function* setup() {
+    let url = document.getElementById("cssProperties").href;
+
+    let attachmentA = yield promiseAttachUrl(url);
+    let attachmentB = yield promiseAttachUrl(url);
+    let attachmentC = yield promiseAttachUrl(url);
+
+    const toolboxMatchingVersions = {
+      target: {
+        hasActor: () => true,
+        client: attachmentA.client,
+        form: attachmentA.tab,
+      },
+      _host: { frame: { contentWindow: window } }
+    };
+    const toolboxUnknownVersions = {
+      target: {
+        hasActor: () => true,
+        client: attachmentB.client,
+        form: attachmentB.tab
+      }
+      // Don't add a host here.
+    };
+    const toolboxDifferentVersions = {
+      target: {
+        hasActor: () => true,
+        client: attachmentB.client,
+        form: attachmentB.tab
+      },
+      _host: { frame: { contentWindow: { navigator: { userAgent:
+               "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:49.0) Gecko/20100101 " +
+               "Firefox/30.0" }}}}
+    };
+
+    // Modify a property on the static database, to differentiate between a generated
+    // and static CSS properties database.
+    CSS_PROPERTIES_DB.properties.color.isStatic = true;
+
+    yield initCssProperties(toolboxMatchingVersions);
+    yield initCssProperties(toolboxUnknownVersions);
+    yield initCssProperties(toolboxDifferentVersions);
+
+    const cssPropertiesMatching = getCssProperties(toolboxMatchingVersions);
+    const cssPropertiesUnknown = getCssProperties(toolboxUnknownVersions);
+    const cssPropertiesDifferent = getCssProperties(toolboxDifferentVersions);
+
+    is(cssPropertiesMatching.properties.color.isStatic, true,
+       "The static CSS database is used when the client and platform versions match.");
+    is(cssPropertiesUnknown.properties.color.isStatic, true,
+       "The static CSS database is used when the client is not a known Firefox.");
+    isnot(cssPropertiesDifferent.properties.color.isStatic, undefined,
+       "The generated CSS database is used when the client and platform versions do " +
+       "not match, but the client is a Firefox.");
+
+    delete CSS_PROPERTIES_DB.properties.color.isStatic;
+
+    runNextTest();
+  });
+
+  SimpleTest.waitForExplicitFinish();
+  runNextTest();
+}
+  </script>
+</head>
+<body>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1265798">Mozilla Bug 1265798</a>
+  <a id="cssProperties" target="_blank" href="inspector_css-properties.html">Test Document</a>
+</body>
+</html>

devtools/shared/fronts/css-properties.js

@@ -7,6 +7,7 @@ const { FrontClassWithSpec, Front } = require("devtools/shared/protocol");
 const { cssPropertiesSpec } = require("devtools/shared/specs/css-properties");
 const { Task } = require("devtools/shared/task");
 const { CSS_PROPERTIES_DB } = require("devtools/shared/css-properties-db");
+const { cssColors } = require("devtools/shared/css-color-db");
 
 /**
  * Build up a regular expression that matches a CSS variable token. This is an
@@ -45,8 +46,6 @@ const CssPropertiesFront = FrontClassWithSpec(cssPropertiesSpec, {
   }
 });
 
-exports.CssPropertiesFront = CssPropertiesFront;
-
 /**
  * Ask questions to a CSS database. This class does not care how the database
  * gets loaded in, only the questions that you can ask to it.
@@ -100,11 +99,19 @@ CssProperties.prototype = {
    */
   supportsType(property, type) {
     return this.properties[property] && this.properties[property].supports.includes(type);
+  },
+
+  /**
+   * Gets the CSS values for a given property name.
+   *
+   * @param {String} property The property to use.
+   * @return {Array} An array of strings.
+   */
+  getValues(property) {
+    return this.properties[property] ? this.properties[property].values : [];
   }
 };
 
-exports.CssProperties = CssProperties;
-
 /**
  * Create a CssProperties object with a fully loaded CSS database. The
  * CssProperties interface can be queried synchronously, but the initialization
@@ -116,8 +123,8 @@ exports.CssProperties = CssProperties;
  * @param {Toolbox} The current toolbox.
  * @returns {Promise} Resolves to {cssProperties, cssPropertiesFront}.
  */
-exports.initCssProperties = Task.async(function* (toolbox) {
-  let client = toolbox.target.client;
+const initCssProperties = Task.async(function* (toolbox) {
+  const client = toolbox.target.client;
   if (cachedCssProperties.has(client)) {
     return cachedCssProperties.get(client);
   }
@@ -127,12 +134,63 @@ exports.initCssProperties = Task.async(function* (toolbox) {
   // Get the list dynamically if the cssProperties actor exists.
   if (toolbox.target.hasActor("cssProperties")) {
     front = CssPropertiesFront(client, toolbox.target.form);
-    db = yield front.getCSSDatabase();
+    const serverDB = yield front.getCSSDatabase(getClientBrowserVersion(toolbox));
 
-    // Even if the target has the cssProperties actor, the returned data may
-    // not be in the same shape or have all of the data we need. The following
-    // code normalizes this data.
+    // The serverDB will be blank if the browser versions match, so use the static list.
+    if (!serverDB.properties && !serverDB.margin) {
+      db = CSS_PROPERTIES_DB;
+    } else {
+      db = normalizeCssData(serverDB);
+    }
+  } else {
+    // The target does not support this actor, so require a static list of supported
+    // properties.
+    db = CSS_PROPERTIES_DB;
+  }
 
+  // Color values are omitted to save on space. Add them back here.
+  reattachCssColorValues(db);
+
+  const cssProperties = new CssProperties(db);
+  cachedCssProperties.set(client, {cssProperties, front});
+  return {cssProperties, front};
+});
+
+/**
+ * Synchronously get a cached and initialized CssProperties.
+ *
+ * @param {Toolbox} The current toolbox.
+ * @returns {CssProperties}
+ */
+function getCssProperties(toolbox) {
+  if (!cachedCssProperties.has(toolbox.target.client)) {
+    throw new Error("The CSS database has not been initialized, please make " +
+                    "sure initCssDatabase was called once before for this " +
+                    "toolbox.");
+  }
+  return cachedCssProperties.get(toolbox.target.client).cssProperties;
+}
+
+/**
+ * Get the current browser version.
+ * @returns {string} The browser version.
+ */
+function getClientBrowserVersion(toolbox) {
+  if (!toolbox._host) {
+    return "0";
+  }
+  const regexResult = toolbox._host.frame.contentWindow.navigator
+                             .userAgent.match(/Firefox\/(\d+)\.\d/);
+  return Array.isArray(regexResult) ? regexResult[1] : "0";
+}
+
+/**
+ * Even if the target has the cssProperties actor, the returned data may not be in the
+ * same shape or have all of the data we need. This normalizes this data.
+ *
+ * @return {Object} The normalized CSS database.
+ */
+function normalizeCssData(db) {
   // Firefox 49's getCSSDatabase() just returned the properties object, but
   // now it returns an object with multiple types of CSS information.
   if (!db.properties) {
@@ -150,30 +208,40 @@ exports.initCssProperties = Task.async(function* (toolbox) {
       }
     }
   }
-  } else {
-    // The target does not support this actor, so require a static list of supported
-    // properties.
-    db = CSS_PROPERTIES_DB;
+
+  // Add "values" information to the css properties if it's missing.
+  if (!db.properties.color.values) {
+    for (let name in db.properties) {
+      if (typeof CSS_PROPERTIES_DB.properties[name] === "object") {
+        db.properties[name].values = CSS_PROPERTIES_DB.properties[name].values;
+      }
+    }
   }
 
-  const cssProperties = new CssProperties(db);
-  cachedCssProperties.set(client, {cssProperties, front});
-  return {cssProperties, front};
-});
+  return db;
+}
 
 /**
- * Synchronously get a cached and initialized CssProperties.
- *
- * @param {Toolbox} The current toolbox.
- * @returns {CssProperties}
+ * Color values are omitted to save on space. Add them back here.
+ * @param {Object} The CSS database.
  */
-exports.getCssProperties = function (toolbox) {
-  if (!cachedCssProperties.has(toolbox.target.client)) {
-    throw new Error("The CSS database has not been initialized, please make " +
-                    "sure initCssDatabase was called once before for this " +
-                    "toolbox.");
-  }
-  return cachedCssProperties.get(toolbox.target.client).cssProperties;
-};
+function reattachCssColorValues(db) {
+  if (db.properties.color.values[0] === "COLOR") {
+    const colors = Object.keys(cssColors);
 
-exports.CssPropertiesFront = CssPropertiesFront;
+    for (let name in db.properties) {
+      const property = db.properties[name];
+      if (property.values[0] === "COLOR") {
+        property.values.shift();
+        property.values = property.values.concat(colors).sort();
+      }
+    }
+  }
+}
+
+module.exports = {
+  CssPropertiesFront,
+  CssProperties,
+  getCssProperties,
+  initCssProperties
+};

devtools/shared/specs/css-properties.js

@@ -3,14 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-const { RetVal, generateActorSpec } = require("devtools/shared/protocol");
+const { Arg, RetVal, generateActorSpec } = require("devtools/shared/protocol");
 
 const cssPropertiesSpec = generateActorSpec({
   typeName: "cssProperties",
 
   methods: {
     getCSSDatabase: {
-      request: {},
+      request: {
+        clientBrowserVersion: Arg(0, "string"),
+      },
+
       response: RetVal("json"),
     }
   }