Last Comment Bug 174397 - Support for getClientRects and getBoundingClientRect
: Support for getClientRects and getBoundingClientRect
Status: RESOLVED FIXED
: dev-doc-complete, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- enhancement with 2 votes (vote)
: mozilla1.9alpha5
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
: Hixie (not reading bugmail)
Mentors:
: 323078 (view as bug list)
Depends on: 441258 344206 392671 396694 487597
Blocks: 343032 364612 340571
  Show dependency treegraph
 
Reported: 2002-10-14 11:49 PDT by Erik Arvidsson
Modified: 2013-04-04 13:53 PDT (History)
29 users (show)
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Move the mouse around and notive statusbar and the dotted borders (8.71 KB, text/html)
2002-10-14 15:45 PDT, Erik Arvidsson
no flags Details
Another test case that shows some interesting relations (2.49 KB, text/html)
2002-10-14 18:44 PDT, Erik Arvidsson
no flags Details
getBoundingClientRect/offsetLeft/offsetTop tests (5.20 KB, text/html)
2006-06-08 19:34 PDT, Beau Hartshorne
no flags Details
getBoundingClientRect/offsetLeft/offsetTop tests (4.89 KB, text/html)
2006-06-09 12:47 PDT, Beau Hartshorne
no flags Details
first cut patch (29.68 KB, patch)
2006-06-19 23:03 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Review
updated patch (46.71 KB, patch)
2006-07-10 23:10 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
jonas: review+
dbaron: review+
tor: superreview+
Details | Diff | Review
updated to trunk (44.99 KB, patch)
2007-05-22 01:41 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Review
update (35.91 KB, patch)
2007-05-23 15:34 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Review

Description Erik Arvidsson 2002-10-14 11:49:13 PDT
http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/getclientrects.asp
http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/getboundingclientrect.asp

These two methods return a TextRectangle object which describes the size and
position of the element relative to the client area. Currently the bounding
rectangle can be found but it is not very straight forward and it is very error
prone. This information is already available inside the layout engine and it
would make sense to provide interfaces for these to scripting.

The client rects for inline elements that wrap is today not available to scripting
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2002-10-14 13:48:07 PDT
Over to the right component. This _could_ be done, except for one little problem
-- the MSDN descriptions are so vague as to be useless.  Do these rects include
margins?  Borders?  Padding?  How does this interact with positioning?  With
floating?  What are the correct values for table elements in the collapsing
border table model?

Without either documentation or a clear and exhaustive series of testcases which
access the property and say what it _should_ be (what IE returns) in addition to
having the test for what it _is_, it's a little hard to do any real work on this
bug.
Comment 2 Erik Arvidsson 2002-10-14 14:42:11 PDT
The rect used is the border-box relative to the client viewport

I'll be willing to create the needed test cases.
Comment 3 Erik Arvidsson 2002-10-14 15:45:31 PDT
Created attachment 102865 [details]
Move the mouse around and notive statusbar and the dotted borders

This test case when run in IE places an element over the target (element, not
text nodes). Notice that to do this in Mozilla would require loops to sum up
offsetsLeft, scrollLeft, paddingLeft, BorderLeftWidth for all ancestors. Not
very efficient and very error prone.

The red dotted border is for the getBoundingClientRect and the black dottet
borders are for getClientRects. The client rects information is not available
in Mozilla.

Implementing getBoundingClientRect should be trivial. The other one is a bit
trickier.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2002-10-14 16:56:32 PDT
Actually, the other is likely a lot more trivial...

How should this interact with -moz-box-sizing?  Or does that not matter?  How
does it interact with overflowing content (eg offsetWidth in IE _includes_ the
overflowing elements, whereas the CSS border-box most obviously does not).
Comment 5 Erik Arvidsson 2002-10-14 18:41:06 PDT
-moz-box-sizing does not pose a problem. The BoundingClientRect is always the
outer border box.

The overflow issue in the case of overflow being set to visible is of course a
problem, just like it is a problem for offsetWith/Height. I think you should be
consistent and use the same logic here as well. That is to ignore the overflown
content and use the outer border edge.

For all elements the following should be true:

el.getBoundingClientRect().right - el.getBoundingClientRect().left = el.offsetWidth
el.getBoundingClientRect().bottom - el.getBoundingClientRect().top = el.offsetHeight

I'll attach another test case that shows that this is always true.


Notice also that getBoundingClientRect can be calcualted directly from
getClientRects by taking the min/max values for all the rects. This will also be
shown in the upcoming attachement.
Comment 6 Erik Arvidsson 2002-10-14 18:44:41 PDT
Created attachment 102883 [details]
Another test case that shows some interesting relations

Resize the browser window so that the text with red background wraps. Click on
elements to alert offsetWidth/Height and getBoundingClientRect

This test case shows that getBoundingClientRect can be calculated from
getClientRects. It also shows the relationship between getBoundingClientRect
and offsetWidth/Height.

