Bugfix: Correctly calculate Overlay position when inspecting a node w…

…ithin iframe(s) (#552)

* Add test app that nests iframes

* Check intermediate iframes when calculating Overlay rect (#443)

* Reuse reference to iframe node

* Move nested frames examples to sink

frontend/Highlighter/Overlay.js

@@ -11,7 +11,7 @@
 'use strict';
 
 var assign = require('object-assign');
-import type {DOMNode} from '../types';
+import type {DOMNode, DOMRect, Window} from '../types';
 
 class Overlay {
   win: Object;
@@ -89,7 +89,7 @@ class Overlay {
     if (node.nodeType !== Node.ELEMENT_NODE) {
       return;
     }
-    var box = node.getBoundingClientRect();
+    var box = getNestedBoundingClientRect(node, this.win);
     var dims = getElementDimensions(node);
 
     boxWrap(dims, 'margin', this.node);
@@ -169,6 +169,99 @@ function getElementDimensions(element) {
   };
 }
 
+// Get the window object for the document that a node belongs to,
+// or return null if it cannot be found (node not attached to DOM,
+// etc).
+function getOwnerWindow(node: DOMNode): Window | null {
+  if (!node.ownerDocument) {
+    return null;
+  }
+  return node.ownerDocument.defaultView;
+}
+
+// Get the iframe containing a node, or return null if it cannot
+// be found (node not within iframe, etc).
+function getOwnerIframe(node: DOMNode): DOMNode | null {
+  var nodeWindow = getOwnerWindow(node);
+  if (nodeWindow) {
+    return nodeWindow.frameElement;
+  }
+  return null;
+}
+
+// Get a bounding client rect for a node, with an
+// offset added to compensate for its border.
+function getBoundingClientRectWithBorderOffset(node: DOMNode) {
+  var dimensions = getElementDimensions(node);
+
+  return mergeRectOffsets([
+    node.getBoundingClientRect(),
+    {
+      top: dimensions.borderTop,
+      left: dimensions.borderLeft,
+      bottom: dimensions.borderBottom,
+      right: dimensions.borderRight,
+      // This width and height won't get used by mergeRectOffsets (since this
+      // is not the first rect in the array), but we set them so that this
+      // object typechecks as a DOMRect.
+      width: 0,
+      height: 0,
+    },
+  ]);
+}
+
+// Add together the top, left, bottom, and right properties of
+// each DOMRect, but keep the width and height of the first one.
+function mergeRectOffsets(rects: Array<DOMRect>): DOMRect {
+  return rects.reduce((previousRect, rect) => {
+    if (previousRect == null) {
+      return rect;
+    }
+
+    return {
+      top: previousRect.top + rect.top,
+      left: previousRect.left + rect.left,
+      width: previousRect.width,
+      height: previousRect.height,
+      bottom: previousRect.bottom + rect.bottom,
+      right: previousRect.right + rect.right,
+    };
+  });
+}
+
+// Calculate a boundingClientRect for a node relative to boundaryWindow,
+// taking into account any offsets caused by intermediate iframes.
+function getNestedBoundingClientRect(node: DOMNode, boundaryWindow: Window): DOMRect {
+  var ownerIframe = getOwnerIframe(node);
+  if (
+    ownerIframe &&
+    ownerIframe !== boundaryWindow
+  ) {
+    var rects = [node.getBoundingClientRect()];
+    var currentIframe = ownerIframe;
+    var onlyOneMore = false;
+    while (currentIframe) {
+      var rect = getBoundingClientRectWithBorderOffset(currentIframe);
+      rects.push(rect);
+      currentIframe = getOwnerIframe(currentIframe);
+
+      if (onlyOneMore) {
+        break;
+      }
+      // We don't want to calculate iframe offsets upwards beyond
+      // the iframe containing the boundaryWindow, but we
+      // need to calculate the offset relative to the boundaryWindow.
+      if (currentIframe && getOwnerWindow(currentIframe) === boundaryWindow) {
+        onlyOneMore = true;
+      }
+    }
+
+    return mergeRectOffsets(rects);
+  } else {
+    return node.getBoundingClientRect();
+  }
+}
+
 function boxWrap(dims, what, node) {
   assign(node.style, {
     borderTopWidth: dims[what + 'Top'] + 'px',

frontend/types.js

@@ -17,17 +17,18 @@ export type Dest = 'firstChild' | 'lastChild' | 'prevSibling' | 'nextSibling' |
 
 export type ElementID = string;
 
+export type Window = {
+  frameElement: DOMNode | null,
+};
+
+export type Document = {
+  defaultView: Window | null,
+};
+
 export type DOMNode = {
   appendChild: (child: DOMNode) => void,
   childNodes: Array<DOMNode>,
-  getBoundingClientRect: () => {
-    top: number,
-    left: number,
-    width: number,
-    height: number,
-    bottom: number,
-    right: number,
-  },
+  getBoundingClientRect: () => DOMRect,
   innerHTML: string,
   innerText: string,
   nodeName: string,
@@ -48,6 +49,7 @@ export type DOMNode = {
   style: Object,
   textContent: string,
   value: string,
+  ownerDocument: Document | null,
 };
 
 export type DOMEvent = {
@@ -61,6 +63,15 @@ export type DOMEvent = {
   target: DOMNode,
 };
 
+export type DOMRect = {
+  top: number,
+  left: number,
+  width: number,
+  height: number,
+  bottom: number,
+  right: number,
+};
+
 export type ControlState = {
   enabled: boolean,
 } & Record;

test/example/sink.js

@@ -136,6 +136,61 @@ class LotsOfMounts extends React.Component {
   }
 }
 
+class IframeWrapper extends React.Component {
+  componentDidMount() {
+    this.root = document.createElement('div');
+    this.frame.contentDocument.body.appendChild(this.root);
+    ReactDOM.render(this.props.children, this.root);
+  }
+
+  componentWillUnmount() {
+    ReactDOM.unmountComponentAtNode(this.root);
+  }
+
+  render() {
+    var { children, ...props } = this.props; // eslint-disable-line no-unused-vars
+
+    return (
+      <div>
+        <div>Iframe below</div>
+        <iframe ref={frame => this.frame = frame} {...props} />
+      </div>
+    );
+  }
+}
+
+class InnerContent extends React.Component {
+  render() {
+    return (
+      <div>
+        Inner content (highlight should overlap properly)
+      </div>
+    );
+  }
+}
+
+class IframeWithMountedChild extends React.Component {
+  render() {
+    return (
+      <IframeWrapper>
+        <InnerContent />
+      </IframeWrapper>
+    );
+  }
+}
+
+class NestedMountedIframesWithVaryingBorder extends React.Component {
+  render() {
+    return (
+      <IframeWrapper>
+        <IframeWrapper frameBorder="0">
+          <InnerContent />
+        </IframeWrapper>
+      </IframeWrapper>
+    );
+  }
+}
+
 // Render the list of tests
 
 class Sink extends React.Component {
@@ -147,6 +202,8 @@ class Sink extends React.Component {
       BadUnmount,
       Nester,
       LotsOfMounts,
+      IframeWithMountedChild,
+      NestedMountedIframesWithVaryingBorder,
     };
 
     var view = Comp => run(View, {Comp});