The default bug view has changed. See this FAQ.

getBoundingClientRect needs to take transforms into account

RESOLVED FIXED in mozilla12

Status

()

Core
DOM: CSS Object Model
P3
normal
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: asa, Assigned: roc)

Tracking

({dev-doc-complete})

unspecified
mozilla12
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 wontfix, blocking2.0 -)

Details

(URL)

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
(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.
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.
Keywords: qablocker, qawanted
Keywords: qablocker → testcase-wanted
blocking2.0: --- → ?
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.
Please ignore comment 2; it was meant for a different bug.
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?
Keywords: qawanted, testcase-wanted
Summary: Offset not accurate after rotation with -moz-transform → getBoundingClientRect needs to take transforms into account
Component: Style System (CSS) → DOM
QA Contact: style-system → general

Comment 5

7 years ago
"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?
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.
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...
We've been shipping transforms since Firefox 3.5 / Mozilla 1.9.1.
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

7 years ago
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 ...
> 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

7 years ago
"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.
blocking2.0: ? → -

Comment 13

6 years ago
Confirmed in Firefox/3.5.16.
After doing a scale transformation, getBoundingClientRect().left returned an old value (pre-transform).

Comment 14

6 years ago
(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.
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.
Blocks: 669887
Duplicate of this bug: 669887

Comment 17

6 years ago
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
Duplicate of this bug: 693501

Comment 19

6 years ago
Created attachment 566410 [details]
IE10 screenshot

IE 10 also adds translate values.
Component: DOM → DOM: CSS Object Model
QA Contact: general → style-system
OS: Windows 7 → All
Priority: -- → P3
Hardware: x86 → All

Comment 20

5 years ago
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

5 years ago
We need this for the devtools.
Blocks: 663852, 663830

Comment 22

5 years ago
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.
roc posted http://lists.w3.org/Archives/Public/www-style/2011Nov/0629.html
Assignee: nobody → roc
Created attachment 576865 [details] [diff] [review]
cleanup frame transform computation code
Attachment #576865 - Flags: review?(matspal)
Created attachment 576866 [details] [diff] [review]
part 2: Add support for taking transforms into account in GetAllInFlowRects(Union)
Attachment #576866 - Flags: review?(matspal)
Created attachment 576867 [details] [diff] [review]
part 3: take transforms into account in getClientRects/getBoundingClientRect

Includes tests.
Attachment #576867 - Flags: review?(matspal)
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
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.
Attachment #576865 - Attachment is obsolete: true
Attachment #576976 - Flags: review?(matspal)
Attachment #576865 - Flags: review?(matspal)
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.
Attachment #576867 - Attachment is obsolete: true
Attachment #576977 - Flags: review?(matspal)
Attachment #576867 - Flags: review?(matspal)
These patches pass debug tests on try.
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
Attachment #576976 - Flags: review?(matspal) → review+
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?
Attachment #576866 - Flags: review?(matspal) → review+
Comment on attachment 576977 [details] [diff] [review]
Part 3 v2

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

this file has DOS line endings
Attachment #576977 - Flags: review?(matspal) → review+
I'm planning to land this after the Dec 20 branch to ensure we get maximum coverage, since this could possibly break sites.
Fixed all nits.

https://hg.mozilla.org/integration/mozilla-inbound/rev/10ea92a6a850
https://hg.mozilla.org/integration/mozilla-inbound/rev/f793f9cfa72c
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc7c7734ec7c
(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.
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.
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
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
Created attachment 584703 [details] [diff] [review]
Part 4: fix mobile browser test to handle getBoundingClientRect including the effect of transforms
Attachment #584703 - Flags: review?(mark.finkle)
Attachment #584703 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/61ee01b6a46d
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8519c842f6e
https://hg.mozilla.org/integration/mozilla-inbound/rev/baa4388f9cea
https://hg.mozilla.org/integration/mozilla-inbound/rev/24bbd63d12d2
https://hg.mozilla.org/mozilla-central/rev/61ee01b6a46d
https://hg.mozilla.org/mozilla-central/rev/a8519c842f6e
https://hg.mozilla.org/mozilla-central/rev/baa4388f9cea
https://hg.mozilla.org/mozilla-central/rev/24bbd63d12d2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12

Updated

5 years ago
Duplicate of this bug: 663852
Depends on: 718809

Updated

5 years ago
Blocks: 722003
status-firefox11: --- → wontfix
Depends on: 725426
Keywords: dev-doc-needed
Documentation updated:

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

Mentioned on Firefox 12 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.