Bug 68922 - Paginated floats should not grow the height of a block when they don't end up being tied to a line.
Summary: Paginated floats should not grow the height of a block when they don't end up...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-27 11:47 PDT by Dave Hyatt
Modified: 2011-09-27 13:39 PDT (History)
1 user (show)

See Also:


Attachments
Patch (80.38 KB, patch)
2011-09-27 11:50 PDT, Dave Hyatt
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2011-09-27 11:47:14 PDT
Paginated floats should not grow the height of a block when they don't end up being tied to a line. Floats encountered in skipLeadingWhitespace that paginate can't immediately grow the height of the block, since there may not end up being any actual line content. We only want to push the height of the block down if we have actual line content that we want to keep with the float.
Comment 1 Dave Hyatt 2011-09-27 11:50:12 PDT
Created attachment 108877 [details]
Patch
Comment 2 mitz 2011-09-27 12:41:17 PDT
Comment on attachment 108877 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108877&action=review

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2696
> +        lineInfo.setFloatPaginationStrut(lineInfo.floatPaginationStrut() + paginationStrut);

I am surprised, without looking at the rest of the code, that the right thing to do here is to add, not to max(). It would be nice to have a test case with two floats that exercises this.
Comment 3 Dave Hyatt 2011-09-27 13:39:55 PDT
(In reply to comment #2)
> (From update of attachment 108877 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108877&action=review
> 
> > Source/WebCore/rendering/RenderBlockLineLayout.cpp:2696
> > +        lineInfo.setFloatPaginationStrut(lineInfo.floatPaginationStrut() + paginationStrut);
> 
> I am surprised, without looking at the rest of the code, that the right thing to do here is to add, not to max(). It would be nice to have a test case with two floats that exercises this.

Adding is correct. A double pagination push can really only be triggered when the height of the pages is non-uniform, so really we're talking about a CSS regions edge case.

I added this test case so that the code is exercised and landed the patch in r96149.