Bug 1385398 - Change the appearance of CSS shapes highlighter markers. r=pbro

MozReview-Commit-ID: JO4TZ1Gsr6m

--HG--
extra : rebase_source : 6c6768ecef35180579c416775822a6960f505b2a

devtools/server/actors/highlighters.css

@@ -613,7 +613,11 @@
 }
 
 :-moz-native-anonymous .shapes-markers {
-  fill: var(--highlighter-marker-color);
+  fill: #fff;
+}
+
+:-moz-native-anonymous .shapes-markers-outline {
+  fill: var(--highlighter-guide-color);
 }
 
 :-moz-native-anonymous .shapes-marker-hover {

devtools/server/actors/highlighters/shapes.js

@@ -17,7 +17,7 @@ const {
 } = require("devtools/server/actors/utils/shapes-geometry-utils");
 const EventEmitter = require("devtools/shared/old-event-emitter");
 
-const BASE_MARKER_SIZE = 10;
+const BASE_MARKER_SIZE = 5;
 // the width of the area around highlighter lines that can be clicked, in px
 const LINE_CLICK_WIDTH = 5;
 const DOM_EVENTS = ["mousedown", "mousemove", "mouseup", "dblclick"];
@@ -116,6 +116,16 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
     });
 
     // Append a path to display the markers for the shape.
+    createSVGNode(this.win, {
+      nodeType: "path",
+      parent: mainSvg,
+      attributes: {
+        "id": "markers-outline",
+        "class": "markers-outline",
+      },
+      prefix: this.ID_CLASS_PREFIX
+    });
+
     createSVGNode(this.win, {
       nodeType: "path",
       parent: mainSvg,
@@ -193,24 +203,11 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
         } else if (this.shapeType === "inset") {
           this._handleInsetClick(pageX, pageY);
         }
-        // Currently, changes to shape-outside do not become visible unless a reflow
-        // is forced (bug 1359834). This is a hack to force a reflow so changes made
-        // using the highlighter can be seen: we change the width of the element
-        // slightly on mousedown on a point, and restore the original width on mouseup.
-        if (this.property === "shape-outside" && this[_dragging]) {
-          let { width } = this.zoomAdjustedDimensions;
-          let origWidth = getDefinedShapeProperties(this.currentNode, "width");
-          this.currentNode.style.setProperty("width", `${width + 1}px`);
-          this[_dragging].origWidth = origWidth;
-        }
         event.stopPropagation();
         event.preventDefault();
         break;
       case "mouseup":
         if (this[_dragging]) {
-          if (this.property === "shape-outside") {
-            this.currentNode.style.setProperty("width", this[_dragging].origWidth);
-          }
           this[_dragging] = null;
         }
         break;
@@ -675,7 +672,7 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
     let { width, height } = this.zoomAdjustedDimensions;
     let zoom = getCurrentZoom(this.win);
     let path = points.map(([x, y]) => {
-      return getCirclePath(x, y, width, height, zoom);
+      return getCirclePath(BASE_MARKER_SIZE, x, y, width, height, zoom);
     }).join(" ");
 
     let markerHover = this.getElement("marker-hover");
@@ -1274,6 +1271,7 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
     this.getElement("polygon").setAttribute("hidden", true);
     this.getElement("rect").setAttribute("hidden", true);
     this.getElement("markers").setAttribute("d", "");
+    this.getElement("markers-outline").setAttribute("d", "");
   }
 
   /**
@@ -1402,10 +1400,14 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
    */
   _drawMarkers(coords, width, height, zoom) {
     let markers = coords.map(([x, y]) => {
-      return getCirclePath(x, y, width, height, zoom);
+      return getCirclePath(BASE_MARKER_SIZE, x, y, width, height, zoom);
+    }).join(" ");
+    let outline = coords.map(([x, y]) => {
+      return getCirclePath(BASE_MARKER_SIZE + 2, x, y, width, height, zoom);
     }).join(" ");
 
     this.getElement("markers").setAttribute("d", markers);
+    this.getElement("markers-outline").setAttribute("d", outline);
   }
 
   /**
@@ -1531,6 +1533,7 @@ exports.shapeModeToCssPropertyName = shapeModeToCssPropertyName;
 
 /**
  * Get the SVG path definition for a circle with given attributes.
+ * @param {Number} size the radius of the circle in pixels
  * @param {Number} cx the x coordinate of the centre of the circle
  * @param {Number} cy the y coordinate of the centre of the circle
  * @param {Number} width the width of the element the circle is being drawn for
@@ -1538,14 +1541,14 @@ exports.shapeModeToCssPropertyName = shapeModeToCssPropertyName;
  * @param {Number} zoom the zoom level of the window the circle is drawn in
  * @returns {String} the definition of the circle in SVG path description format.
  */
