410229 - getBoundingClientRect/getClientRects do not process block-in-inline "special siblings"

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect, P3)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See attached testcase. We're not looking at special siblings, so the rectangle(s) returned only include the first part of the IB split. This will be easy to fix and we should fix it because this is a new feature and we don't want it to be broken for the Web.

Comment 1

[Uri Bernstein]

(In reply to comment #0)
> See attached testcase.

I think Bugzilla ate your attachment (this happened to me recently) :-(

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes the problem with liberal use of GetNextContinuationOrSpecialSibling, and adds a mochitest.

I'm fixing GetAllInFlowBoundingRect because these DOM APIs use it. This will also affect offset* properties, but that's OK --- this bug applies to them too.

Comment 3

[Mats Palmgren (inactive)]

Attachment: A few more tests...

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 295185 [details] [diff] [review]
fix

>     nsIFrame* next;
>-    while ((next = frame->GetNextContinuation()) != nsnull) {

I think this loop would be clearer if we remove 'next',
can we just assign 'frame' directly instead?

>       NS_ASSERTION(isSVG, "SVG frames must have SVG continuations");

Does this still hold?  What about a html:div in a SVG inline, wouldn't
that create a special sibling where 'isSVG' is false?

r+sr=mats with the above addressed as you see fit, but please also
consider adding some of the additional tests I attached above.
Please also verify that those test results are the desired results,
specifically for the empty rects - should we have these at all and
should they be outside the parent's bounding rect? and in the last
case - should we only have two rects?

Just so I know - we are trying to emulate IE here I suppose?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> can we just assign 'frame' directly instead?

Yeah I guess so!

> Does this still hold?  What about a html:div in a SVG inline, wouldn't
> that create a special sibling where 'isSVG' is false?

We don't have real SVG inline frames.

> Just so I know - we are trying to emulate IE here I suppose?

No. This was just a bug I realized we had.

I'll check in your tests, thanks. Those zero width rects seem a little strange but I guess that's where the left and right border of the span would go if it had them, so I guess they're logical. It is correct per spec for empty rects to not contribute to the bounding rect.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix test failures

Two of the tests failed on Mac. This patch fixes and reenables them. I was just scaling/rounding the rect badly.

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 295323 [details] [diff] [review]
fix test failures

r+sr=mats

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I already checked that patch in to fix Tinderbox orange.