From 50995502c6425fbe324743f301a456ff1f03286c Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Thu, 19 Jan 2012 17:13:08 +0200 Subject: [PATCH] Bug 719049 - Sometimes iframes xy positions aren't calculated correctly in Tilt; r=rcampbell --- browser/devtools/shared/LayoutHelpers.jsm | 50 ++++++++ browser/devtools/tilt/TiltUtils.jsm | 107 +----------------- .../tilt/test/browser_tilt_utils05.js | 17 +-- 3 files changed, 63 insertions(+), 111 deletions(-) diff --git a/browser/devtools/shared/LayoutHelpers.jsm b/browser/devtools/shared/LayoutHelpers.jsm index c5523985e7e3..a166fe50cfae 100644 --- a/browser/devtools/shared/LayoutHelpers.jsm +++ b/browser/devtools/shared/LayoutHelpers.jsm @@ -118,6 +118,51 @@ LayoutHelpers = { return rect; }, + /** + * Compute the absolute position and the dimensions of a node, relativalely + * to the root window. + * + * @param nsIDOMNode aNode + * a DOM element to get the bounds for + * @param nsIWindow aContentWindow + * the content window holding the node + */ + getRect: function LH_getRect(aNode, aContentWindow) { + let frameWin = aNode.ownerDocument.defaultView; + let clientRect = aNode.getBoundingClientRect(); + + // Go up in the tree of frames to determine the correct rectangle. + // clientRect is read-only, we need to be able to change properties. + rect = {top: clientRect.top + aContentWindow.pageYOffset, + left: clientRect.left + aContentWindow.pageXOffset, + width: clientRect.width, + height: clientRect.height}; + + // We iterate through all the parent windows. + while (true) { + + // Are we in the top-level window? + if (frameWin.parent === frameWin || !frameWin.frameElement) { + break; + } + + // We are in an iframe. + // We take into account the parent iframe position and its + // offset (borders and padding). + let frameRect = frameWin.frameElement.getBoundingClientRect(); + + let [offsetTop, offsetLeft] = + this.getIframeContentOffset(frameWin.frameElement); + + rect.top += frameRect.top + offsetTop; + rect.left += frameRect.left + offsetLeft; + + frameWin = frameWin.parent; + } + + return rect; + }, + /** * Returns iframe content offset (iframe border + padding). * Note: this function shouldn't need to exist, had the platform provided a @@ -135,6 +180,11 @@ LayoutHelpers = { getIframeContentOffset: function LH_getIframeContentOffset(aIframe) { let style = aIframe.contentWindow.getComputedStyle(aIframe, null); + // In some cases, the computed style is null + if (!style) { + return [0, 0]; + } + let paddingTop = parseInt(style.getPropertyValue("padding-top")); let paddingLeft = parseInt(style.getPropertyValue("padding-left")); diff --git a/browser/devtools/tilt/TiltUtils.jsm b/browser/devtools/tilt/TiltUtils.jsm index b90286e27679..d1469a477c08 100644 --- a/browser/devtools/tilt/TiltUtils.jsm +++ b/browser/devtools/tilt/TiltUtils.jsm @@ -37,7 +37,7 @@ * ***** END LICENSE BLOCK *****/ -/*global Components, Services, XPCOMUtils */ +/*global Components, Services, XPCOMUtils, LayoutHelpers */ "use strict"; const Cc = Components.classes; @@ -46,6 +46,7 @@ const Cu = Components.utils; Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource:///modules/devtools/LayoutHelpers.jsm"); let EXPORTED_SYMBOLS = ["TiltUtils"]; @@ -402,108 +403,6 @@ TiltUtils.DOM = { }; }, - /** - * Returns the absolute x, y, width and height coordinates of a node, or null - * if the passed node is not an ELEMENT_NODE. - * - * @param {Element} aNode - * the node which coordinates need to be calculated - * @param {Window} aContentWindow - * optional, the window content holding the the document - * - * @return {Object} an object containing the top, left, width, height coords - */ - getNodeCoordinates: function TUD_getNodeCoordinates(aNode, aContentWindow) { - // make sure the contentWindow parameter is a valid object - aContentWindow = aContentWindow || {}; - - if (aNode.nodeType !== 1) { // Node.ELEMENT_NODE - return null; - } - - let rect = { - top: 0, - left: 0, - width: 0, - height: 0 - }; - - // the preferred way of getting the bounding client rectangle - let clientRect = aNode.getBoundingClientRect(); - rect.top = clientRect.top + aContentWindow.pageYOffset; - rect.left = clientRect.left + aContentWindow.pageXOffset; - rect.width = clientRect.width; - rect.height = clientRect.height; - - // compute the iframe position and its offset if necessary - let frameRect = this.getFrameOffset( - aNode.ownerDocument.defaultView.frameElement, aContentWindow); - - if (frameRect) { - rect.top += frameRect.top; - rect.left += frameRect.left; - } - - return rect; - }, - - /** - * Retuns the parent iframe position and its offset (borders and padding), - * or null if the passed frame is not valid. - * - * @param {Element} aNode - * the iframe which offset need to be calculated - * @param {Window} aContentWindow - * optional, the window content holding the the document - * - * @return {Object} an object containing the top and left coords - */ - getFrameOffset: (function() { - let cache = {}; - - return function TUD_getFrameOffset(aFrame, aContentWindow) { - // make sure the contentWindow parameter is a valid object - aContentWindow = aContentWindow || {}; - - if (!aFrame) { - return null; - } - - let id = TiltUtils.getWindowId(aFrame.contentWindow) + "," + - aContentWindow.pageXOffset || 0 + "," + - aContentWindow.pageYOffset || 0; - - // check the cache to see if this iframe offset wasn't calculated already - if (cache[id] !== undefined) { - return cache[id]; - } - - let offset = { - top: 0, - left: 0 - }; - - // take the parent iframe bounding rect position into account - let frameRect = aFrame.getBoundingClientRect(); - offset.top = frameRect.top; - offset.left = frameRect.left; - - // compute the iframe content offset (iframe border + padding) - // bug #626359 - let style = aFrame.contentWindow.getComputedStyle(aFrame, null); - if (style) { - offset.top += - parseInt(style.getPropertyValue("padding-top")) + - parseInt(style.getPropertyValue("border-top-width")); - offset.left += - parseInt(style.getPropertyValue("padding-left")) + - parseInt(style.getPropertyValue("border-left-width")); - } - - return (cache[id] = offset); - }; - }()), - /** * Traverses a document object model & calculates useful info for each node. * @@ -549,7 +448,7 @@ TiltUtils.DOM = { } // get the x, y, width and height coordinates of the node - let coord = this.getNodeCoordinates(node, aContentWindow); + let coord = LayoutHelpers.getRect(node, aContentWindow); if (!coord) { continue; } diff --git a/browser/devtools/tilt/test/browser_tilt_utils05.js b/browser/devtools/tilt/test/browser_tilt_utils05.js index cff28d4da476..858ae6f31a93 100644 --- a/browser/devtools/tilt/test/browser_tilt_utils05.js +++ b/browser/devtools/tilt/test/browser_tilt_utils05.js @@ -1,9 +1,12 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -/*global ok, is, waitForExplicitFinish, finish, gBrowser, TiltUtils */ +/*global ok, is, waitForExplicitFinish, finish, gBrowser */ +/*global Cu, LayoutHelpers, TiltUtils */ "use strict"; +Cu.import("resource:///modules/devtools/LayoutHelpers.jsm"); + function init(callback) { let iframe = gBrowser.ownerDocument.createElement("iframe"); @@ -55,15 +58,15 @@ function test() { iframe.contentWindow.innerHeight, "The content window height wasn't calculated correctly."); - - let nodeCoordinates = dom.getNodeCoordinates( + let nodeCoordinates = LayoutHelpers.getRect( iframe.contentDocument.getElementById("test-div"), iframe.contentWindow); + + let frameOffset = LayoutHelpers.getIframeContentOffset(iframe); + let frameRect = iframe.getBoundingClientRect(); - let frameOffset = dom.getFrameOffset(iframe); - - is(nodeCoordinates.top, frameOffset.top + 98, + is(nodeCoordinates.top, frameRect.top + frameOffset[0] + 98, "The node coordinates top value wasn't calculated correctly."); - is(nodeCoordinates.left, frameOffset.left + 76, + is(nodeCoordinates.left, frameRect.left + frameOffset[1] + 76, "The node coordinates left value wasn't calculated correctly."); is(nodeCoordinates.width, 123, "The node coordinates width value wasn't calculated correctly.");