Bug 33245 - Second right floated image misplacment
Summary: Second right floated image misplacment
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.blahertech.org/about/staff
Keywords: InRadar, NeedsReduction
Depends on:
Blocks: 44640
  Show dependency treegraph
Reported: 2010-01-05 18:17 PST by Benjamin Jay Young
Modified: 2011-01-04 17:56 PST (History)
4 users (show)

See Also:

Test case (742 bytes, text/html)
2010-01-17 20:46 PST, mitz
no flags Details
Test case (794 bytes, text/html)
2010-01-17 21:29 PST, mitz
no flags Details
Work in progress (1.60 KB, patch)
2010-01-18 01:09 PST, mitz
no flags Details | Formatted Diff | Diff
Create a root line box for trailing floats after the line break on the last line (30.92 KB, patch)
2010-01-18 12:38 PST, mitz
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
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:

Do they just need a new baseline?