-const getCirclePath = (cx, cy, width, height, zoom) => {
+const getCirclePath = (size, cx, cy, width, height, zoom) => {
   // We use a viewBox of 100x100 for shape-container so it's easy to position things
   // based on their percentage, but this makes it more difficult to create circles.
   // Therefor, 100px is the base size of shape-container. In order to make the markers'
   // size scale properly, we must adjust the radius based on zoom and the width/height of
   // the element being highlighted, then calculate a radius for both x/y axes based
   // on the aspect ratio of the element.
-  let radius = BASE_MARKER_SIZE * (100 / Math.max(width, height)) / zoom;
+  let radius = size * (100 / Math.max(width, height)) / zoom;
   let ratio = width / height;
   let rx = (ratio > 1) ? radius : radius / ratio;
   let ry = (ratio > 1) ? radius * ratio : radius;

devtools/server/tests/unit/test_shapes_highlighter_helpers.js

@@ -106,25 +106,25 @@ function test_shape_mode_to_css_property_name() {
 
 function test_get_circle_path() {
   const tests = [{
-    desc: "getCirclePath with no resizing, no zoom, 1:1 ratio",
-    cx: 0, cy: 0, width: 100, height: 100, zoom: 1,
-    expected: "M-10,0a10,10 0 1,0 20,0a10,10 0 1,0 -20,0"
-  }, {
-    desc: "getCirclePath with resizing, no zoom, 1:1 ratio",
-    cx: 0, cy: 0, width: 200, height: 200, zoom: 1,
+    desc: "getCirclePath with size 5, no resizing, no zoom, 1:1 ratio",
+    size: 5, cx: 0, cy: 0, width: 100, height: 100, zoom: 1,
     expected: "M-5,0a5,5 0 1,0 10,0a5,5 0 1,0 -10,0"
   }, {
-    desc: "getCirclePath with resizing, zoom, 1:1 ratio",
-    cx: 0, cy: 0, width: 200, height: 200, zoom: 2,
-    expected: "M-2.5,0a2.5,2.5 0 1,0 5,0a2.5,2.5 0 1,0 -5,0"
+    desc: "getCirclePath with size 7, resizing, no zoom, 1:1 ratio",
+    size: 7, cx: 0, cy: 0, width: 200, height: 200, zoom: 1,
+    expected: "M-3.5,0a3.5,3.5 0 1,0 7,0a3.5,3.5 0 1,0 -7,0"
   }, {
-    desc: "getCirclePath with resizing, zoom, non-square ratio",
-    cx: 0, cy: 0, width: 100, height: 200, zoom: 2,
-    expected: "M-5,0a5,2.5 0 1,0 10,0a5,2.5 0 1,0 -10,0"
+    desc: "getCirclePath with size 5, resizing, zoom, 1:1 ratio",
+    size: 5, cx: 0, cy: 0, width: 200, height: 200, zoom: 2,
+    expected: "M-1.25,0a1.25,1.25 0 1,0 2.5,0a1.25,1.25 0 1,0 -2.5,0"
+  }, {
+    desc: "getCirclePath with size 5, resizing, zoom, non-square ratio",
+    size: 5, cx: 0, cy: 0, width: 100, height: 200, zoom: 2,
+    expected: "M-2.5,0a2.5,1.25 0 1,0 5,0a2.5,1.25 0 1,0 -5,0"
   }];
 
-  for (let { desc, cx, cy, width, height, zoom, expected } of tests) {
-    equal(getCirclePath(cx, cy, width, height, zoom), expected, desc);
+  for (let { desc, size, cx, cy, width, height, zoom, expected } of tests) {
+    equal(getCirclePath(size, cx, cy, width, height, zoom), expected, desc);
   }
 }