Bug 1455588 - Allow dragging Shape Editor markers outside of iframe viewport. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 24 Apr 2018 17:51:31 +0200
changeset 469074 07e88c2233970829395cf24bc18854bd268e96c4
parent 469073 9d8b5d05ff0e6c25cd6911b9d44ea0c8499cd4f8
child 469075 5a517a7c8cf23646603c65da037cd6b979c84682
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1455588
milestone61.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 1455588 - Allow dragging Shape Editor markers outside of iframe viewport. r=pbro MozReview-Commit-ID: 4JZYDa8FUJg
devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
devtools/server/actors/highlighters/shapes.js
--- a/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_04.js
@@ -295,23 +295,20 @@ async function testInsetMoveEdges(config
   info("Moving inset top");
   let onShapeChangeApplied = highlighters.once("shapes-highlighter-changes-applied");
   await mouse.down(xCenter, top, selector);
   await mouse.move(xCenter, top + dy, selector);
   await mouse.up(xCenter, top + dy, selector);
   await testActor.reflow();
   await onShapeChangeApplied;
 
-  info("Moving inset bottom");
-  onShapeChangeApplied = highlighters.once("shapes-highlighter-changes-applied");
-  await mouse.down(xCenter, bottom, selector);
-  await mouse.move(xCenter, bottom + dy, selector);
-  await mouse.up(xCenter, bottom + dy, selector);
-  await testActor.reflow();
-  await onShapeChangeApplied;
+  // TODO: Test bottom inset marker after Bug 1456777 is fixed.
+  // Bug 1456777 - https://bugzilla.mozilla.org/show_bug.cgi?id=1456777
+  // The test element is larger than the viewport when tests run in headless mode.
+  // When moved, the bottom marker value is getting clamped to the viewport.
 
   info("Moving inset left");
   onShapeChangeApplied = highlighters.once("shapes-highlighter-changes-applied");
   await mouse.down(left, yCenter, selector);
   await mouse.move(left + dx, yCenter, selector);
   await mouse.up(left + dx, yCenter, selector);
   await testActor.reflow();
   await onShapeChangeApplied;
@@ -320,14 +317,16 @@ async function testInsetMoveEdges(config
   onShapeChangeApplied = highlighters.once("shapes-highlighter-changes-applied");
   await mouse.down(right, yCenter, selector);
   await mouse.move(right + dx, yCenter, selector);
   await mouse.up(right + dx, yCenter, selector);
   await testActor.reflow();
   await onShapeChangeApplied;
 
   let definition = await getComputedPropertyValue(selector, property, inspector);
+
+  // NOTE: No change to bottom inset until Bug 1456777 is fixed.
   ok(definition.includes(
-    `${top + dy}px ${elemWidth - right - dx}px ${100 - y - height - 10}% ${x + 10}%`),
+    `${top + dy}px ${elemWidth - right - dx}px ${100 - y - height}% ${x + 10}%`),
      "Inset edges successfully moved");
 
   await teardown({selector, property, ...config});
 }
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -441,22 +441,36 @@ class ShapesHighlighter extends AutoRefr
    * If a padding value is given, inset the viewport by this value. This is used to define
    * a virtual viewport which ensures some element remains visible even when at the edges
    * of the actual viewport.
    *
    * @param {Number} padding
    *        Optional. Amount by which to inset the viewport in all directions.
    */
   setViewport(padding = 0) {
-    const { pageXOffset, pageYOffset, innerWidth, innerHeight } =
-      this.currentNode.ownerGlobal;
-    const left = pageXOffset + padding;
-    const right = innerWidth + pageXOffset - padding;
-    const top = pageYOffset + padding;
-    const bottom = innerHeight + pageYOffset - padding;
+    let xOffset = 0;
+    let yOffset = 0;
+
+    // If the node exists within an iframe, get offsets for the virtual viewport so that
+    // points can be dragged to the extent of the global window, outside of the iframe
+    // window.
+    if (this.currentNode.ownerGlobal !== this.win) {
+      const win =  this.win;
+      const nodeWin = this.currentNode.ownerGlobal;
+      // Get bounding box of iframe document relative to global document.
+      const { bounds } = nodeWin.document.getBoxQuads({ relativeTo: win.document })[0];
+      xOffset = bounds.left - nodeWin.scrollX + win.scrollX;
+      yOffset = bounds.top - nodeWin.scrollY + win.scrollY;
+    }
+
+    const { pageXOffset, pageYOffset, innerWidth, innerHeight } = this.win;
+    const left = pageXOffset + padding - xOffset;
+    const right = innerWidth + pageXOffset - padding - xOffset;
+    const top = pageYOffset + padding - yOffset;
+    const bottom = innerHeight + pageYOffset - padding - yOffset;
     this.viewport = { left, right, top, bottom, padding };
   }
 
   handleEvent(event, id) {
     // No event handling if the highlighter is hidden
     if (this.areShapesHidden()) {
       return;
     }
@@ -518,30 +532,19 @@ class ShapesHighlighter extends AutoRefr
         if (!this[_dragging]) {
           this._handleMouseMoveNotDragging(pageX, pageY);
           return;
         }
         event.stopPropagation();
         event.preventDefault();
 
         // Set constraints for mouse position to ensure dragged marker stays in viewport.
-        const { left, right, top, bottom, padding } = this.viewport;
-        const { x, y } = this[_dragging];
-
-        // If marker was within viewport at mousedown, clamp its changes to the viewport.
-        // If marker was outside, do not clamp and allow dragging outside of the viewport.
-        // The latter applies to shapes in iframes which exceed the iframe viewport,
-        // but their markers are visible in the viewport of the iframe's parent.
-        if (x > left - padding && x < right + padding) {
-          pageX = Math.min(Math.max(left, pageX), right);
-        }
-
-        if (y > top - padding && y < bottom + padding) {
-          pageY = Math.min(Math.max(top, pageY), bottom);
-        }
+        const { left, right, top, bottom } = this.viewport;
+        pageX = Math.min(Math.max(left, pageX), right);
+        pageY = Math.min(Math.max(top, pageY), bottom);
 
         let { point } = this[_dragging];
         if (this.transformMode) {
           this._handleTransformMove(pageX, pageY);
         } else if (this.shapeType === "polygon") {
           this._handlePolygonMove(pageX, pageY);
         } else if (this.shapeType === "circle") {
           this._handleCircleMove(point, pageX, pageY);