Bug 1483575 - (Part 3) Update unit conversion to handle unitless values. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 16 Aug 2018 14:49:36 +0000
changeset 487204 26c699458e1fbaa99c3915d9fb0bb3f04b559797
parent 487203 961c73e8fac1b302699d3258a3b00aac7a66cb58
child 487205 acbbc15249bf3fcb645ac5d685a7e0fdccd9cae2
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1483575
milestone63.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 1483575 - (Part 3) Update unit conversion to handle unitless values. r=gl Add special handling for line-height property. Depends on D3498 Differential Revision: https://phabricator.services.mozilla.com/D3500
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js
devtools/client/inspector/fonts/utils/font-utils.js
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -142,47 +142,58 @@ class FontInspector {
     });
   }
 
   /**
    * Convert a value for font-size between two CSS unit types.
    * Conversion is done via pixels. If neither of the two given unit types is "px",
    * recursively get the value in pixels, then convert that result to the desired unit.
    *
+   * @param  {String} property
+   *         Property name for the converted value.
+   *         Assumed to be "font-size", but special case for "line-height".
    * @param  {Number} value
    *         Numeric value to convert.
    * @param  {String} fromUnit
    *         CSS unit to convert from.
    * @param  {String} toUnit
    *         CSS unit to convert to.
    * @return {Number}
    *         Converted numeric value.
    */
-  async convertUnits(value, fromUnit, toUnit) {
+  async convertUnits(property, value, fromUnit, toUnit) {
     if (value !== parseFloat(value)) {
       throw TypeError(`Invalid value for conversion. Expected Number, got ${value}`);
     }
 
     if (fromUnit === toUnit) {
       return value;
     }
 
+    // Special case for line-height. Consider em and untiless to be equivalent.
+    if (property === "line-height" &&
+       (fromUnit === "" && toUnit === "em") || (fromUnit === "em" && toUnit === "")) {
+      return value;
+    }
+
     // If neither unit is in pixels, first convert the value to pixels.
     // Reassign input value and source CSS unit.
     if (toUnit !== "px" && fromUnit !== "px") {
-      value = await this.convertUnits(value, fromUnit, "px");
+      value = await this.convertUnits(property, value, fromUnit, "px");
       fromUnit = "px";
     }
 
     // Whether the conversion is done from pixels.
     const fromPx = fromUnit === "px";
     // Determine the target CSS unit for conversion.
     const unit = toUnit === "px" ? fromUnit : toUnit;
     // NodeFront instance of selected element.
     const node = this.inspector.selection.nodeFront;
+    // Reference node based on which to convert relative sizes like "em" and "%".
+    const referenceNode = (property === "line-height") ? node : node.parentNode();
     // Default output value to input value for a 1-to-1 conversion as a guard against
     // unrecognized CSS units. It will not be correct, but it will also not break.
     let out = value;
     // Computed style for reference node used for conversion of "em", "rem", "%".
     let computedStyle;
 
     if (unit === "in") {
       out = fromPx
@@ -211,30 +222,31 @@ class FontInspector {
     if (unit === "pc") {
       out = fromPx
         ? value * 0.0625
         : value / 0.0625;
     }
 
     if (unit === "%") {
       computedStyle =
-        await this.pageStyle.getComputed(node.parentNode()).catch(console.error);
+        await this.pageStyle.getComputed(referenceNode).catch(console.error);
 
       if (!computedStyle) {
         return value;
       }
 
       out = fromPx
         ? value * 100 / parseFloat(computedStyle["font-size"].value)
         : value / 100 * parseFloat(computedStyle["font-size"].value);
     }
 
-    if (unit === "em") {
+    // Special handling for unitless line-height.
+    if (unit === "em" || (unit === "" && property === "line-height")) {
       computedStyle =
-        await this.pageStyle.getComputed(node.parentNode()).catch(console.error);
+        await this.pageStyle.getComputed(referenceNode).catch(console.error);
 
       if (!computedStyle) {
         return value;
       }
 
       out = fromPx
         ? value / parseFloat(computedStyle["font-size"].value)
         : value * parseFloat(computedStyle["font-size"].value);
@@ -767,18 +779,19 @@ class FontInspector {
    *         Optional CSS unit to convert from
    * @param  {String|undefined} toUnit
    *         Optional CSS unit to convert to
    */
   async onPropertyChange(property, value, fromUnit, toUnit) {
     if (FONT_PROPERTIES.includes(property)) {
       let unit = fromUnit;
 
-      if (toUnit && fromUnit) {
-        value = await this.convertUnits(value, fromUnit, toUnit);
+      // Strict checks because "line-height" value may be unitless (empty string).
+      if (toUnit !== undefined && fromUnit !== undefined) {
+        value = await this.convertUnits(property, value, fromUnit, toUnit);
         unit = toUnit;
       }
 
       // Cast value to string.
       this.onFontPropertyUpdate(property, value + "", unit);
     } else {
       // Cast axis value to string.
       this.onAxisUpdate(property, value + "");
--- a/devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js
@@ -44,28 +44,28 @@ add_task(async function() {
   // Starting value and unit for conversion.
   let prevValue = fontSize.value;
   let prevUnit = fontSize.unit;
 
   for (const unit in UNITS) {
     const value = UNITS[unit];
 
     info(`Convert font-size from ${prevValue}${prevUnit} to ${unit}`);
-    const convertedValue = await view.convertUnits(prevValue, prevUnit, unit);
+    const convertedValue = await view.convertUnits(property, prevValue, prevUnit, unit);
     is(convertedValue, value, `Converting to ${unit} returns transformed value.`);
 
     // Store current unit and value to use in conversion on the next iteration.
     prevUnit = unit;
     prevValue = value;
   }
 
   info(`Check that conversion from fake unit returns 1-to-1 mapping.`);
-  const valueFromFakeUnit = await view.convertUnits(1, "fake", "px");
+  const valueFromFakeUnit = await view.convertUnits(property, 1, "fake", "px");
   is(valueFromFakeUnit, 1, `Converting from fake unit returns same value.`);
 
   info(`Check that conversion to fake unit returns 1-to-1 mapping`);
-  const valueToFakeUnit = await view.convertUnits(1, "px", "fake");
+  const valueToFakeUnit = await view.convertUnits(property, 1, "px", "fake");
   is(valueToFakeUnit, 1, `Converting to fake unit returns same value.`);
 
   info(`Check that conversion between fake units returns 1-to-1 mapping.`);
-  const valueBetweenFakeUnit = await view.convertUnits(1, "bogus", "fake");
+  const valueBetweenFakeUnit = await view.convertUnits(property, 1, "bogus", "fake");
   is(valueBetweenFakeUnit, 1, `Converting between fake units returns same value.`);
 });
--- a/devtools/client/inspector/fonts/utils/font-utils.js
+++ b/devtools/client/inspector/fonts/utils/font-utils.js
@@ -12,16 +12,17 @@ module.exports = {
    * @param {String} unit
    *        CSS unit type (px, %, em, rem, vh, vw, ...)
    * @return {Number}
    *         Amount by which to increment.
    */
   getStepForUnit(unit) {
     let step;
     switch (unit) {
+      case "":
       case "em":
       case "rem":
       case "vw":
       case "vh":
       case "vmin":
       case "vmax":
         step = 0.1;
         break;