https://bugs.webkit.org/show_bug.cgi?id=68940

Narrow the float/lines pagination heuristic to only kick in if
the previous line broke cleanly and if the floats are occurring
at the start of the line.

Reviewed by Dan Bernstein.

Source/WebCore:

* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::positionNewFloatOnLine):

LayoutTests:

* fast/regions/webkit-flow-float-pushed-to-last-region.html:
* platform/mac/fast/multicol/float-paginate-empty-lines-expected.txt: Added.
* platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@96162 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 545c657..d0fc3ec 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2011-09-27  David Hyatt  <hyatt@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=68940
+
+        Narrow the float/lines pagination heuristic to only kick in if
+        the previous line broke cleanly and if the floats are occurring
+        at the start of the line.
+
+        Reviewed by Dan Bernstein.
+
+        * fast/regions/webkit-flow-float-pushed-to-last-region.html:
+        * platform/mac/fast/multicol/float-paginate-empty-lines-expected.txt: Added.
+        * platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt:
+
 2011-09-27  James Robinson  <jamesr@chromium.org>
 
         Add a mechanism to test for the compositing tree mutated during painting

diff --git a/LayoutTests/fast/regions/webkit-flow-float-pushed-to-last-region.html b/LayoutTests/fast/regions/webkit-flow-float-pushed-to-last-region.html
index ed19252..87e456d 100644 (file)
--- a/LayoutTests/fast/regions/webkit-flow-float-pushed-to-last-region.html
+++ b/LayoutTests/fast/regions/webkit-flow-float-pushed-to-last-region.html
@@ -47,7 +47,7 @@
 <div id="content">
     <div id="first-box">
         <div id="second-box">
-            <p>This line of text <img id="float1"> should not get out of the  region. This line of text should not get out of the region. This line of text should not get out of the region. This line of text should not get out of the region.</p>
+            <p> <img id="float1"> This line of text should not get out of the  region. This line of text should not get out of the region. This line of text should not get out of the region. This line of text should not get out of the region.</p>
             <p>This line of text should not get out of the region. This line of text should not get out of the region. This line of text should not get out of the region. This line of text should not get out of the region.</p>
             <p>This line of text should not get out of the region.</p>
         </div>

