Bug 68590 - Floats pushed to next page don't reposition properly.
Summary: Floats pushed to next page don't reposition properly.
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
Depends on:
Reported: 2011-09-21 19:37 PDT by Dave Hyatt
Modified: 2011-09-22 09:32 PDT (History)
1 user (show)

See Also:

Patch (14.63 KB, patch)
2011-09-21 19:40 PDT, Dave Hyatt
aroben: 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-21 19:37:38 PDT
When a floating object is pushed to the next page, column or region as a result of either being unsplittable or because of a pagination strut, we need to recalculate its position. It may move to an area with more available width, below other floats, etc.,
Comment 1 Dave Hyatt 2011-09-21 19:40:03 PDT
Created attachment 108270 [details]
Comment 2 Adam Roben (:aroben) 2011-09-22 08:59:18 PDT
Comment on attachment 108270 [details]

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

> Source/WebCore/rendering/RenderBlock.cpp:3272
> +LayoutPoint RenderBlock::computeLogicalLocationForFloat(FloatingObject* floatingObject, LayoutUnit floatLogicalTop, LayoutUnit leftOffset, LayoutUnit rightOffset, LayoutUnit floatLogicalWidth)

Can this be a const member function? Can it take a const FloatingObject*?

Is "location" specific enough in this function name? Should it say "top left" instead?

It's confusing that you pass a value named floatLogicalTop into a function that is supposedly calculating that value for you. Is there a clearer way we can name the parameter that makes it clear what the caller has to do when calculating that value vs. what the function will do based on that value?

> LayoutTests/ChangeLog:11
> +        * fast/regions/webkit-flow-floats-inside-regions-bounds-expected.txt:

Don't you need to update pixel results, too?
Comment 3 Dave Hyatt 2011-09-22 09:32:54 PDT
Fixed in r95724.