235778 - Overflowing absolute children of a relatively positioned span not included in span's overflow area

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

I thought we killed this snake long ago, but the upcoming testcase shows that
the bounds of a relative span's absolute children aren't included in its
overflow area.

Comment 1

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

Attachment: testcase

This isn't quite minimal, but it's good enough to show the problem. The DIV "I
am absolutely positioned." won't repaint properly if you dirty it. A moment's
work with the layout debugger confirms that the relatively positioned span
doesn't have the right combined area.

Comment 2

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

Attachment: fix

2 problems:
-- nsLineLayout::RelativePositionFrames just ignored a span frame's
pfd->mCombinedArea, wiping it out with a new combined area that's the union of
its child inline frame combined areas. Solution: initialize the accumulated
area with the span frame combined area instead of the span frame's bounds.
-- nsPositionedInlineFrame was never computing the overflow area properly
because someone noticed it was just being ignored and turned it off. Turn it
back on.

This fixes the bug.

I took the liberty of cleaning up RelativePositionFrames a bit.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

How about getting rid of this declaration,
>   PerFrameData* pfd;

using psd->mFrame->mCombinedArea directly here,
>     pfd = psd->mFrame;
>+    combinedAreaResult = pfd->mCombinedArea;

declaring pfd in the for-loop initializer,
>   for (pfd = psd->mFirstFrame; pfd; pfd = pfd->mNext) {

and making this what VC++ will think is a shadowing variable?
>   if (psd->mFrame) {
>     pfd = psd->mFrame;

More comments shortly...

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

r+sr=dbaron if you've checked some testcases (e.g., bug 79315 and its fixed
dependencies)

Comment 5

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

I'll fix up the pfds as you suggest, but I'll use a new name at the end to avoid
shadowing.

Comment 6

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

Fix checked in. Testcases in 79315 and everything hanging off it work fine.

Comment 7

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

Actually marking fixed

Comment 8

[PikeUK]

I think the changes from this bug caused Bug 237734, backing out the checkin
seems to make the testcase work every time.