Last Comment Bug 591718 - getBoundingClientRect needs to take transforms into account
: getBoundingClientRect needs to take transforms into account
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: unspecified
: All All
: P3 normal with 2 votes (vote)
: mozilla12
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
http://marcfuehnen.com/car/offset.htm
: 663852 669887 693501 (view as bug list)
Depends on: 718809 725426
Blocks: 663830 663852 669887 722003
  Show dependency treegraph
 
Reported: 2010-08-28 21:32 PDT by Asa Dotzler [:asa]
Modified: 2012-04-24 06:41 PDT (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
-


Attachments
IE10 screenshot (81.33 KB, image/png)
2011-10-11 18:17 PDT, Andrew Udvare
no flags Details
Simple HTML test case for this bug. (1.30 KB, text/html)
2011-11-23 15:34 PST, Atul Varma [:atul]
no flags Details
cleanup frame transform computation code (15.41 KB, patch)
2011-11-24 20:17 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
part 2: Add support for taking transforms into account in GetAllInFlowRects(Union) (6.13 KB, patch)
2011-11-24 20:18 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
mats: review+
Details | Diff | Splinter Review
part 3: take transforms into account in getClientRects/getBoundingClientRect (6.83 KB, patch)
2011-11-24 20:19 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
Part 1 v2 (15.43 KB, patch)
2011-11-25 11:41 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
mats: review+
Details | Diff | Splinter Review
Part 3 v2 (9.17 KB, patch)
2011-11-25 11:43 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
mats: review+
Details | Diff | Splinter Review
Part 4: fix mobile browser test to handle getBoundingClientRect including the effect of transforms (2.69 KB, patch)
2011-12-28 20:56 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
mark.finkle: review+
Details | Diff | Splinter Review

Description Asa Dotzler [:asa] 2010-08-28 21:32:28 PDT
(from email to evangelism@moz)

I played around with rotation lately and created this
driving game using jquery:

http://marcfuehnen.com/car/ (watch in webkit first ...)

To handle my collision detection I need to rely on correct
offset values after the car has been rotated.

Everything's fine in webkit browsers (you can try it by
bumping into the 'no way' blocks) but in mozilla i guess
there is a problem with the offset values.

I did this experiment:

http://marcfuehnen.com/car/offset.htm

If you choose player and 'Get Offset' you will see that little
red dot marking the rotation's origin, and it's a different value
than shown in webkit browsers.
Comment 1 Boris Zbarsky [:bz] 2010-08-29 05:50:28 PDT
Need a testcase without huge gobs of JS gunk that has browser-specific codepaths...  Without that, hard to tell whether it's our issue or an issue in the gunk.
Comment 2 Boris Zbarsky [:bz] 2010-08-29 06:43:59 PDT
Also, looks like the old parser dropped all tags that were kids of <select> completely, while the new one drops the tags but lets their text kids be kids of the <select>?

If we do address this on the layout side, I'll need to know exactly what I can expect the new content model of <select> to look like.
Comment 3 Boris Zbarsky [:bz] 2010-08-29 06:45:14 PDT
Please ignore comment 2; it was meant for a different bug.
Comment 4 Boris Zbarsky [:bz] 2010-08-29 06:59:12 PDT
Looks like the relevant function ("offset") is:

function (a) {
    var b = this[0];
    if (!b || !b.ownerDocument) {
        return null;
    }
    if (a) {
        return this.each(function (e) {c.offset.setOffset(this, a, e);});
    }
    if (b === b.ownerDocument.body) {
        return c.offset.bodyOffset(b);
    }
    var d = b.getBoundingClientRect(), f = b.ownerDocument;
    b = f.body;
    f = f.documentElement;
    return {top: d.top + 
       (self.pageYOffset || c.support.boxModel && f.scrollTop || b.scrollTop) -
       (f.clientTop ||  b.clientTop || 0),
     left: d.left + 
     (self.pageXOffset || c.support.boxModel && f.scrollLeft || b.scrollLeft) -
     (f.clientLeft || b.clientLeft || 0) };
}

In Gecko, the getBoundingClientRect.top of the rotated rectangle is 244.5.  In webkit it's 250.

The relevant CSS declarations are:

  position:absolute;
  top: 200px;
  left: 225px;
  width: 48px;
  height: 24px;
  -moz-transform: rotate(45deg);
  -moz-transform-origin: left bottom;

and the containing block's bounding client rect is already different in the two browsers: ours has a top at 44.5 px while webkit's is at 43px (which makes sense, since that offset depends on the exact font metrics of the text used for "Content", and the default font seems to be different).

Past that, it looks like we set getBoundingClientRect().top to be 200px + 44.5px, for a total of 244.5px.  Webkit sets it to 200px + 24px - 16.97px + 43px for a total of 250.03, which they round to 250.  In other words, their bounding client rect puts the top at the position of the top-left corner of the rect _after_ the rotation, while ours is at the untransformed position.

