Last Comment Bug 235778 - Overflowing absolute children of a relatively positioned span not included in span's overflow area
: Overflowing absolute children of a relatively positioned span not included in...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-02-26 19:32 PST by Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
Modified: 2004-03-18 15:06 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (507 bytes, text/html)
2004-02-26 19:34 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details
fix (6.97 KB, patch)
2004-02-26 19:39 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
dbaron: review+
dbaron: superreview+
Details | Diff | Review

Description Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-02-26 19:32:58 PST
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) (Exited; email my personal email if necessary) 2004-02-26 19:34:47 PST
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.
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-02-26 19:39:59 PST
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.
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-02-26 23:48:44 PST
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-7 (review requests must explain patch) 2004-02-26 23:53:10 PST
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) (Exited; email my personal email if necessary) 2004-02-27 07:11:00 PST
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) (Exited; email my personal email if necessary) 2004-02-27 18:56:30 PST
Fix checked in. Testcases in 79315 and everything hanging off it work fine.
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-02-27 18:57:23 PST
Actually marking fixed
Comment 8 PikeUK 2004-03-18 15:06:37 PST
I think the changes from this bug caused Bug 237734, backing out the checkin
seems to make the testcase work every time.

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