Last Comment Bug 816238 - Range.getClientRects() should include rects for all descendants in the range
: Range.getClientRects() should include rects for all descendants in the range
Status: UNCONFIRMED
: testcase
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-28 13:09 PST by Daniel Trebbien
Modified: 2013-01-18 15:43 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case (905 bytes, text/html)
2012-11-28 13:09 PST, Daniel Trebbien
no flags Details
A <span> with 5 text nodes (1.84 KB, text/html)
2012-12-01 17:13 PST, Mats Palmgren (:mats)
no flags Details
A descendant <span> with 5 text nodes (1.87 KB, text/html)
2012-12-01 17:31 PST, Mats Palmgren (:mats)
no flags Details

Description Daniel Trebbien 2012-11-28 13:09:49 PST
Created attachment 686244 [details]
Test case

Range.getBoundingClientRect() does not consider overflow content of children that is visible as a result of the parent element having overflow:visible. Horizontally and vertically overflown content is excluded.

Attached is a test case. In Firefox 17.0, the alert dialog shows the message "bcr.width = 14, bcr.height = 54", which I would expect for the bounding client rect of the inner box, not the bounding client rect of a Range selecting the inner box.

In Safari 6.0.2, Safari 5.0.5, and Chrome 23.0.1271.91, the alert message is "bcr.width = 195, bcr.height = 200", which seems correct based on measuring the content in a screenshot.
Comment 1 Mats Palmgren (:mats) 2012-11-28 21:10:45 PST
The CSSOM spec says it's "border boxes" (does not include overflow)
http://dvcs.w3.org/hg/csswg/raw-file/tip/cssom-view/Overview.html#extensions-to-the-range-interface
http://dvcs.w3.org/hg/csswg/raw-file/tip/cssom-view/Overview.html#the-getclientrects-and-getboundingclientrect-methods

"For each element selected by the range, whose parent is not selected by the range"
that would be the <div id="innerBox"> in your example.
"For each Text node selected or partially selected by the range"
there are no partially selected text nodes in your example.

AFAICT, Firefox is correct per the spec.  Opera does the same.
IE10 seem to match webkit.
Comment 2 Daniel Trebbien 2012-11-29 06:10:43 PST
The way I interpret the spec, a Range selecting an element also selects the child elements and Text nodes.  Therefore, a Range around the inner box also selects all of the Text node and BR element children of the inner box.

DOM Level 2 Traversal and Range mentions the example of a user making a selection in a text editor or word processor by pressing down the mouse at one point in a document, moving the mouse to another point, and releasing the mouse.  http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html
If I use my mouse to try to select the inner box in Firefox, some of the text overflowing the border box is also selected.