I suppose getBoundingClientRect should take transforms into account, right?
Comment 5 ezmilhouse 2010-08-30 06:04:58 PDT
"In other words, their
bounding client rect puts the top at the position of the top-left corner of the
rect _after_ the rotation, while ours is at the untransformed position."

Any suggestions how to fix that mathematically?
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-08-30 06:12:27 PDT
I'm actually not sure. This is un(der)specified. getBoundingClientRect() and getClientRects() expect the element to decompose into axis-aligned rectangles, so maybe it's better to not take transforms into account.
Comment 7 Boris Zbarsky [:bz] 2010-08-30 08:37:10 PDT
Not sure I follow the question in comment 5.  Fix what where?  In a web page, in Gecko, in Webkit?

roc, can we get Anne and the webkit guys to talk to us about this ASAP?  We haven't shipped transforms before, and it would be good to not change this behavior once we do so...
Comment 8 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2010-08-30 08:44:34 PDT
We've been shipping transforms since Firefox 3.5 / Mozilla 1.9.1.
Comment 9 Boris Zbarsky [:bz] 2010-08-30 08:50:44 PDT
Oh, ok.  I guess the recent thing was transitioning transforms....

In that case, I think we should just bring this up in whatever wg deals with this stuff nowadays and get it sorted out.  Maybe what we really need here is a new API to replace getBoundingClientRect(); asking for the "position" of a transformed rectangle isn't really a sensical operation...

(And I'm not sure how one can sanely do hit-testing on transformed elements using it; clearly the testcase in comment 0 is leaving out some key aspect of what's actually going on in the game.)
Comment 10 ezmilhouse 2010-08-31 10:00:18 PDT
I don't know too many use cases where you might be interested in getting the "real" offset of a rectangle after it's been rotated, but in my case it got critical.

I guess when people start using css transforms more often they will also run into this problem. I don't know what's the best solution - the way webkit does it or the way mozilla does it, but I would like to get the same results in both browsers.

Therefore I wrote a little function that gives you the webkit results also in mozilla:

/*
 * MOZILLA OFFSET CORRECTION AFTER ROTATION
 * 
 * offset_x 	| rotation's turning point's offset (x)
 * offset_y 	| rotation's turning point's offset (y)
 * w			| width of rectangle
 * h			| height of rectangle
 * x			| relative distance of turning point to left side of rectangle as in -moz-transform-origin:12px 12px;
 * y			| relative distance of turning point to top side of rectangle as in -moz-transform-origin:12px 12px;
 * degree		| recatangle's rotation (in degree) as in -moz-transform:rotate(60deg);
 * round		| round results true/false
 * 
 * @return: array of all four corner coordinates of the rotated rectangle, starting with the real (webkit-like) offset 
 * (x= box[0][0], y= box[0][1]) and going on clockwise.
 * 
 */

function getOffsetAfterRotation(offset_x, offset_y, w, h, x, y, degree, round){
	
	var rad = -degree * Math.PI * 2.0 / 360.0;
	
	// move rectangle to (0,0) position
	var box = [[0,0],[0,h],[w,h],[w,0]].map(function(p){
		return [p[0]-x,p[1]-y]});

	// rotate rectangle
	function rotate(point){
		return [point[0]*Math.cos(rad) + point[1]*Math.sin(rad),
		        -point[0]*Math.sin(rad) + point[1]*Math.cos(rad)]
	}
	
	box = box.map(rotate);
	
	// return rectangle to turning point position
	box = box.map(function(p){
		return [p[0]+offset_x, p[1]+offset_y]});
	
	if(round){
		return box.map(function(p){return [Math.round(p[0]),
		                                   Math.round(p[1])]});
	} else{
		return box;
	}

}

See it in action here:
http://marcfuehnen.com/hacks/mozilla_offset_correction_after_rotation/

Maybe it helps ...

Thx for your time ...
Comment 11 Boris Zbarsky [:bz] 2010-08-31 10:04:38 PDT
> the "real" offset

Which offset do you mean by that?  Offset from where to where?

> but I would like to get the same results in both browsers.