(it also shows a but in Mozilla implementation of offsetWidth/Height... I'll
see if I can find the related bug or file a new one.)
Comment 7 Erik Arvidsson 2002-12-19 13:40:40 PST
I just found out about document.getBoxObjectFor(oElement). This seems to allow
me to find the position of an element without relying on recursive functions
using offset* and scroll*. This practically removes the need for
getBoundingClientRect.
Comment 8 David Akdikmen 2002-12-19 15:44:32 PST
Hi, Just tested document.getBoxObjectFor(oElement), and it doesn't work for 
#text nodes. We need to get the rects for a selection of text that my be 
wrapping inside a middle of <P> element (for example).  That's what 
getBoundingClientRect does for us..
Comment 9 David Akdikmen 2002-12-19 15:48:49 PST
Oops, I meant getClientRects() get the rects for wrapped text, in last post.
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2002-12-20 07:51:46 PST
I wouldn't rely on getBoxObjectFor too much. It's designed for XUL; its 
interaction with the CSS box model is rather undefined.  And it may be getting 
moved to XULDocument, in which case it would just not be available in HTML.
Comment 11 Erik Arvidsson 2002-12-20 08:24:57 PST
That was what I was afraid of. These properties are needed in all Documents with
a view, not only some certain flavors of XML.

I can see a few possible ways to go:

1. Keep getBoxObjectFor for all Documents (or make boxObject a property of all
Elements and not only XULElements)

2. Implement Element getBoundingClientRect. Maybe this is better because it will
make it compatible with IE and will hide the other methods and properties
visible in the BoxObject interface which may or may not be desired to be public
in all kinds of Document views.

3. Introduce something entirely new that is part of the view. I thought this was
the initial intention for the W3C DOM Views but that part of the DOM has totally
failed. Maybe in level 4? But the world cannot wait that long.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2003-03-23 13:09:39 PST
Mass-reassigning bugs to dom_bugs@netscape.com
Comment 13 Brad Fults 2004-11-09 16:21:30 PST
"But the world cannot wait that long."

Ironic, as that was posted 23 months ago. Is there a specific reason that this
functionality can't be added in? The comments seem inconclusive. I think this
bug deserves a revisitation.
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2004-11-09 17:36:54 PST
> Is there a specific reason that this functionality can't be added in?

Lack of a clear concept of what we want exactly, coupled with lack of time to
clearly spec this out and then implement it.

If someone has time to work on this, they should go for it.
Comment 15 Erik Arvidsson 2004-11-10 00:55:18 PST
(In reply to comment #13)
> "But the world cannot wait that long."
> 
> Ironic, as that was posted 23 months ago. Is there a specific reason that this
> functionality can't be added in? The comments seem inconclusive. I think this
> bug deserves a revisitation.

I agree that it is ironic. During this time document.getBoxObjectFor has become
the defacto way for doing this in Mozilla (or recursive solutions that always
seem to fail in some cases). So I guess the world did not stop dead ;-)
Comment 16 Darin Fisher 2005-05-17 16:44:01 PDT
Any chance of this bug being fixed for Gecko 1.8?
Comment 17 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-05-17 19:23:59 PDT
Darin, see comment 1.  Most of what I said in it still stands -- if someone can
clearly describe what needs to be implemented we could implement it, but I,
personally, am not willing to implement something different from IE and then
spend forever fixing it up one hack at a time...
Comment 18 Darin Fisher 2005-05-17 19:46:52 PDT
bz: that sounds reasonable.
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-01-11 19:12:29 PST
*** Bug 323078 has been marked as a duplicate of this bug. ***
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 13:05:38 PDT
My latest thinking on this, after chatting with one of the Mochikit people, is that implementing getBoundingClientRect is not a good idea. The IE implementation has some nasty quirks which we don't want to emulate (as well as being poorly specified), and if we're not going to emulate them, we might as well call our API something else.

Hopefully this person is going to propose an API with a solid spec to WHATWG; then we can implement that.
Comment 21 Hixie (not reading bugmail) 2006-06-08 14:47:46 PDT
I can fix "poorly specified"; can you expand on "nasty quirks"?
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 15:14:17 PDT
http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/getboundingclientrect.asp

"In Microsoft Internet Explorer 5, the window's upper-left is at 2,2 (pixels) with respect to the true client."

I'm not even sure what that means, but it can't be good :-)
Comment 23 Hixie (not reading bugmail) 2006-06-08 16:32:30 PDT
If that's it, then that's not enough to invent a whole new API, IMHO.
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 17:06:02 PDT
Beau, is the mysterious 2px offset the only problem, or are there other problems? I got the impression there were other problems, but I could have the wrong impression.
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 17:08:58 PDT
Note, apparently whether there is a 2px offset or not depends on quirks vs standards mode.
Comment 26 Hixie (not reading bugmail) 2006-06-08 17:10:05 PDT
That's even better, it means we can implement it without feeling bad. :-)
Comment 27 Beau Hartshorne 2006-06-08 19:33:20 PDT
IE inserts the (2,2) offset in standards mode, not quirks mode. It's easy to work around. They set document.documentElement.clientLeft and document.documentElement.clientTop to 2 in standards mode, or 0 in quirks mode.

From limited testing, IE's getBoundingClientRect() seems to return the same values as a recursive offsetLeft + clientLeft summation. They even share a bug where the left padding is included in a calculation, but the top padding is not.

IE's offsetLeft/offsetTop are a real mess of hasLayout and who knows what else. If getBoundingClientRect() is just a different interface to the same mess, I don't see how trying to clone that in Mozilla will help. IE, Mozilla, WebKit, and Opera return different summations for an object positioned on a test page I built (and will attach).

