-------- Summary: -------- It appears that tall floated elements can be cleared properly on screen but not necessarily when printing (or viewing through print-preview). ------------------- Steps to Reproduce: ------------------- I created a test cast which has two columns of text followed by a clearing element and then some more text -- something like this: ###### ######### ###### ######### ###### ######### ###### ######### ###### ######### ###### ###### ###### ###### ########################## ########################## ########################## ########################## Steps to reproduce: 1. Load the testcase: http://www.handcoding.com/documents/testing/floating-printing/floating-printing.html 2. Confirm that the two columns are floated and that the third block of text is cleared from the floated elements. 3. Run print preview 4. While the third block of text should still be cleared (in the print-preview window), it appears next to the first column of text ------------ More Detail: ------------ The clearing is done via a clearing element just after the two sets of columns which has the properties "clear:both" and "height:0". On screen, this works fine. However, when printing (or viewing through print-preview), that last block of text ends up jutted against the left-most column, as if the clearing hadn't occurred. I should also mention that the left-most column has enough text to span two pages. Scenarios which prevent this bug from occurring: * If the first column has less than a page's worth of text, the clearing works fine. * If the clearing element has "clear:both" but not "height:0", the clearing works fine ---------- Build Info ---------- I saw this on both 20070411/Trunk (Minefield/3.0a4pre) and 20070411/Branch (BonEcho/2.0.0.4pre) on OSX. I also confirmed that this occurs on 20070411/Branch (BonEcho/2.0.0.4pre) on WinXP. (I didn't have a chance test 20070411/Branch on WinXP.)

Oops -- that last sentence should say that I didn't have a chance test WinXP with 20070411/Trunk.

Tested on WinXP 20070420 as well. I think it's interesting that the clearing element's content disappears. It doesn't matter what value height is set to. The clearing element only clears the first page if a) its height is set to any fixed value or b) the clearing element is empty.

The element also clears properly if it has more than one line (in this case no text disappears) *and* the height is nonzero.

Hmm, this is kind of difficult to deal with in our current architecture (at least in the zero-height clear case). I'll think about it. The fixed-height case is pretty easy to explain; it's caused by the messed up code at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsBlockFrame.cpp&rev=3.838&mark=1268-1276#1268.)

(In reply to comment #4) > Hmm, this is kind of difficult to deal with in our current architecture (at > least in the zero-height clear case). I'll think about it. I think we could check whether the frames we're clearing-after are complete; if one or more is not complete, just push the clearing frame. > The fixed-height case is pretty easy to explain; it's caused by the messed up > code at > http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsBlockFrame.cpp&rev=3.838&mark=1268-1276#1268.) So that should be fixed by fantasai's work then?

(In reply to comment #5) > (In reply to comment #4) > > Hmm, this is kind of difficult to deal with in our current architecture (at > > least in the zero-height clear case). I'll think about it. > > I think we could check whether the frames we're clearing-after are complete; if > one or more is not complete, just push the clearing frame. Where would be a good place to check this? > > The fixed-height case is pretty easy to explain; it's caused by the messed up > > code at > > http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsBlockFrame.cpp&rev=3.838&mark=1268-1276#1268.) > > So that should be fixed by fantasai's work then? > No, it looks like that code is still around. I'm not sure what you were trying to fix with it, though; the comment is completely wrong. Is the "aMetrics.height > aReflowState.availableHeight" comparison flipped the wrong way? (The way it's written, it has the effect of cutting off blocks that don't fit rather than expanding blocks that need to take up more space.)

This code is trying to handle the case where the content-rect of the block fits but the padding + border doesn't: the goal is to push the padding + border to the next page. We shouldn't be cutting off block content (i.e. lines) because the block was complete, so the content should fit into the available height (although I could easily believe there's a bug here). > Where would be a good place to check this? Maybe the space manager could remember if it has any incomplete floats? Then we could check that where we ask the space manager where the floats end.

> This code is trying to handle the case where the content-rect of the block fits > but the padding + border doesn't: the goal is to push the padding + border to > the next page. We shouldn't be cutting off block content (i.e. lines) because > the block was complete, so the content should fit into the available height > (although I could easily believe there's a bug here). Okay, I understand the issue now; we should be doing the same thing as the other branch, and set aMetrics.height to PR_MAX(aReflowState.availableHeight, aState.mY + nonCarriedOutVerticalMargin). The current code leads to the truncation bit not getting set when it should be set. > > Where would be a good place to check this? > > Maybe the space manager could remember if it has any incomplete floats? Then we > could check that where we ask the space manager where the floats end. I was actually thinking the same thing. I'm not sure where would be appropriate to check the flag, though.

nsBlockReflowState::ClearFloats and/or its callers?

Patch for the fixed-height case.

Checked in part 1.

WIP; does this look sane?

+ (aLine->HasClearance() && aState.mY + clearance > aState.mReflowState.availableHeight)) { Why check this here? I think either we should check this before we reflow the child block, or (better) NS_INLINE_IS_BREAK_BEFORE(frameReflowStatus) should be true in this case. Although I guess it's a misapplication of NS_INLINE_IS_BREAK_BEFORE --- but the child block reflow should saying that none of it fit.

Does this look better?

Comment on attachment 272866 [details] [diff] [review] Patch part 2 WIP 2 This is no good... the nsBlockReflowState change messes up floats in columns.

Bug 377204 might be related here.

and by bug 377204 I mean bug 507510...

Nevermind, it is definitely a separate bug. Here's a testcase that uses fixed-height floats. The problem is that we don't push zero-height lines even if they have clearance that requires going to the next page.