Closed
Bug 274249
Opened 20 years ago
Closed 20 years ago
Float is placed in the margin area
Categories
(Core :: Layout: Floats, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: MatsPalmgren_bugz, Assigned: roc)
References
Details
(Keywords: regression, testcase)
Attachments
(2 files)
397 bytes,
text/html
|
Details | |
6.61 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
Float is placed in the margin area. Regression occured 2004-11-25-06 -- 2004-11-26-06 (likely bug 209694) STEPS TO REPRODUCE: 1. Load testcase ACTUAL RESULTS: The float is placed at the top, adjacent to the top black border. EXPECTED RESULTS: The float should be placed at the bottom, below the margin, and overflowing the black box.
Reporter | ||
Comment 1•20 years ago
|
||
Reporter | ||
Comment 2•20 years ago
|
||
Probably the same as bug 273946, marking depending for now...
Depends on: 273946
Assignee | ||
Comment 3•20 years ago
|
||
The fix for bug 273946 did exactly the right thing to position the empty block. The problem is that it makes us double-count the empty block's top margin: it's included in the block's position (which the containing block then uses as the starting point for the next line), but it's also included in the carried-out bottom margin. So we need the YMost of the empty block frame to be different from the Y-coordinate where its parent block will place the next line. That's straightforward to do for regular reflow but it's hard to do for incremental reflow; when ReflowDirtyLines skips reflowing a line, it wants to use line->mBounds.YMost() as the starting point for the next line, and we don't have anywhere to record that for *this* line, you should use a different value. My approach is to allow line->mBounds to be different from the block's position; we back up line->mBounds by the empty block's mTopMargin, but leave the block frame where it is. This gives us the correct placement of placeholder frames but also lets the container block put subsequent lines in the right place. Note that line->mBounds could already be different from the block position due to relative positioning.
Attachment #173048 -
Flags: superreview?(dbaron)
Attachment #173048 -
Flags: review?(dbaron)
Assignee | ||
Updated•20 years ago
|
Assignee: nobody → roc
Comment on attachment 173048 [details] [diff] [review] fix > if (needToRecoverState && > (line->IsDirty() || line->IsPreviousMarginDirty())) { Now this could just be: if (needToRecoverState && line->IsDirty()) { It might be good to add a little more of what you put in the previous comment into the bit where you declare or set backupContainingBlockAdvance. I understood what this patch is doing from the bugzilla comment but not from the source code comment (which I read first). r+sr=dbaron
Attachment #173048 -
Flags: superreview?(dbaron)
Attachment #173048 -
Flags: superreview+
Attachment #173048 -
Flags: review?(dbaron)
Attachment #173048 -
Flags: review+
In particular, making the point that what you're doing is making the line bounds different from the block's rect.
Assignee | ||
Comment 6•20 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•