I think Mozilla should implement something new and correct, and leave it to the JS libraries to work around IE's bugs. Either way, let's just get this right.
Comment 28 Beau Hartshorne 2006-06-08 19:34:45 PDT
Created attachment 224960 [details]
getBoundingClientRect/offsetLeft/offsetTop tests
Comment 29 Hixie (not reading bugmail) 2006-06-08 19:52:34 PDT
The problem with anything "new and correct" is that it would be just as underdefined unless someone actually defined it, and defining it is hard. We're going to have to define offsetTop/getClientRects anyway, so we might as well define them sensibly and implement them the same in all modern browsers, instead of inventing yet another set of methods.

Anne has worked on defining offsetTop, etc, maybe he knows how it should work in enough detail to spec it?
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 20:24:35 PDT
The version of Anne's spec that I saw was very complicated, as it tried to capture a lot of quirks.

If we define offset* "sensibly", then it won't be anything like what any browser currently implements, so why give it the same name and break people who are currently using it (or force them to add browser version tests instead of just a feature test).
Comment 31 Hixie (not reading bugmail) 2006-06-08 20:28:29 PDT
I would imagine that anything we implement would be very complicated, even without quirks.
Comment 32 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 21:12:05 PDT
http://dump.testsuite.org/2006/dom/style/offset/spec
has a spec proposal for the offset* properties (written by Anne among others).

[A few more issues that need to be addressed there:
-- what about elements that generate multiple CSS boxes (e.g. broken inlines)?
-- overflow can affect offset*, because overflow affects whether scrollbars are present and hence layout; the text needs to be clarified
-- table collapsed borders, are the cells treated as having overlapping border boxes?
-- what about SVG and content inside a transformed (rotated) foreignobject?]

In comment #30 I jumped the gun a bit. If we define the "client rects" of an element in terms of the sum of the offset* properties (reinterpreted as to apply to each of the element's CSS boxes) from the element all the way up the offsetParent chain, then a lot of the offset* quirks become irrelevant because it doesn't really matter how offsetParent is chosen. In fact I think it would work quite well except for one thing: elements with a null offsetParent have their offsetX/Y relative to the canvas origin if they are fixed-position, but the initial containing block origin otherwise. This means that the client rect(s) for an element would have a different reference point depending on whether the element is in a fixed-pos container or not, which would be very bad.

Perhaps we could fix that by having the offset* spec simply treat position:fixed the same as other positions and returned offsets relative to the initial containing block origin. position:fixed isn't used much on the Web yet.
Comment 33 Hixie (not reading bugmail) 2006-06-08 21:19:02 PDT
The initial containing block is defined as being a rectangle with the dimensions of the viewport, anchored at the canvas origin. So there should be no difference there (maybe apart from the 2,2 weirdness mentioned earlier).
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-08 21:27:09 PDT
The 2,2 weirdness is simply inconsistent with this spec. I'm happy to ignore it :-).

Assuming you're right, then that spec should be amended to make it clear there is no difference.

Then defining getBoundingClientRect in terms of the offset* properties basically amounts to "the offset of the top-left of the border-box from the origin of the canvas, assuming everything scrolled to the default position". Which is what I was aiming for all along :-).
Comment 35 Anne (:annevk) 2006-06-09 03:49:18 PDT
My specification doesn't try to capture many quirks actually. If it did, it would require changes to the CSS box model and would require browsers to implement hasLayout. I think it's more sane than Mozilla's current implementation actually. The part after "Notes" is to be ignored. FWIW, that specification is here:

  http://dump.testsuite.org/2006/dom/style/offset/spec