As in the given example, because the overflown content is visible, I think that the bounding client rect of the selection should not be limited to the border box of #innerBox, but also include the overflown content.  However, if I were to change overflow to hidden on #innerBox, then I think that 14 x 54 is correct.  https://bugs.webkit.org/show_bug.cgi?id=103430
Comment 3 Mats Palmgren (:mats) 2012-11-29 12:23:53 PST
(In reply to Daniel Trebbien from comment #2)
> The way I interpret the spec, a Range selecting an element also selects the
> child elements and Text nodes. 

Yes...

> Therefore, a Range around the inner box also
> selects all of the Text node and BR element children of the inner box.

... but the spec for getClientRects explicitly says "whose parent is not
selected by the range" so, in your example, all the descendants of the inner
<div> should be excluded.

> As in the given example, because the overflown content is visible, I think
> that the bounding client rect of the selection should not be limited to the
> border box of #innerBox, but also include the overflown content.

That's not what CSSOM specifies.
I think the spec is very clear that getClientRects returns border boxes,
not a hypothetical box around what is visible.

In fact, webkit also returns border boxes, it just includes too many rectangles
in the getClientRects result.  It seems to ignore the "whose parent is not
selected" part of the spec.  I've filed that as
https://bugs.webkit.org/show_bug.cgi?id=103658
(I've attached a testcase there that demonstrates the webkit bug.)
Comment 4 Daniel Trebbien 2012-11-29 15:30:29 PST
> ... but the spec for getClientRects explicitly says "whose parent is not
> selected by the range" so, in your example, all the descendants of the inner
> <div> should be excluded.

Oh, sorry. You are right. My mistake.
Comment 5 Daniel Trebbien 2012-12-01 12:52:57 PST
I was thinking about creating a patch for the WebKit bug that you created, Bug 103658, as my first foray into WebKit development.  It seems like a good first bug to work on because the patch shouldn't be that involved.

The source code behind WebKit's Range.getBoundingClientRect() is located in WebCore/dom/Range.cpp.  Upon looking at the implementation of a helper, getBorderAndTextQuads(), it appears immediately obvious that to fix this bug, I would simply need to check whether each Text node has been seen before as well.  However, now I am wondering why the WebKit developers have deliberately handled Text nodes and Elements differently.

As you pointed out, Range.getClientRects() includes the border boxes of elements selected by the range whose parents are not also selected by the range.  With regard to Text nodes, I am now confused.

The spec says:

> For each Text node selected or partially selected by the range (including
> when the boundary-points are identical), include a ClientRect object (for
> the part that is selected, not the whole line box). The bounds of these
> ClientRect objects are computed using font metrics; thus, for horizontal
> writing, the vertical dimension of each box is determined by the font ascent
> and descent, and the horizontal dimension by the text advance width.

But it does not say anything about not including ClientRects for Text nodes that are children of elements selected by the Range.

What am I missing?
Comment 6 Mats Palmgren (:mats) 2012-12-01 16:23:22 PST
> But it does not say anything about not including ClientRects for Text nodes
> that are children of elements selected by the Range.

That's a good point.  I somehow got the idea that "whose parent is not
selected by the range" also affected the recursive traversal[1] that
Element.getClientRects() does.  But you're right, it shouldn't.

So after a more careful reading of the spec, I think we should indeed
include all text nodes, and all elements that are *fully* inside the
range, recursively.  Which is what you said in comment 2 I think.

It appears that none of IE, Chrome, Firefox or Opera implements that
though, so I'm not sure I've understood it correctly.

[1]
http://dvcs.w3.org/hg/csswg/raw-file/tip/cssom-view/Overview.html#widl-Element-getClientRects-ClientRectList
Comment 7 Mats Palmgren (:mats) 2012-12-01 16:36:21 PST
Our implementation lives here (if you want to take a stab at it ;-) ):
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsRange.cpp#2586
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsRange.cpp#2612
Comment 8 Mats Palmgren (:mats) 2012-12-01 17:13:34 PST
Created attachment 687476 [details]
A <span> with 5 text nodes
Comment 9 Mats Palmgren (:mats) 2012-12-01 17:31:27 PST
Created attachment 687477 [details]
A descendant <span> with 5 text nodes

This is a better test actually.
Comment 10 Mats Palmgren (:mats) 2012-12-01 18:08:58 PST
Hmm, it appears Element.getClientRects() isn't recursive at all, which makes me
confused as to what bullet 2 of Range.getClientRects() really means.  Maybe I was
right the first time that it only address text nodes in the range that are not
a descendant of elements covered by bullet 1.
Comment 11 Daniel Trebbien 2012-12-03 13:51:10 PST
> Maybe I was right the first time that it only address text nodes in the range
> that are not a descendant of elements covered by bullet 1.

I am thinking that this is correct, but the spec appears to be ambiguous on the matter.

Would you re-open https://bugs.webkit.org/show_bug.cgi?id=103658 ?  I think it is worth keeping that WebKit bug open until this ambiguity is resolved, and I can't re-open it myself.
Comment 12 Mats Palmgren (:mats) 2012-12-05 09:57:58 PST
Yeah, the spec for Element.getClientRects() and Range.getClientRects() isn't
very good.  But since Element.getClientRects() is non-recursive in all UAs
I've tested it seems logical that the text in Range.getClientRects() about
"whose parent is not selected by the range" is intended to make that the
same, basically the border boxes for the top-most nodes inside the range
whether they are elements or text nodes.  That's what Fx and Opera does and
IE10 is roughly compatible.  Webkit seems to recurse for Range.getClientRects()
which gives different results for
   range.selectNode(elem); range.getClientRects();
compared to
  elem.getClientRects();
which intuitively feels wrong.

I'll file a new bug on Webkit since the old bug is just confusing now :-)
Comment 13 Mats Palmgren (:mats) 2012-12-05 10:02:16 PST
Anne, perhaps you can clarify what the spec says about Range.getClientRects() ?
We think it's ambiguous and hard to understand.
http://dvcs.w3.org/hg/csswg/raw-file/tip/cssom-view/Overview.html#extensions-to-the-range-interface
Comment 14 charles.kendrick 2013-01-18 15:43:53 PST
Just a note that one of the primary use cases I can see for this API is for JavaScript libraries finding out how big content is, or whether a coordinate falls in a rect for hit detection / overlap detection / etc.  If this API is non-recursive it implies JavaScript has to do recursive traversals instead, and it's hard to see what purpose the API fulfills at that point..

So if the spec is going to be clarified I would vote for WebKit's more useful behavior here.

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