Bug 1453428 - Handle cases for shape coordinates like 0%, 0em, 0rem, etc. without overwriting to pixels. r=gl
The previous implementation treated any zero coordinate as pixels,
regardless of its unit type. For example, 0% would be converted to 0px.
This changed the shape value resulting in unintentional unit mismatch
after editing a coordinate which started off as 0% or 0em or 0vh, etc.
This patch fixes that and preserves the unit type for zero coordinates.
It changes the implementation of isUnitless() to return false for values
like 0%, 0px, 0em, 0.00000%, etc.
It introduces getUnitToPixelRatio() to get the ratio by which to
multiply a pixel value to convert it to the given unit during shape
editing operations (point move, scale, translate, rotate, etc.)
The ratio is constant by unit type. Previously, this ratio was
calculated in-place for each unit value. Values which started as zero,
always resulted in a ratio equal to zero. This multiplied with a pixel
value resulted zero. So the previous code defaulted to ratio of 1 when
the ratio was zero, but this meant that a 1:1 ratio between pixels and
another unit type (%, em, vh) would result in disproportionate
shape changes (1px of mouse travel would result in 1em). This is why
isUnitless() originally discarded the original unit from a zero
coordinate and replaced it with pixels; to account for this fallback
ratio of 1.
MozReview-Commit-ID: 49tyLfYjkLO
--- a/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
@@ -57,30 +57,32 @@ async function testPolygonMovePoint(conf
let points = await testActor.getHighlighterNodeAttribute(
"shapes-polygon", "points", highlighters.highlighters[HIGHLIGHTER_TYPE]);
let [x, y] = points.split(" ")[0].split(",");
let quads = await testActor.getAllAdjustedQuads(selector);
let { top, left, width, height } = quads.border[0].bounds;
x = left + width * x / 100;
y = top + height * y / 100;
let dx = width / 10;
- let dy = height / 10;
+ let dyPercent = 10;
+ let dy = height / dyPercent;
let onRuleViewChanged = view.once("ruleview-changed");
info("Moving first polygon point");
let { mouse } = helper;
await mouse.down(x, y);
await mouse.move(x + dx, y + dy);
await mouse.up();
await testActor.reflow();
info("Waiting for rule view changed from shape change");
await onRuleViewChanged;
let definition = await getComputedPropertyValue(selector, property, inspector);
- ok(definition.includes(`${dx}px ${dy}px`), `Point moved to ${dx}px ${dy}px`);
+ ok(definition.includes(`${dx}px ${dyPercent}%`),
+ `Point moved to ${dx}px ${dyPercent}%`);
await teardown({selector, property, ...config});
}
async function testPolygonAddPoint(config) {
const {inspector, view, highlighters, testActor, helper} = config;
const selector = "#polygon";
const property = "clip-path";
--- a/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_iframe_01.js
@@ -30,30 +30,34 @@ async function testPolygonIframeMovePoin
let highlightedNode = await getNodeFrontInFrame(selector, "#frame", inspector);
// Select the nested node so toggling of the shapes highlighter works from the rule view
await selectNode(highlightedNode, inspector);
await toggleShapesHighlighter(view, selector, property, true);
let { mouse } = helper;
let onRuleViewChanged = view.once("ruleview-changed");
info("Moving polygon point visible in iframe");
+ // Iframe has 10px margin. Element in iframe is 800px by 800px. First point is at 0 0%
await mouse.down(10, 10);
await mouse.move(20, 20);
await mouse.up();
await testActor.reflow();
await onRuleViewChanged;
let computedStyle = await inspector.pageStyle.getComputed(highlightedNode);
let definition = computedStyle["clip-path"].value;
- ok(definition.includes("10px 10px"), "Point moved to 10px 10px");
+ ok(definition.includes("10px 1.25%"), "Point moved to 10px 1.25%");
onRuleViewChanged = view.once("ruleview-changed");
info("Moving polygon point not visible in iframe");
await mouse.down(110, 410);
await mouse.move(120, 420);
await mouse.up();
await testActor.reflow();
await onRuleViewChanged;
computedStyle = await inspector.pageStyle.getComputed(highlightedNode);
definition = computedStyle["clip-path"].value;
ok(definition.includes("110px 51.25%"), "Point moved to 110px 51.25%");
+
+ info(`Turn off shapes highlighter for ${selector}`);
+ await toggleShapesHighlighter(view, selector, property, false);
}
--- a/devtools/client/inspector/test/doc_inspector_highlighter_cssshapes.html
+++ b/devtools/client/inspector/test/doc_inspector_highlighter_cssshapes.html
@@ -8,17 +8,17 @@
margin: 0;
}
.wrapper {
width: 800px;
height: 800px;
background: #f06;
}
#polygon {
- clip-path: polygon(0 0,
+ clip-path: polygon(0 0%,
100px 50%,
200px 0,
300px 50%,
400px 0,
500px 50%,
600px 0,
700px 50%,
800px 0,
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -558,18 +558,18 @@ class ShapesHighlighter extends AutoRefr
let pointsInfo = this.origCoordUnits.map(([x, y], i) => {
let xComputed = this.origCoordinates[i][0] / 100 * width;
let yComputed = this.origCoordinates[i][1] / 100 * height;
let unitX = getUnit(x);
let unitY = getUnit(y);
let valueX = (isUnitless(x)) ? xComputed : parseFloat(x);
let valueY = (isUnitless(y)) ? yComputed : parseFloat(y);
- let ratioX = (valueX / xComputed) || 1;
- let ratioY = (valueY / yComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
return { unitX, unitY, valueX, valueY, ratioX, ratioY };
});
this[_dragging] = { type, pointsInfo, x: pageX, y: pageY, bb: this.boundingBox,
matrix: this.transformMatrix,
transformedBB: this.transformedBoundingBox };
}
/**
@@ -583,26 +583,26 @@ class ShapesHighlighter extends AutoRefr
let { cx, cy } = this.origCoordUnits;
let cxComputed = this.origCoordinates.cx / 100 * width;
let cyComputed = this.origCoordinates.cy / 100 * height;
let unitX = getUnit(cx);
let unitY = getUnit(cy);
let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
- let ratioX = (valueX / cxComputed) || 1;
- let ratioY = (valueY / cyComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
let { radius } = this.origCoordinates;
let computedSize = Math.sqrt((width ** 2) + (height ** 2)) / Math.sqrt(2);
radius = radius / 100 * computedSize;
let valueRad = this.origCoordUnits.radius;
let unitRad = getUnit(valueRad);
valueRad = (isUnitless(valueRad)) ? radius : parseFloat(valueRad);
- let ratioRad = (valueRad / radius) || 1;
+ let ratioRad = this.getUnitToPixelRatio(unitRad, computedSize);
this[_dragging] = { type, unitX, unitY, unitRad, valueX, valueY,
ratioX, ratioY, ratioRad, x: pageX, y: pageY,
bb: this.boundingBox, matrix: this.transformMatrix,
transformedBB: this.transformedBoundingBox };
}
/**
@@ -616,18 +616,18 @@ class ShapesHighlighter extends AutoRefr
let { cx, cy } = this.origCoordUnits;
let cxComputed = this.origCoordinates.cx / 100 * width;
let cyComputed = this.origCoordinates.cy / 100 * height;
let unitX = getUnit(cx);
let unitY = getUnit(cy);
let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
- let ratioX = (valueX / cxComputed) || 1;
- let ratioY = (valueY / cyComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
let { rx, ry } = this.origCoordinates;
rx = rx / 100 * width;
let valueRX = this.origCoordUnits.rx;
let unitRX = getUnit(valueRX);
valueRX = (isUnitless(valueRX)) ? rx : parseFloat(valueRX);
let ratioRX = (valueRX / rx) || 1;
ry = ry / 100 * height;
@@ -653,17 +653,17 @@ class ShapesHighlighter extends AutoRefr
let { width, height } = this.currentDimensions;
let pointsInfo = {};
["top", "right", "bottom", "left"].forEach(point => {
let value = this.origCoordUnits[point];
let size = (point === "left" || point === "right") ? width : height;
let computedValue = this.origCoordinates[point] / 100 * size;
let unit = getUnit(value);
value = (isUnitless(value)) ? computedValue : parseFloat(value);
- let ratio = (value / computedValue) || 1;
+ let ratio = this.getUnitToPixelRatio(unit, size);
pointsInfo[point] = { value, unit, ratio };
});
this[_dragging] = { type, pointsInfo, x: pageX, y: pageY, bb: this.boundingBox,
matrix: this.transformMatrix,
transformedBB: this.transformedBoundingBox };
}
@@ -941,18 +941,18 @@ class ShapesHighlighter extends AutoRefr
let [x, y] = this.coordUnits[point];
let xComputed = this.coordinates[point][0] / 100 * width;
let yComputed = this.coordinates[point][1] / 100 * height;
let unitX = getUnit(x);
let unitY = getUnit(y);
let valueX = (isUnitless(x)) ? xComputed : parseFloat(x);
let valueY = (isUnitless(y)) ? yComputed : parseFloat(y);
- let ratioX = (valueX / xComputed) || 1;
- let ratioY = (valueY / yComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
this.setCursor("grabbing");
this[_dragging] = { point, unitX, unitY, valueX, valueY,
ratioX, ratioY, x: pageX, y: pageY };
}
/**
* Update the dragged polygon point with the given x/y coords and update
@@ -1036,29 +1036,29 @@ class ShapesHighlighter extends AutoRefr
let { cx, cy } = this.coordUnits;
let cxComputed = this.coordinates.cx / 100 * width;
let cyComputed = this.coordinates.cy / 100 * height;
let unitX = getUnit(cx);
let unitY = getUnit(cy);
let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
- let ratioX = (valueX / cxComputed) || 1;
- let ratioY = (valueY / cyComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
this[_dragging] = { point, unitX, unitY, valueX, valueY,
ratioX, ratioY, x: pageX, y: pageY };
} else if (point === "radius") {
let { radius } = this.coordinates;
let computedSize = Math.sqrt((width ** 2) + (height ** 2)) / Math.sqrt(2);
radius = radius / 100 * computedSize;
let value = this.coordUnits.radius;
let unit = getUnit(value);
value = (isUnitless(value)) ? radius : parseFloat(value);
- let ratio = (value / radius) || 1;
+ let ratio = this.getUnitToPixelRatio(unit, computedSize);
this[_dragging] = { point, value, origRadius: radius, unit, ratio };
}
}
/**
* Set the center/radius of the circle according to the mouse position and
* update the element style.
@@ -1115,37 +1115,37 @@ class ShapesHighlighter extends AutoRefr
let { cx, cy } = this.coordUnits;
let cxComputed = this.coordinates.cx / 100 * width;
let cyComputed = this.coordinates.cy / 100 * height;
let unitX = getUnit(cx);
let unitY = getUnit(cy);
let valueX = (isUnitless(cx)) ? cxComputed : parseFloat(cx);
let valueY = (isUnitless(cy)) ? cyComputed : parseFloat(cy);
- let ratioX = (valueX / cxComputed) || 1;
- let ratioY = (valueY / cyComputed) || 1;
+ let ratioX = this.getUnitToPixelRatio(unitX, width);
+ let ratioY = this.getUnitToPixelRatio(unitY, height);
this[_dragging] = { point, unitX, unitY, valueX, valueY,
ratioX, ratioY, x: pageX, y: pageY };
} else if (point === "rx") {
let { rx } = this.coordinates;
rx = rx / 100 * width;
let value = this.coordUnits.rx;
let unit = getUnit(value);
value = (isUnitless(value)) ? rx : parseFloat(value);
- let ratio = (value / rx) || 1;
+ let ratio = this.getUnitToPixelRatio(unit, width);
this[_dragging] = { point, value, origRadius: rx, unit, ratio };
} else if (point === "ry") {
let { ry } = this.coordinates;
ry = ry / 100 * height;
let value = this.coordUnits.ry;
let unit = getUnit(value);
value = (isUnitless(value)) ? ry : parseFloat(value);
- let ratio = (value / ry) || 1;
+ let ratio = this.getUnitToPixelRatio(unit, height);
this[_dragging] = { point, value, origRadius: ry, unit, ratio };
}
}
/**
* Set center/rx/ry of the ellispe according to the mouse position and update the
* element style.
@@ -1208,17 +1208,17 @@ class ShapesHighlighter extends AutoRefr
}
this.setCursor("grabbing");
let value = this.coordUnits[point];
let size = (point === "left" || point === "right") ? width : height;
let computedValue = this.coordinates[point] / 100 * size;
let unit = getUnit(value);
value = (isUnitless(value)) ? computedValue : parseFloat(value);
- let ratio = (value / computedValue) || 1;
+ let ratio = this.getUnitToPixelRatio(unit, size);
let origValue = (point === "left" || point === "right") ? pageX : pageY;
this[_dragging] = { point, value, origValue, unit, ratio };
}
/**
* Set the top/left/right/bottom of the inset shape according to the mouse position
* and update the element style.
@@ -2495,16 +2495,56 @@ class ShapesHighlighter extends AutoRefr
return "ns";
} else if (dy >= -0.33 && dy <= 0.33) {
return "ew";
} else if ((dx > 0.33 && dy < -0.33) || (dx < -0.33 && dy > 0.33)) {
return "nesw";
}
return "nwse";
}
+
+ /**
+ * Given a unit type, get the ratio by which to multiply a pixel value in order to
+ * convert pixels to that unit.
+ *
+ * Percentage units (%) are relative to a size. This must be provided when requesting
+ * a ratio for converting from pixels to percentages.
+ *
+ * @param {String} unit
+ * One of: %, em, rem, vw, vh
+ * @param {Number} size
+ * Size to which percentage values are relative to.
+ * @return {Number}
+ */
+ getUnitToPixelRatio(unit, size) {
+ let ratio;
+ switch (unit) {
+ case "%":
+ ratio = 100 / size;
+ break;
+ case "em":
+ ratio = 1 / parseFloat(getComputedStyle(this.currentNode).fontSize);
+ break;
+ case "rem":
+ const root = this.currentNode.ownerDocument.documentElement;
+ ratio = 1 / parseFloat(getComputedStyle(root).fontSize);
+ break;
+ case "vw":
+ ratio = 100 / this.win.innerWidth;
+ break;
+ case "vh":
+ ratio = 100 / this.win.innerHeight;
+ break;
+ default:
+ // If unit is not recognized, peg ratio 1:1 to pixels.
+ ratio = 1;
+ }
+
+ return ratio;
+ }
}
/**
* Get the "raw" (i.e. non-computed) shape definition on the given node.
* @param {nsIDOMNode} node the node to analyze
* @param {String} property the CSS property for which a value should be retrieved.
* @returns {String} the value of the given CSS property on the given node.
*/
@@ -2684,21 +2724,20 @@ const getUnit = (point) => {
exports.getUnit = getUnit;
/**
* Check if the given point value has a unit.
* @param {any} point a point value.
* @returns {Boolean} whether the given value has a unit.
*/
const isUnitless = (point) => {
- // We treat all values that evaluate to 0 as unitless, regardless of whether
- // they originally had a unit.
return !point ||
!point.match(/[^\d]+$/) ||
- parseFloat(point) === 0 ||
+ // If zero doesn't have a unit, its numeric and string forms should be equal.
+ (parseFloat(point) === 0 && (parseFloat(point).toString() === point)) ||
point.includes("(") ||
point === "closest-side" ||
point === "farthest-side";
};
/**
* Return the anchor corresponding to the given scale type.
* @param {String} type a scale type, of form "scale-[direction]"
--- a/devtools/server/tests/unit/test_shapes_highlighter_helpers.js
+++ b/devtools/server/tests/unit/test_shapes_highlighter_helpers.js
@@ -163,20 +163,26 @@ function test_get_unit() {
}, {
desc: "getUnit with em",
expr: "4em", expected: "em"
}, {
desc: "getUnit with 0",
expr: "0", expected: "px"
}, {
desc: "getUnit with 0%",
- expr: "0%", expected: "px"
+ expr: "0%", expected: "%"
+ }, {
+ desc: "getUnit with 0.00%",
+ expr: "0.00%", expected: "%"
}, {
- desc: "getUnit with no unit",
- expr: "30", expected: "px"
+ desc: "getUnit with 0px",
+ expr: "0px", expected: "px"
+ }, {
+ desc: "getUnit with 0em",
+ expr: "0em", expected: "em"
}, {
desc: "getUnit with calc",
expr: "calc(30px + 5%)", expected: "px"
}, {
desc: "getUnit with var",
expr: "var(--variable)", expected: "px"
}, {
desc: "getUnit with closest-side",