Of course.  I did start a thread on www-style about this, fwiw.
Comment 12 ezmilhouse 2010-08-31 10:09:13 PDT
"real offset" => by real I mean the visible top left corner of a rotated rectangle, the one that you get with $('#rect').offset() - in moz it's always the same, even after rotation - in webkit it moves on with the rotation, my function gives you the webkit values as a workaround for moz.
Comment 13 Luka Marčetić 2011-01-03 13:58:44 PST
Confirmed in Firefox/3.5.16.
After doing a scale transformation, getBoundingClientRect().left returned an old value (pre-transform).
Comment 14 Luka Marčetić 2011-01-03 14:00:03 PST
(In reply to comment #13)
> Confirmed in Firefox/3.5.16.
> After doing a scale transformation, getBoundingClientRect().left returned an
> old value (pre-transform).

I should add I'm using GNU/Linux on a x86-64 platform.
Comment 15 Michael Ratcliffe [:miker] [:mratcliffe] 2011-01-17 14:31:36 PST
Also from Firebugs perspective our highlighter needs to be able to match the same
shape and position of any element under it and there is currently no way to do
that for hierarchical css transforms because each ancestor can have a different
transform origin. In this situation the offset correction included here would not work.

e.g. If you have a div rotated by 20% that contains a child div that is rotated by 30% then both rotations need to be taken into account and also the transform origins of all parent elements.
Comment 16 Boris Zbarsky [:bz] 2011-08-09 06:14:35 PDT
*** Bug 669887 has been marked as a duplicate of this bug. ***
Comment 17 4esn0k 2011-08-14 12:30:44 PDT
i implement solution to get get the MouseEvent coordinates for an element(in local coordinate system of element) that has CSS3 2d Transforms

i have to implement fix for getBoundingClientRect too
see gist.github.com/1145197 for solution
live demo:
http://jsfiddle.net/YLCd8/2/

solutin is based on iterating on all parents and multiplication of all transformation matrices
Comment 18 Boris Zbarsky [:bz] 2011-10-11 11:33:22 PDT
*** Bug 693501 has been marked as a duplicate of this bug. ***
Comment 19 Andrew Udvare 2011-10-11 18:17:27 PDT
Created attachment 566410 [details]
IE10 screenshot

IE 10 also adds translate values.
Comment 20 Atul Varma [:atul] 2011-11-23 15:34:39 PST
Created attachment 576636 [details]
Simple HTML test case for this bug.

I stumbled upon this bug independently and wrote this simple "test case" which runs as expected on Webkit and Opera (possibly IE9 too, but I don't have it on hand) but doesn't on Firefox. I hope it's somehow useful.
Comment 21 Paul Rouget [:paul] 2011-11-24 06:39:35 PST
We need this for the devtools.
Comment 22 Atul Varma [:atul] 2011-11-24 10:42:50 PST
I just tried my test case out on IE9 and it works there too, so Firefox is the only major browser that it doesn't work on. That's not to say that Firefox is wrong, of course, but it sure would be easy for Web developers if this function worked the same way across every browser.
Comment 23 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-11-24 16:35:56 PST
roc posted http://lists.w3.org/Archives/Public/www-style/2011Nov/0629.html
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-24 20:17:04 PST
Created attachment 576865 [details] [diff] [review]
cleanup frame transform computation code
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-24 20:18:16 PST
Created attachment 576866 [details] [diff] [review]
part 2: Add support for taking transforms into account in GetAllInFlowRects(Union)
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-24 20:19:22 PST
Created attachment 576867 [details] [diff] [review]
part 3: take transforms into account in getClientRects/getBoundingClientRect

Includes tests.
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-25 00:17:25 PST
Try shows some test failures:
1257 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug677878.html | Test timed out.
3538 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_selection_expanding.html | The contents of fixedDiv1 aren't selected (div1-fixedDiv1, all boxes are overflow: visible;): Selected String: "aaa aaaaaaa aaaaaaa\naaaaaaa aaaaaaa aaaaaaa aaaaaaa
(and other failures in test_selection_expanding.html)
16497 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_mousecapture.xul | setCapture on body retargets to root node mousemove event fired - got false, expected true
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-25 11:41:44 PST
Created attachment 576976 [details] [diff] [review]
Part 1 v2

nsIFrame::GetTransformMatrix had a bug that meant it wouldn't stop correctly when aStopAtAncestor was this frame's parent. Fixing that fixed most of the test failures.
Comment 29 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-25 11:43:36 PST
Created attachment 576977 [details] [diff] [review]
Part 3 v2

Added more tests to test_clientRects.html.

Fixed test_bug677878.html which relied on getBoundingClientRect not taking transforms into account.
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-25 11:44:14 PST
These patches pass debug tests on try.
Comment 31 Mats Palmgren (vacation) 2011-12-01 06:37:07 PST
Comment on attachment 576976 [details] [diff] [review]
Part 1 v2

A few nits - fix as you see fit.  r=mats

>layout/base/nsLayoutUtils.cpp
>+GetTransformToAncestor( ...
>+    ctm = ctm*parent->GetTransformMatrix(aAncestor, &parent);

add spaces around "*"

>layout/base/nsLayoutUtils.h
>+   * Transform aRect relative to aAncestor down to the coordinate system of aFrame.

line has > 80 cols

>+   * Transform aRect relative to aFrame up to the coordinate system of aAncestor.

line has > 80 cols

>layout/forms/nsComboboxControlFrame.cpp
>     nsIFrame* parent = nsnull;
>-    gfx3DMatrix ctm = frame->GetTransformMatrix(&parent);
>+    gfx3DMatrix ctm = frame->GetTransformMatrix(nsnull, &parent);

while you are here, remove the initialization of 'parent'

>layout/generic/nsFrame.cpp
>+nsIFrame::GetTransformMatrix(nsIFrame* aStopAtAncestor, nsIFrame **aOutAncestor)

I would prefer 'nsIFrame** aOutAncestor'

>layout/generic/nsIFrame.h
>+   * @param aStopAtAncestor don't look further than aStopAtAncestor.

please say explicitly that it looks at all ancestors when null

>+  virtual gfx3DMatrix GetTransformMatrix(nsIFrame* aStopAtAncestor, nsIFrame **aOutAncestor);

I would prefer 'nsIFrame** aOutAncestor'
line has > 80 cols

>layout/svg/base/src/nsSVGForeignObjectFrame.cpp
>+nsSVGForeignObjectFrame::GetTransformMatrix(nsIFrame* aAncestor, nsIFrame **aOutAncestor)

I would prefer 'nsIFrame** aOutAncestor'
line has > 80 cols

>layout/svg/base/src/nsSVGForeignObjectFrame.h
>+  virtual gfx3DMatrix GetTransformMatrix(nsIFrame* aAncestor, nsIFrame **aOutAncestor);

I would prefer 'nsIFrame** aOutAncestor'
line has > 80 cols
Comment 32 Mats Palmgren (vacation) 2011-12-01 06:38:40 PST
Comment on attachment 576866 [details] [diff] [review]
part 2: Add support for taking transforms into account in GetAllInFlowRects(Union)

> layout/base/nsLayoutUtils.cpp
> nsIFrame* nsLayoutUtils::GetContainingBlockForClientRect(nsIFrame* aFrame)
> {
>-  // get the nearest enclosing SVG foreign object frame or the root frame

https://developer.mozilla.org/en/DOM/element.getClientRects
", unless the element is inside an SVG foreignobject element, in which case the top-left is relative to the nearest foreignobject ancestor and in the coordinate system of that foreignobject"
needs to be removed, and add a note about this bug in older versions?
Comment 33 Mats Palmgren (vacation) 2011-12-01 06:39:19 PST
Comment on attachment 576977 [details] [diff] [review]
Part 3 v2

> dom/tests/mochitest/general/test_clientRects.html

this file has DOS line endings
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-15 02:19:04 PST
I'm planning to land this after the Dec 20 branch to ensure we get maximum coverage, since this could possibly break sites.
Comment 36 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-27 19:43:13 PST
(In reply to Mats Palmgren [:mats] from comment #32)
> https://developer.mozilla.org/en/DOM/element.getClientRects
> ", unless the element is inside an SVG foreignobject element, in which case
> the top-left is relative to the nearest foreignobject ancestor and in the
> coordinate system of that foreignobject"
> needs to be removed, and add a note about this bug in older versions?

I removed the text, but didn't add a note about the bug. I expect no-one will actually run into it.
Comment 37 Phil Ringnalda (:philor, back in August) 2011-12-27 20:50:45 PST
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d07146e9c62e because the wily mobile/xul/chrome/tests/browser_scrollbar.js didn't like it.
Comment 38 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-27 21:04:09 PST
The vertical scrollbar can be transformed by _updateScrollbars in browser.js. Simplest fix is hopefully to check for a translate() transform on the scrollbar and include any X-translation in the expected getBoundingClientRect value.

Try push with hypothetical test fix:
https://tbpl.mozilla.org/?tree=Try&rev=812bee53c58d
Comment 39 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-28 01:18:08 PST
Hmm, running browser-chrome alone didn't seem to work at all (lots of "Browser undefined" errors on many tests). Retrying with a bigger set of Android tests:
https://tbpl.mozilla.org/?tree=Try&rev=02d99bc76219
Comment 40 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-28 20:56:14 PST
Created attachment 584703 [details] [diff] [review]
Part 4: fix mobile browser test to handle getBoundingClientRect including the effect of transforms
Comment 43 Paul Rouget [:paul] 2012-01-03 03:24:45 PST
*** Bug 663852 has been marked as a duplicate of this bug. ***
Comment 44 Eric Shepherd [:sheppy] 2012-04-24 06:41:05 PDT
Documentation updated:

https://developer.mozilla.org/en/DOM/element.getBoundingClientRect

Mentioned on Firefox 12 for developers.

Note You need to log in before you can comment on or make changes to this bug.