If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Float is placed in the margin area

RESOLVED FIXED

Status

()

Core
Layout: Floats
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: mats, Assigned: roc)

Tracking

({regression, testcase})

Trunk
x86
Linux
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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

13 years ago
Created attachment 168530 [details]
Testcase
(Reporter)

Comment 2

13 years ago
Probably the same as bug 273946, marking depending for now...
Depends on: 273946
Created attachment 173048 [details] [diff] [review]
fix

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: 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.
checked in
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.