If you want the reason for various design choices, feel free to ask. It would be great to get this the same between Opera, Mozilla and Safari (and others).
Comment 36 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-09 04:43:57 PDT
What about comment #33?
Comment 37 Anne (:annevk) 2006-06-09 04:48:05 PDT
(In reply to comment #32)
> -- what about elements that generate multiple CSS boxes (e.g. broken inlines)?

Haven't gotten around to that yet. I'm open to suggestions.


> -- overflow can affect offset*, because overflow affects whether scrollbars
> are present and hence layout; the text needs to be clarified

I should probably state this differently, but the intend was to say what you said in comment 34.


> -- table collapsed borders, are the cells treated as having overlapping
> border boxes?

Open to suggestions. Might also be good to test IE for this. (Can't really test that at the moment.)


> -- what about SVG and content inside a transformed (rotated) foreignobject?]

SVG elements don't implement HTMLElement so they don't have to work wit this model. I guess we could move the properties to Element instead and define it for all possible layout cases... Not sure about transformed content.


I think comment 33 can be implemented as stated given that you don't take scrolling into account ("everything scrolled to its default position," as you said). Will make these modifications either this weekend or early next week.
Comment 38 Beau Hartshorne 2006-06-09 12:47:26 PDT
Created attachment 225048 [details]
getBoundingClientRect/offsetLeft/offsetTop tests

This test shows which dimensions are included in Mozilla’s getBoxObjectFor(), IE’s getBoundingClientRect(), and by recursively summing offset and (client || 0). getBoundingClientRect() seems to include different measurements than the summation. (div#one’s margin is counted in offset *and* client.)
Comment 39 Anne (:annevk) 2006-06-12 02:33:35 PDT
Clarified the specification a bit with regard to some of the comments. For inline elements that generate multiple boxes you take the first box and calculate from there. That seems to be what browsers do at the moment.
Comment 40 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-12 02:40:21 PDT
Is that "first" in content order or visual order (for bidi-reordered content)?

The question about SVG is, if I have a situation where an SVG foreignobject element contains an HTML element, and I call offsetX and offsetY on that HTML element, what are the results?
Comment 41 Anne (:annevk) 2006-06-12 04:22:11 PDT
I tried to make a test using RTL...

  http://dump.testsuite.org/2006/dom/style/offset/049.htm

  Browser      offsetParent  offsetLeft

  F 1.5        BODY          64
  IE 7         DIV           16
  O 9          HTML          48

Given the result from IE I'd say you have to take the first in visual order. I'm not entirely sure about this though. I haven't really checked how CSS works together with RTL etc.


Regarding the SVG stuff, I can't really tell from the SVG specification how they expect that to work. For <svg:foreignobject xlink:href=""> it's quite clear I guess, but when you use HTML inline it's unclear to me what should happen. Does it create a new viewport or do you expect to be able to overlay the SVG content with CSS styled elements etc?
Comment 42 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-12 15:48:14 PDT
That's all poorly defined right now for the inline HTML case. The behaviour of position:fixed, background-attachment:fixed etc is completely undefined. Establishing a new viewport is probably the most reasonable thing to do. We probably don't want to try to resolve that as part of the offset* spec, but I think it should be listed as an issue.

I don't really care how you resolve the RTL issues as long as you do :-). We can handle visual order or content order with approximately equal ease. Maybe the best thing would be to think about what Web authors would most want --- probably visual order.
Comment 43 Erik Arvidsson 2006-06-12 16:55:04 PDT
(In reply to comment #39)
> Clarified the specification a bit with regard to some of the comments. For
> inline elements that generate multiple boxes you take the first box and
> calculate from there. That seems to be what browsers do at the moment.
> 

That is incorrect (or I missed what you are referring to).

If an element creates multiple boxes offsetWidth/Height is returned as the bounding box of all those boxes. offsetLeft/Top is returned as the position of the first box.
Comment 44 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-12 17:02:27 PDT
Wow, so offsetLeft/Top/Width/Height don't actually describe any rectangle? That sucks!
Comment 45 Anne (:annevk) 2006-06-12 23:47:39 PDT
Erik, this is just about offsetLeft and offsetTop.

For the new API you can't directly reuse offsetLeft and offsetTop I think. Mostly because of the lovely first implementation and the web that evolved around that "the <body> element" is special. When offsetParent of a certain element becomes <body> you have to calculate its position against the initial containing block, not the <body> element. When the element itself is <body> offsetParent has to be null and offsetLeft and offsetParent are 0, always.

So for a positioned <body> element you can't ever get the position using offsetLeft and offsetTop. When we tried to work around those limitations in Opera by providing a "better" solution we broke the web. (This is part of the specification now so no "new" implementations have to break the web before "getting it right.")

And if we get an API to do these things more properly than with offset* I'd like to see that it addresses all cases at least.

Roc, noted rotated content inside <svg:foreignObject> as an open issue.
Comment 46 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-13 03:17:29 PDT
(In reply to comment #45)
> When offsetParent of a certain
> element becomes <body> you have to calculate its position against the initial
> containing block, not the <body> element. When the element itself is <body>
> offsetParent has to be null and offsetLeft and offsetParent are 0, always.

I see you've just revised your spec to do this :-).

So with the current revision of your spec, if you sum up the offsetLeft/offsetTop values all the way up the offsetParent chain, you always get the offset of the starting element's border-box left/top from the origin of the initial containing block --- UNLESS the starting element was the BODY, in which case you get (0,0).

[I guess you also get something weird for image maps and their parts, but that doesn't really worry me because they're not rendered elements in the normal sense.]

I guess I could implement that quirk for getClientRects/getBoundingClientRect, but should I? Hmm.
Comment 47 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 19:19:00 PDT
According to MSDN the rectangle(s) returned by getBoundingClientRect/getClientRects contain just left/top/right/bottom properties. should we have x/y/width/height as well?
Comment 48 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 19:31:54 PDT
Another interesting question is whether these properties should always be integers or whether we should allow floating point results. CSS does.
Comment 49 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 21:21:30 PDT
Yet another important question is what element types should expose getBoundingClientRect/getClientRects. Right now I just have them on HTML elements (where offset* are also defined), but maybe we should put them on all elements.
Comment 50 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 22:53:07 PDT
Another issue is whether the rectangle returned for a table should include the caption, which happens to be outside the table borders.
Comment 51 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 23:03:41 PDT
Created attachment 226276 [details] [diff] [review]
first cut patch

Implement getClientRects/getBoundingClientRect.

This is the simple implementation without any quirks (I hope!). In particular BODY is not at (0,0) in general. getClientRects returns rectangles in content order, simply because this turned out to be more convenient for me.
Comment 52 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 23:15:34 PDT
This patch returns floating point coordinates when things are subpixel positioned, up to the resolution of our internal coordinates. For tables, it includes the caption, which is arguably wrong.

If someone wants a build with this patch for testing, and you aren't able to make one yourself, jump onto Mozilla IRC and you might be able to find someone to create a test build for you.
Comment 53 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-19 23:16:51 PDT
What I'd like to do, if we can get the spec and implementation of these methods nailed down soon, is land this on trunk and also on FF2 branch. Then after FF2 has shipped I would disable getBoxObjectFor for Web content on the trunk for FF3.
Comment 54 Anne (:annevk) 2006-06-20 00:55:49 PDT
(In reply to comment #50)
> Another issue is whether the rectangle returned for a table should include
> the caption, which happens to be outside the table borders.

  http://dump.testsuite.org/2006/dom/style/offset/057.htm
  http://dump.testsuite.org/2006/dom/style/offset/058.htm

The expected results where not really what I expected, but apparently O9, IE7 and F1.5 all think they're perfect. So when it includes a caption you calculate from border edge of the caption and not the table (or perhaps the top border edge of the "table box"?).


Also, what is the difference between left, top and x, y? And width and height can be easily calculated if you really need them.


I'd be nice if this could work for all elements, not just HTML elements, imho.


Please leave out the BODY quirk (as you've done now :-) ). As opposed to offset* etc. this can hopefully be a more sane API.
Comment 55 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-06-28 16:18:13 PDT
Okay. So the remaining issues are:

-- Making it work for all elements. What should we do about SVG elements?
-- Foreignobject: I think we've agreed that foreignobject establishes a new reference coordinate system for the content inside it.
-- Should we return integers only, or allow floats to be returned?
-- Should we make top/bottom/left/right mutable, as IE does, or readonly?
Comment 56 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-03 20:10:34 PDT
anyone want to suggest an opinion on those questions?
Comment 57 Anne (:annevk) 2006-07-04 00:34:18 PDT
For SVG you could take the smallest rectangle the figure fits in. For paths and such you could let getClientRects return multiple objects I guess representing the multiple points.

I'd say we allow floats to be returned for precision. Given that these interfaces are used in JavaScript it shouldn't do any harm.

I'd also propose to make top/bottom/etc. readonly unless we get a very good use case not to do that.
Comment 58 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-10 23:10:28 PDT
Created attachment 228780 [details] [diff] [review]
updated patch

Alright, this patch does that. I'm going for review now. Requesting r from jst as DOM peer for the new DOM API, and sr from tor for the SVG bits
Comment 59 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-10 23:11:55 PDT
tor, I put the new function in nsSVGUtils because I didn't want to export more stuff from layout/svg.
Comment 60 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-10 23:14:02 PDT
Anne, two things to note:
-- The returned rectangles do not include the bounds of any child elements that might happen to overflow.
-- For HTML AREA elements, SVG elements that do not render anything themselves, display:none elements, and generally any elements that are not directly rendered, we return an empty list or a (0,0,0,0) rect.
Comment 61 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-10 23:19:12 PDT
Anne: also:
-- for getClientRects we return rectangles even for CSS boxes that have empty border-boxes. The left, top, right and bottom coordinates can still be meaningful.
-- for getBoundingClientRect, we completely ignore CSS boxes that have empty border-boxes. If all the element's CSS boxs' border-boxes are empty, then we return a rectangle whose width and height are zero and whose top and left are the top-left of the border-box for the first (content order) CSS box for the element.
Comment 62 tor 2006-07-11 06:36:19 PDT
Comment on attachment 228780 [details] [diff] [review]
updated patch

From an SVG point of view this looks ok, though we be returning the "inflated" pixel rounded covered region rather the fractional coverage.  To backport to the branch, GetOuterSVGFrameAndCoveredRegion will need some work to look inside a svg region.

You will need to add some ifdefs to handle non-SVG builds.  Probably the easiest way of doing that would be to ifdef the body of TryGetSVGBoundingRect.  With that, sr=tor for the SVG portion.
Comment 63 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-11 14:43:30 PDT
This gives slightly wrong results for SVG without the fix in bug 344206.
Comment 64 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-20 10:01:16 PDT
Comment on attachment 228780 [details] [diff] [review]
updated patch

Jonas, if you could review this before jst gets to it, that would be great.
Comment 65 Jonas Sicking (:sicking) PTO Until July 5th 2006-07-25 18:13:06 PDT
Comment on attachment 228780 [details] [diff] [review]
updated patch

>Index: layout/svg/base/src/nsSVGUtils.h
>+  /**
>+   * Get the covered region for a frame. Return null if it's not an SVG frame.
>+   * @param aRect gets a rectangle in *pixels*
>+   * @return the outer SVG frame which aRect is relative to
>+   */
>+  static nsIFrame*
>+  GetOuterSVGFrameAndCoveredRegion(nsIFrame* aFrame, nsRect* aRect);

No description of the aFrame param. Also, please add a comment saying that aRect is an out-param.

>Index: content/base/src/nsGenericElement.cpp

>+NS_IMPL_ISUPPORTS1(nsDOMNSElementTearoff, nsIDOMNSElement)

This is not right. You want to be able to QI back to any interface the element implements even after having QIed to nsIDOMNSElement. See nsGenericHTMLElementTearoff.

>+static nsPoint
>+GetOffsetFromInitialContainingBlock(nsIFrame* aFrame)
>+{
>+  nsPresContext* presContext = aFrame->GetPresContext();
>+  nsIPresShell* shell = presContext->PresShell();
>+  nsIFrame* rootScrollFrame = shell->GetRootScrollFrame();
>+  nsPoint pt(0,0);
>+  nsIFrame* child = aFrame;
>+  for (nsIFrame* p = aFrame->GetParent(); p && p != rootScrollFrame;
>+       p = p->GetParent()) {
>+    pt += p->GetPositionOfChildIgnoringScrolling(child);
>+    // coordinates of elements inside a foreignobject are relative to the top-left
>+    // of the nearest foreignobject
>+    if (p->IsFrameOfType(nsIFrame::eSVGForeignObject))
>+      return pt;
>+    child = p;
>+  }
>+  return pt;
>+}
>+
>+static double
>+RoundFloat(double aValue)
>+{
>+  return floor(aValue + 0.5);

Does this need to be |0.5f|? Feel free to make this function inline.


>+TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
>+nsDOMNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
>+nsDOMNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)

I would rather that someone else reviewed these functions as I don't know our layout code well enough to verify that they do the right thing.

One thing I did note though was that GetClientRects does not return a live list, was this intended?

Most current functions in the DOM, such as .childNodes or .frames return live lists. I don't really have an oppinion on if this should be one way or the other, but I wanted to make sure it was an intended desition. I like the fact that the list is returned in a different manner though (from a function rather than from a property), which IMHO makes it more ok that it is non-live.

r=me with a comment on the above question.
Comment 66 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-25 20:14:35 PDT
(In reply to comment #65)
> No description of the aFrame param. Also, please add a comment saying that
> aRect is an out-param.

OK

> >+NS_IMPL_ISUPPORTS1(nsDOMNSElementTearoff, nsIDOMNSElement)
> 
> This is not right. You want to be able to QI back to any interface the element
> implements even after having QIed to nsIDOMNSElement. See
> nsGenericHTMLElementTearoff.

OK

> >+static double
> >+RoundFloat(double aValue)
> >+{
> >+  return floor(aValue + 0.5);
> 
> Does this need to be |0.5f|?

No

> Feel free to make this function inline.

It's 'static' so the compiler should be able to do the right thing, whatever that is.

> >+TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
> >+nsDOMNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
> >+nsDOMNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)
> 
> I would rather that someone else reviewed these functions as I don't know our
> layout code well enough to verify that they do the right thing.

OK

> One thing I did note though was that GetClientRects does not return a live
> list, was this intended?

Yes. The individual rects have no identity of their own, unlike in the DOM node lists. It's really just a representation of a shape.
Comment 67 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-25 20:28:00 PDT
Comment on attachment 228780 [details] [diff] [review]
updated patch

Can you review just these functions 

>+TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
> >+nsDOMNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
> >+nsDOMNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)
Comment 68 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-12-22 00:34:09 PST
(In reply to comment #65)
> (From update of attachment 228780 [details] [diff] [review] [edit])
> >+TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
> >+nsDOMNSElementTearoff::GetBoundingClientRect(nsIDOMTextRectangle** aResult)
> >+nsDOMNSElementTearoff::GetClientRects(nsIDOMTextRectangleList** aResult)
> 
> I would rather that someone else reviewed these functions as I don't know our
> layout code well enough to verify that they do the right thing.

So apparently I've been asked to do this (a number of months ago, in comment 67).

I've started to try to do this a few times.  And like I just did, I look at the patch for a few minutes, or part of an hour, and read through the bug comments for similar amounts of time, and as of yet I've been unable to figure out what "the right thing" is, or if anybody knows what it is.

But I can't even tell from the maze of comments whether this is intended to be IE-compatible or whether it's a new invention.  And either way I don't see a single word of documentation about what these two methods do other than the MSDN URLs.

Sorry for the huge delay here -- I tend to stall rather than give feedback on review requests that I don't know how to review -- simply because I think I should be able to figure out how to review them, given just a little more time.

I was actually going to rubber-stamp this in the hopes that somebody else knows what's going on, but now I'm having second thoughts about that given that I haven't found any documentation of what these methods do other than what's on MSDN (Anne's spec only seems to cover offset*, not these methods), and the comments seem to indicate that this is *not* an attempt to be IE-compatible, but is rather a new invention, I don't see how this is going to be much use to Web developers without *some* indication of what it's intended to do (at the very least so they can distinguish bug from feature).

Could somebody summarize:
 * what are these methods intended to do?
 * what quirky IE behavior are we trying to match?
 * what quirky IE behavior are we choosing not to match?
 * how carefully has this been tested for IE-compatibility?
 * how carefully has this been tested for what we think it's supposed to do?

It would probably be good if the patch had some comments in nsIDOMNSElement.idl describing the methods, or at least a pointer to a spec for them.
Comment 69 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-12-22 00:48:10 PST
Comment on attachment 228780 [details] [diff] [review]
updated patch

I think the behavior described in comment 61 is almost definitely a bug.  Ignoring rects that happen to have 0 width or height doesn't make sense in some contexts.  But there are other contexts where it will probably cover up some layout bugs we have.

Other than that, I think the behavior in this patch seems pretty reasonable.  But it still seems sad not to have a spec for it.

r=dbaron since it's at least as good as getBoxObjectFor, though.
Comment 70 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-12-22 02:19:59 PST
Sorry that things weren't clear.

> * what are these methods intended to do?

The intended behaviour is:
-- getClientRects returns the CSS border-boxes for the element, in CSS pixels, with the top left measured relative to the top left of the canvas, assuming everything scrolled to its default position (per comment #34), unless the element is a descendant of an SVG foreignobject, in which case the rectangles are measured relative to the origin of the nearest foreignobject ancestor, in the foreignobject subtree's CSS coordinate system (i.e. ignoring any transforms applied to the foreignobject or its ancestors).
-- getBoundingClientRect returns the union of those rectangles.

Comments #60 and #61 apply to resolve some ambiguities in the above definition and to clarify some properties that might not be obvious from the definition:

-- The returned rectangles do not include the bounds of any child elements that
might happen to overflow (ok, this was not ambiguous in the above definition, it's just a note)
-- For HTML AREA elements, SVG elements that do not render anything themselves,
display:none elements, and generally any elements that are not directly
rendered, we return an empty list or a (0,0,0,0) rect (arguably this also follows from the definition)
-- for getClientRects we return rectangles even for CSS boxes that have empty
border-boxes. The left, top, right and bottom coordinates can still be
meaningful.
-- for getBoundingClientRect, we completely ignore CSS boxes that have empty
border-boxes. If all the element's CSS boxs' border-boxes are empty, then we
return a rectangle whose width and height are zero and whose top and left are
the top-left of the border-box for the first (content order) CSS box for the
element.

> * what quirky IE behavior are we trying to match?

No particular IE quirks are being specifically accounted for. If I understand what I've been told correctly, there is limited IE compatibility to the extent that "elem.getBoundingClientRect().left - document.body.getBoundingClientRect().left" (and ditto for .top) is equal in both implementations (modulo IE bugs that I haven't been told about).

> * what quirky IE behavior are we choosing not to match?

Setting document.body.getBoundingClientRect().left (and .top) to 2 or 0 depending on standards vs quirks mode and making everything relative to that.

> * how carefully has this been tested for IE-compatibility?

It hasn't. It would be useful to test for the "relative to body" compatibility property that I mentioned above.

> * how carefully has this been tested for what we think it's supposed to do?

I tested individual cases that I thought of, including cases I mentioned here and tested for in the code.

> I think the behavior described in comment 61 is almost definitely a bug.

OK, we can easily change it and it sounds like we should.
Comment 71 Anne (:annevk) 2006-12-22 03:53:57 PST
We also made the attributes of Textrectangle readonly. (Because writing them didn't do anything in Internet Explorer and therefore it didn't make much sense for them not to be readonly.)

My plan is to standardize these in the CSSOM: http://dev.w3.org/cvsweb/csswg/cssom/
Comment 72 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-12-22 11:53:57 PST
So Opera's implemented this?
Comment 73 Anne (:annevk) 2006-12-23 02:26:49 PST
Oops, confusing use of "we". I was referring to our previous discussion on this when "we" (you and I) decided to change that as well. I do think we'll implement this in due course. Not sure when.
Comment 74 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-22 01:41:45 PDT
Created attachment 265652 [details] [diff] [review]
updated to trunk
Comment 75 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-05-22 10:16:48 PDT
Thanks, roc. However to get the build buildable, I had to change this:
-    BREAK_WHITESPACE           = 0x01,
+    BREAK_WHITESPACE_END           = 0x01,

-  PRPackedBool             mBreakBeforeNextWord;
+  PRPackedBool             mBreakBeforeNonWhitespace;

in nsLineBreaker.h.
Comment 76 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-22 17:21:49 PDT
Sorry, that crept in by mistake.
Comment 77 Daniel Bainton 2007-05-22 23:00:19 PDT
And it wont actually build with SVG disabled... How much does it depend on SVG?
Comment 78 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-23 02:22:10 PDT
Not much ... something like this should work

+static PRBool
+TryGetSVGBoundingRect(nsIFrame* aFrame, nsRect* aRect)
+{
+#ifdef MOZ_ENABLE_SVG
+  nsRect r;
+  nsIFrame* outer = nsSVGUtils::GetOuterSVGFrameAndCoveredRegion(aFrame, &r);
+  if (!outer)
+    return PR_FALSE;
+
+  // r is in pixels relative to 'outer', get it into twips
+  // relative to ICB origin
+  r.ScaleRoundOut(1.0/aFrame->PresContext()->AppUnitsPerCSSPixel());
+  *aRect = r + GetOffsetFromInitialContainingBlock(outer);
+  return PR_TRUE;
+#else
+  return PR_FALSE;
+#endif
+}

along with #ifdefs protecting #include "nsSVGUtils.h" of course
Comment 79 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-05-23 07:03:07 PDT
I'm trying to make Mochitests for the patch.
One thing I noticed: http://martijn.martijn.googlepages.com/display_none.htm
I get this error with the patch:
Error: uncaught exception: [Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: 
In IE7 it alerts undefined.

I also get this error with the patch with this example:
http://martijn.martijn.googlepages.com/display_none_body.htm
It returns '0-0-0-0' in IE7.

document.documentElement.getBoundingClientRect().left - document.body.getBoundingClientRect().left is -8 in Mozilla with the patch, in IE7 it is 0. So that's a case where it isn't compatible.

Like you mentioned in comment 66, you made the rect lists non-live. That's not what IE is doing. Maybe it is wiser to be compatible with IE in this regard, to avoid confusion with web developers?

I'm still working on more testcases.
Comment 80 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-23 11:41:39 PDT
Thanks Martijn, this is exactly what I needed :-)

I'll look into those issues. One question: IE makes the rect list live? Do you mean the list is live, but the rects aren't? Making the list live is probably not hard.
Comment 81 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-23 15:14:56 PDT
One question about live lists, though: if we make it a live list, when should the objects in the list change? E.g. what happens if the author pulls out a rectangle, keeps a reference to it, and changes the layout, then looks at the rectangles in the list again, possibly comparing the identity of those objects with the old rectangle(s)? What sort of layout changes cause the list to be filled with new objects? If we don't use a live list, we don't have to specify that.
Comment 82 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-23 15:19:09 PDT
(In reply to comment #79)
> I'm trying to make Mochitests for the patch.
> One thing I noticed: http://martijn.martijn.googlepages.com/display_none.htm
> I get this error with the patch:
> Error: uncaught exception: [Exception... "Illegal value"  nsresult: "0x80070057
> (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: 
> In IE7 it alerts undefined.

Fixed in an upcoming patch.

> I also get this error with the patch with this example:
> http://martijn.martijn.googlepages.com/display_none_body.htm
> It returns '0-0-0-0' in IE7.

I think this should return undefined like other elements that have no CSS boxes. This is probably an IE bug related to its broken handling of BODY.

> document.documentElement.getBoundingClientRect().left -
> document.body.getBoundingClientRect().left is -8 in Mozilla with the patch, in
> IE7 it is 0. So that's a case where it isn't compatible.

OK. We need to restrict the compatibility guarantee to elements that are descendants of the body.

Thanks!
Comment 83 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-23 15:34:05 PDT
Created attachment 265854 [details] [diff] [review]
update
Comment 84 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-05-23 15:56:44 PDT
"
Like you mentioned in comment 66, you made the rect lists non-live. That's not
what IE is doing. Maybe it is wiser to be compatible with IE in this regard, to
avoid confusion with web developers?
"
Ok, this part of my comment 79 is wrong.
See these rough examples:
http://martijn.martijn.googlepages.com/boundingrect3.html
http://martijn.martijn.googlepages.com/display_none_d.htm
The seem to indicate that IE doesn't do anything live, not the getClientRects()
list, nor the rect itself.
Comment 85 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-28 21:32:29 PDT
Martijn, are you planning to do anything more here, or should I just land it?
Comment 86 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-30 15:39:02 PDT
OK, I landed this. Any remaining issues can be addressed in followup bugs. Thanks everyone!
Comment 87 Anne (:annevk) 2007-06-06 05:36:24 PDT
I cleaned up the specification a bit more: http://dev.w3.org/cvsweb/~checkout~/csswg/cssom/Overview.html?content-type=text/html;%20charset=utf-8#elementlayout

Let me know if you guys have any input!
Comment 88 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-06-06 11:17:15 PDT
A couple of suggestions:
-- should specify the handling of content inside a foreignobject (in a way that could be extended to elements that apply other complex transformations); see comment #70. What you have right now isn't very useful in those situations.
-- should specify how empty boxes are treated, hopefully like I did in comment #70 :-)
Comment 89 Anne (:annevk) 2007-06-07 06:41:37 PDT
Doesn't foreignObject just become the initial containing block? (Someone really needs to define foreignObject...) Could you give an example that would create empty border boxes? Just an element with height and width of zero and no border or padding?
Comment 90 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-06-07 15:04:03 PDT
(In reply to comment #89)
> Doesn't foreignObject just become the initial containing block?

Maybe that's the right way to define it. I don't know.

> Could you give an example that would create
> empty border boxes? Just an element with height and width of zero and no border
> or padding?

Sure. Or a <span style="font-size:0">, I guess. That could create multiple empty border-boxes.
Comment 91 Anne (:annevk) 2007-06-09 08:21:45 PDT
Clarified the empty boxes situation (to match comment 70) and added a note for svg:foreignObject.
Comment 92 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-06-10 14:40:10 PDT
Thanks. Martijn, you were going to turn your tests into Mochitests, right?
Comment 93 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-06-10 14:47:25 PDT
Yeah. You shouldn't worry about those. Although I might ask you review for them.
I really should finish it by tomorrow (or just ask review on what I've done so far).
Comment 94 Brian Rakowski 2007-08-09 13:19:17 PDT
Sorry for being dense, is this going in to FF3?
Comment 95 Neil Deakin 2007-08-09 13:30:13 PDT
(In reply to comment #94)
> Sorry for being dense, is this going in to FF3?
> 

Yes
Comment 96 philippe (part-time) 2007-08-22 05:55:18 PDT
Could this possibly have caused bug 392671 ?
Comment 97 Ivan Vadovic 2007-09-06 15:40:47 PDT
IE provides bounding properties and methods also for their TextRange object. I think it would make sense and be useful to extend the Range object similarily.

http://msdn2.microsoft.com/en-us/library/ms535872.aspx
Comment 98 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-09-06 16:07:05 PDT
Ivan, please file a new bug for that and mention the bug number here.
Comment 99 Nickolay_Ponomarev 2007-09-08 21:01:23 PDT
Some documentation is at:
http://developer.mozilla.org/en/docs/DOM:element.getClientRects
http://developer.mozilla.org/en/docs/DOM:element.getBoundingClientRect

It isn't very clear though and could use some examples.
Comment 100 Steve England [:stevee] 2007-09-19 07:04:07 PDT
It looks like this caused bug 396694
Comment 101 Eric Shepherd [:sheppy] 2007-09-25 09:56:25 PDT
The docs for this have been twiddled and are pretty acceptable now.  Marking as dev-doc-complete.

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