Bug 33245

Summary: Second right floated image misplacment
Product: WebKit Reporter: Benjamin Jay Young <blaher09>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, gustavo, mitz, xan.lopez
Priority: P2 Keywords: InRadar, NeedsReduction
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.blahertech.org/about/staff
Bug Depends on:    
Bug Blocks: 44640    
Attachments:
Description Flags
Test case
none
Test case
none
Work in progress
none
Create a root line box for trailing floats after the line break on the last line darin: review+

Description Benjamin Jay Young 2010-01-05 18:17:24 PST
In firefox, ie, opera the page (http://www.blahertech.org/about/staff) displays fine. However when using a webkit browser such as chrome and safari, my left floated images are displaying in misplaced areas. There is no known CSS issue that I know of that would be causing this, since I have cleared both sides.
Comment 1 Benjamin Jay Young 2010-01-05 18:18:57 PST
Excuse me, I meant right floated.
Comment 2 mitz 2010-01-05 20:49:12 PST
This behaves the way I would expect it to behave, given the <br> elements with "clear: both;" but perhaps I am missing something. A reduction would help.
Comment 3 Benjamin Jay Young 2010-01-06 06:14:53 PST
The br element is before the floated image. It should behave just as the image above it and have the text wrap around it. Yet the text seems to be behaving as if the br element is below the image, which it's not.

The only reason I would expect this behavior is if the h3 tag would be clearing it, however this is not the case since the above method is repeated above and everything responds normally.
Comment 4 Benjamin Jay Young 2010-01-06 06:15:42 PST
Excuse me again, I meant the br element is above the image.
Comment 5 mitz 2010-01-15 08:35:36 PST
<rdar://problem/7546035>
Comment 6 mitz 2010-01-17 20:46:05 PST
Created attachment 46784 [details]
Test case

This is based on the original page. It demonstrates how incremental layout can lead to wrong behavior.
Comment 7 mitz 2010-01-17 21:29:06 PST
Created attachment 46787 [details]
Test case

Simplified test case
Comment 8 mitz 2010-01-17 22:30:58 PST
See bug 19278. When the last line contains a line break, the floats vector for that line may potentially contain both floats occurring before the line break and floats occurring after the line break. For example,
<img align="right"><br><img align="right">

Later, during incremental layout, when the line with the line break happens to be the last clean line, both kinds of floats are “restored”. If the <br> happens to be clearing, it will try to clear both, although it should not clear the image that comes after it.

Perhaps the solution is not to put the “after the line break” floats in the next line box’s floats vector. When there is no next line box, make one.
Comment 9 mitz 2010-01-17 22:31:43 PST
(In reply to comment #8)
> Perhaps the solution is not to put

I meant “…the solution is to put…”.
Comment 10 mitz 2010-01-18 01:09:27 PST
Created attachment 46797 [details]
Work in progress

First cut. Breaks repaint and hit-testing due to the extra line. May want to have a RootInlineBox subclass for that, with virtualHeight().
Comment 11 mitz 2010-01-18 12:38:09 PST
Created attachment 46837 [details]
Create a root line box for trailing floats after the line break on the last line
Comment 12 Darin Adler 2010-01-18 12:44:30 PST
Comment on attachment 46837 [details]
Create a root line box for trailing floats after the line break on the last line

Neither the change log nor the source files explain the role of this additional box. I think you need a one-sentence comment stating what, to you, is obvious.

> +    TrailingFloatsRootInlineBox(RenderObject* obj) : RootInlineBox(obj)

I would call this "object", not "obj".

> +    virtual int virtualHeight() const { return 0; }

I would make this member function private. It would be a programming error to call this with a TrailingFloatsRootInlineBox*.
Comment 14 Eric Seidel (no email) 2010-01-18 18:02:13 PST
Looks like this is a failure on the Gtk bots:
http://build.webkit.org/results/GTK%20Linux%2032-bit%20Debug/r53420%20(2468)/fast/dynamic/float-in-trailing-whitespace-after-last-line-break-2-pretty-diff.html

Do they just need a new baseline?