Bug 60729

Summary: Split more logic out from determineStartPosition
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, dglazkov, leviw, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 60925    
Bug Blocks: 50912    
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02 none

Description Eric Seidel (no email) 2011-05-12 14:59:34 PDT
Split more logic out from determineStartPosition
Comment 1 Eric Seidel (no email) 2011-05-12 15:02:47 PDT
Created attachment 93350 [details]
Patch
Comment 2 WebKit Review Bot 2011-05-12 15:46:14 PDT
Comment on attachment 93350 [details]
Patch

Attachment 93350 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8689840

New failing tests:
fast/block/float/float-in-float-hit-testing.html
fast/repaint/line-flow-with-floats-4.html
fast/repaint/line-flow-with-floats-3.html
fast/repaint/line-flow-with-floats-10.html
fast/repaint/line-flow-with-floats-5.html
Comment 3 WebKit Review Bot 2011-05-12 15:46:19 PDT
Created attachment 93359 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 4 Brent Fulgham 2011-05-31 20:34:12 PDT
Comment on attachment 93350 [details]
Patch

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

Looks good to me, but I'm worried about the cr-linux failure.  Is that expected?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1183
> +// Floats is a non-const refernce as we may update the size of the float after laying it out.

s/refernce/reference/

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1259
> +        // We have a dirty line.

I'm not sure this comment is telling me anything that "if (firstDirtyLine)" didn't already! ;-)
Comment 5 Adam Barth 2011-10-14 22:47:00 PDT
Comment on attachment 93350 [details]
Patch

My guess is that this isn't needed anymore now that your bidi isolate work is complete.