diff --git a/LayoutTests/platform/mac/fast/multicol/float-paginate-empty-lines-expected.txt b/LayoutTests/platform/mac/fast/multicol/float-paginate-empty-lines-expected.txt
new file mode 100644 (file)
index 0000000..d274757
--- /dev/null
+++ b/LayoutTests/platform/mac/fast/multicol/float-paginate-empty-lines-expected.txt
@@ -0,0 +1,51 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x54
+        RenderText {#text} at (0,0) size 764x54
+          text run at (0,0) width 764: "This test is ensuring we don't grow the height of a block improperly when a float has no line association (e.g., when it's at"
+          text run at (0,18) width 741: "the end of a block). The complete dashed border should be in the first column, with none of it appearing in the second"
+          text run at (0,36) width 51: "column."
+layer at (8,78) size 784x400
+  RenderBlock {DIV} at (0,70) size 784x400
+    RenderBlock {DIV} at (0,0) size 384x236 [border: (10px dashed #800000)]
+      RenderText {#text} at (10,10) size 110x18
+        text run at (10,10) width 110: "This is some text."
+      RenderBR {BR} at (120,24) size 0x0
+      RenderText {#text} at (10,28) size 110x18
+        text run at (10,28) width 110: "This is some text."
+      RenderBR {BR} at (120,42) size 0x0
+      RenderText {#text} at (10,46) size 110x18
+        text run at (10,46) width 110: "This is some text."
+      RenderBR {BR} at (120,60) size 0x0
+      RenderText {#text} at (10,64) size 110x18
+        text run at (10,64) width 110: "This is some text."
+      RenderBR {BR} at (120,78) size 0x0
+      RenderText {#text} at (10,82) size 110x18
+        text run at (10,82) width 110: "This is some text."
+      RenderBR {BR} at (120,96) size 0x0
+      RenderText {#text} at (10,100) size 110x18
+        text run at (10,100) width 110: "This is some text."
+      RenderBR {BR} at (120,114) size 0x0
+      RenderText {#text} at (10,118) size 110x18
+        text run at (10,118) width 110: "This is some text."
+      RenderBR {BR} at (120,132) size 0x0
+      RenderText {#text} at (10,136) size 110x18
+        text run at (10,136) width 110: "This is some text."
+      RenderBR {BR} at (120,150) size 0x0
+      RenderText {#text} at (10,154) size 110x18
+        text run at (10,154) width 110: "This is some text."
+      RenderBR {BR} at (120,168) size 0x0
+      RenderText {#text} at (10,172) size 110x18
+        text run at (10,172) width 110: "This is some text."
+      RenderBR {BR} at (120,186) size 0x0
+      RenderText {#text} at (10,190) size 110x18
+        text run at (10,190) width 110: "This is some text."
+      RenderBR {BR} at (120,204) size 0x0
+      RenderText {#text} at (10,208) size 110x18
+        text run at (10,208) width 110: "This is some text."
+      RenderBR {BR} at (120,222) size 0x0
+      RenderImage {IMG} at (274,400) size 100x200 [bgcolor=#CCCCCC]
+      RenderText {#text} at (0,0) size 0x0

diff --git a/LayoutTests/platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt b/LayoutTests/platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt
index a46dd66..eb558eb 100644 (file)
--- a/LayoutTests/platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt
+++ b/LayoutTests/platform/mac/fast/regions/webkit-flow-float-pushed-to-last-region-expected.txt
@@ -15,11 +15,9 @@ Flow Threads
           RenderBlock {DIV} at (5,5) size 390x443 [border: (1px solid #0000FF)]
             RenderBlock {DIV} at (11,11) size 368x421 [border: (1px solid #008000)]
               RenderBlock {P} at (1,17) size 366x265
-                RenderText {#text} at (0,157) size 104x18
-                  text run at (0,157) width 104: "This line of text "
                 RenderImage {IMG} at (236,157) size 130x100 [bgcolor=#008000]
-                RenderText {#text} at (104,157) size 236x108
-                  text run at (104,157) width 132: "should not get out of"
+                RenderText {#text} at (0,157) size 236x108
+                  text run at (0,157) width 236: "This line of text should not get out of"
                   text run at (0,175) width 26: "the "
                   text run at (26,175) width 210: "region. This line of text should"
                   text run at (0,193) width 236: "not get out of the region. This line of"

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fe6ea41..0c50c3a 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2011-09-27  David Hyatt  <hyatt@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=68940
+
+        Narrow the float/lines pagination heuristic to only kick in if
+        the previous line broke cleanly and if the floats are occurring
+        at the start of the line.
+
+        Reviewed by Dan Bernstein.
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlock::positionNewFloatOnLine):
+
 2011-09-27  James Robinson  <jamesr@chromium.org>
 
         Add a mechanism to test for the compositing tree mutated during painting

diff --git a/Source/WebCore/rendering/RenderBlockLineLayout.cpp b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
index 498bd85..dd40f92 100644 (file)
--- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp
+++ b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
@@ -2653,7 +2653,10 @@ bool RenderBlock::positionNewFloatOnLine(FloatingObject* newFloat, FloatingObjec
 
     width.shrinkAvailableWidthForNewFloatIfNeeded(newFloat);
 
-    if (!newFloat->m_paginationStrut)
+    // We only connect floats to lines for pagination purposes if the floats occur at the start of
+    // the line and the previous line had a hard break (so this line is either the first in the block
+    // or follows a <br>).
+    if (!newFloat->m_paginationStrut || !lineInfo.previousLineBrokeCleanly() || !lineInfo.isEmpty())
         return true;
 
     const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
@@ -2689,16 +2692,9 @@ bool RenderBlock::positionNewFloatOnLine(FloatingObject* newFloat, FloatingObjec
         }
     }
 
-    if (lineInfo.isEmpty()) {
-        // Just update the line info's pagination strut without altering our logical height yet. If the line ends up containing
-        // no content, then we don't want to improperly grow the height of the block.
-        lineInfo.setFloatPaginationStrut(lineInfo.floatPaginationStrut() + paginationStrut);
-    } else {
-        // The line already has content on it, so we want to go ahead and shift everything down.
-        setLogicalHeight(logicalHeight() + paginationStrut);
-        width.updateAvailableWidth();
-    }
-
+    // Just update the line info's pagination strut without altering our logical height yet. If the line ends up containing
+    // no content, then we don't want to improperly grow the height of the block.
+    lineInfo.setFloatPaginationStrut(lineInfo.floatPaginationStrut() + paginationStrut);
     return true;
 }