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

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
Created attachment 142398 [details]
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.
Created attachment 142399 [details] [diff] [review]
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.
Attachment #142399 - Flags: superreview?(dbaron)
Attachment #142399 - Flags: review?(dbaron)
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 on attachment 142399 [details] [diff] [review]
fix

r+sr=dbaron if you've checked some testcases (e.g., bug 79315 and its fixed
dependencies)
Attachment #142399 - Flags: superreview?(dbaron)
Attachment #142399 - Flags: superreview+
Attachment #142399 - Flags: review?(dbaron)
Attachment #142399 - Flags: review+
I'll fix up the pfds as you suggest, but I'll use a new name at the end to avoid
shadowing.
Fix checked in. Testcases in 79315 and everything hanging off it work fine.
Actually marking fixed
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 8

14 years ago
I think the changes from this bug caused Bug 237734, backing out the checkin
seems to make the testcase work every time.
You need to log in before you can comment on or make changes to this bug.