https://bugs.webkit.org/show_bug.cgi?id=68590
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2011 16:33:40 +0000 (16:33 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2011 16:33:40 +0000 (16:33 +0000)
Floats pushed to next page, column or region don't reposition properly if the amount of
available logical width at the new position changes. Refactor the code so that we can
run the float placement algorithm again when this happens.

Source/WebCore:

Covered by an existing regions test that exposes the issue.

Reviewed by Adam Roben.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeLogicalLocationForFloat):
(WebCore::RenderBlock::positionNewFloats):
* rendering/RenderBlock.h:

LayoutTests:

Reviewed by Adam Roben.

* fast/regions/webkit-flow-floats-inside-regions-bounds-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/fast/regions/webkit-flow-floats-inside-regions-bounds-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h

index dca8ea5..f59139c 100644 (file)
@@ -1,3 +1,15 @@
+2011-09-21  David Hyatt  <hyatt@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=68590
+        
+        Floats pushed to next page, column or region don't reposition properly if the amount of
+        available logical width at the new position changes. Refactor the code so that we can
+        run the float placement algorithm again when this happens.
+
+        Reviewed by Adam Roben.
+
+        * fast/regions/webkit-flow-floats-inside-regions-bounds-expected.txt:
+
 2011-09-22  Csaba Osztrogonác  <ossy@webkit.org>
 
         [Qt] Unreviewed. Add Qt specific expected results for css3/selectors3 tests.
index 51ca2cc..ab62c93 100644 (file)
@@ -11,42 +11,39 @@ Flow Threads
   Thread with flow-name 'flow1'
     layer at (0,0) size 300x400
       RenderFlowThread at (0,0) size 300x400
-        RenderBlock {DIV} at (0,0) size 300x505
-          RenderBlock {DIV} at (5,5) size 290x495 [border: (1px solid #0000FF)]
-            RenderBlock {DIV} at (11,11) size 268x473 [border: (1px solid #008000)]
+        RenderBlock {DIV} at (0,0) size 300x469
+          RenderBlock {DIV} at (5,5) size 290x459 [border: (1px solid #0000FF)]
+            RenderBlock {DIV} at (11,11) size 268x437 [border: (1px solid #008000)]
               RenderBlock {P} at (1,17) size 266x36
                 RenderText {#text} at (0,0) size 166x36
                   text run at (0,0) width 166: "This line of text should"
                   text run at (0,18) width 155: "not get out of the region."
-              RenderBlock (floating) {DIV} at (95,84) size 72x37 [border: (1px solid #FF0000)]
+              RenderBlock (floating) {DIV} at (195,84) size 72x37 [border: (1px solid #FF0000)]
                 RenderText {#text} at (1,1) size 44x18
                   text run at (1,1) width 44: "Float 1"
-              RenderBlock (anonymous) at (1,84) size 266x108
-                RenderText {#text} at (0,0) size 266x108
+              RenderBlock (anonymous) at (1,84) size 266x90
+                RenderText {#text} at (0,0) size 266x90
                   text run at (0,0) width 166: "This line of text should"
-                  text run at (0,18) width 94: "not get out of"
-                  text run at (0,36) width 94: "the region."
-                  text run at (0,54) width 266: "This line of text should not get out of the"
-                  text run at (0,72) width 266: "region. This line of text should not get out"
-                  text run at (0,90) width 84: "of the region."
-              RenderBlock {P} at (1,208) size 266x0
-              RenderBlock {P} at (1,208) size 266x164
-                RenderText {#text} at (0,0) size 266x164
+                  text run at (0,18) width 194: "not get out of the region. This"
+                  text run at (0,36) width 194: "line of text should not get out"
+                  text run at (0,54) width 266: "of the region. This line of text should not"
+                  text run at (0,72) width 131: "get out of the region."
+              RenderBlock {P} at (1,190) size 266x0
+              RenderBlock {P} at (1,190) size 266x128
+                RenderText {#text} at (0,0) size 266x128
                   text run at (0,0) width 266: "This line of text should not get out of the"
                   text run at (0,18) width 266: "region. This line of text should not get out"
                   text run at (0,36) width 266: "of the region. This line of text should not"
-                  text run at (0,56) width 86: "get out of the"
-                  text run at (0,74) width 86: "region. This"
-                  text run at (0,92) width 86: "line of text"
-                  text run at (0,110) width 86: "should not"
-                  text run at (0,128) width 86: "get out of the"
-                  text run at (0,146) width 44: "region."
-              RenderBlock {P} at (1,384) size 266x72
-                RenderText {#text} at (0,0) size 86x72
+                  text run at (0,54) width 266: "get out of the region. This line of text"
+                  text run at (0,74) width 86: "should not"
+                  text run at (0,92) width 86: "get out of the"
+                  text run at (0,110) width 44: "region."
+              RenderBlock {P} at (1,334) size 266x86
+                RenderText {#text} at (0,0) size 86x86
                   text run at (0,0) width 86: "This line of"
                   text run at (0,18) width 86: "text should"
-                  text run at (0,36) width 86: "not get out of"
-                  text run at (0,54) width 67: "the region."
+                  text run at (0,50) width 86: "not get out of"
+                  text run at (0,68) width 67: "the region."
   Regions for flow 'flow1'
     RenderRegion {DIV} #region1 with index 0
     RenderRegion {DIV} #region2 with index 0
index 58c0fea..1bdc1b9 100644 (file)
@@ -1,3 +1,20 @@
+2011-09-21  David Hyatt  <hyatt@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=68590
+        
+        Floats pushed to next page, column or region don't reposition properly if the amount of
+        available logical width at the new position changes. Refactor the code so that we can
+        run the float placement algorithm again when this happens.
+
+        Covered by an existing regions test that exposes the issue.
+
+        Reviewed by Adam Roben.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::computeLogicalLocationForFloat):
+        (WebCore::RenderBlock::positionNewFloats):
+        * rendering/RenderBlock.h:
+
 2011-09-22  Leandro Gracia Gil  <leandrogracia@chromium.org>
 
         Fire TextInput events on speech input, but not set/add any inputMethod attribute.
index 6733718..b6e02cf 100644 (file)
@@ -3269,6 +3269,38 @@ void RenderBlock::removeFloatingObjectsBelow(FloatingObject* lastFloat, int logi
     }
 }
 
+LayoutPoint RenderBlock::computeLogicalLocationForFloat(const FloatingObject* floatingObject, LayoutUnit logicalTopOffset,
+    LayoutUnit logicalLeftOffset, LayoutUnit logicalRightOffset, LayoutUnit floatLogicalWidth) const
+{
+    RenderBox* childBox = floatingObject->renderer();
+
+    LayoutUnit floatLogicalLeft;
+
+    if (childBox->style()->floating() == LeftFloat) {
+        LayoutUnit heightRemainingLeft = 1;
+        LayoutUnit heightRemainingRight = 1;
+        floatLogicalLeft = logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft);
+        while (logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight) - floatLogicalLeft < floatLogicalWidth) {
+            logicalTopOffset += min(heightRemainingLeft, heightRemainingRight);
+            floatLogicalLeft = logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft);
+        }
+        floatLogicalLeft = max<LayoutUnit>(0, floatLogicalLeft);
+    } else {
+        LayoutUnit heightRemainingLeft = 1;
+        LayoutUnit heightRemainingRight = 1;
+        floatLogicalLeft = logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight);
+        while (floatLogicalLeft - logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft) < floatLogicalWidth) {
+            logicalTopOffset += min(heightRemainingLeft, heightRemainingRight);
+            floatLogicalLeft = logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight);
+        }
+        floatLogicalLeft -= logicalWidthForFloat(floatingObject); // Use the original width of the float here, since the local variable
+                                                                  // |floatLogicalWidth| was capped to the available line width.
+                                                                  // See fast/block/float/clamped-right-float.html.
+    }
+    
+    return LayoutPoint(floatLogicalLeft, logicalTopOffset);
+}
+
 bool RenderBlock::positionNewFloats()
 {
     if (!m_floatingObjects)
@@ -3307,7 +3339,7 @@ bool RenderBlock::positionNewFloats()
     FloatingObjectSetIterator end = floatingObjectSet.end();
     // Now walk through the set of unpositioned floats and place them.
     for (; it != end; ++it) {
-         FloatingObject* floatingObject = *it;
+        FloatingObject* floatingObject = *it;
         // The containing block is responsible for positioning floats, so if we have floats in our
         // list that come from somewhere else, do not attempt to position them. Also don't attempt to handle
         // positioned floats, since the positioning layout code handles those.
@@ -3330,32 +3362,11 @@ bool RenderBlock::positionNewFloats()
         if (childBox->style()->clear() & CRIGHT)
             logicalTop = max(lowestFloatLogicalBottom(FloatingObject::FloatRight), logicalTop);
 
-        LayoutUnit floatLogicalLeft;
-        if (childBox->style()->floating() == LeftFloat) {
-            LayoutUnit heightRemainingLeft = 1;
-            LayoutUnit heightRemainingRight = 1;
-            floatLogicalLeft = logicalLeftOffsetForLine(logicalTop, leftOffset, false, &heightRemainingLeft);
-            while (logicalRightOffsetForLine(logicalTop, rightOffset, false, &heightRemainingRight) - floatLogicalLeft < floatLogicalWidth) {
-                logicalTop += min(heightRemainingLeft, heightRemainingRight);
-                floatLogicalLeft = logicalLeftOffsetForLine(logicalTop, leftOffset, false, &heightRemainingLeft);
-            }
-            floatLogicalLeft = max<LayoutUnit>(0, floatLogicalLeft);
-        } else {
-            LayoutUnit heightRemainingLeft = 1;
-            LayoutUnit heightRemainingRight = 1;
-            floatLogicalLeft = logicalRightOffsetForLine(logicalTop, rightOffset, false, &heightRemainingRight);
-            while (floatLogicalLeft - logicalLeftOffsetForLine(logicalTop, leftOffset, false, &heightRemainingLeft) < floatLogicalWidth) {
-                logicalTop += min(heightRemainingLeft, heightRemainingRight);
-                floatLogicalLeft = logicalRightOffsetForLine(logicalTop, rightOffset, false, &heightRemainingRight);
-            }
-            floatLogicalLeft -= logicalWidthForFloat(floatingObject); // Use the original width of the float here, since the local variable
-                                                                      // |floatLogicalWidth| was capped to the available line width.
-                                                                      // See fast/block/float/clamped-right-float.html.
-        }
-        
-        setLogicalLeftForFloat(floatingObject, floatLogicalLeft);
-        setLogicalLeftForChild(childBox, floatLogicalLeft + childLogicalLeftMargin);
-        setLogicalTopForChild(childBox, logicalTop + marginBeforeForChild(childBox));
+        LayoutPoint floatLogicalLocation = computeLogicalLocationForFloat(floatingObject, logicalTop, leftOffset, rightOffset, floatLogicalWidth);
+
+        setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
+        setLogicalLeftForChild(childBox, floatLogicalLocation.x() + childLogicalLeftMargin);
+        setLogicalTopForChild(childBox, floatLogicalLocation.y() + marginBeforeForChild(childBox));
 
         if (view()->layoutState()->isPaginated()) {
             RenderBlock* childBlock = childBox->isRenderBlock() ? toRenderBlock(childBox) : 0;
@@ -3366,7 +3377,7 @@ bool RenderBlock::positionNewFloats()
 
             // If we are unsplittable and don't fit, then we need to move down.
             // We include our margins as part of the unsplittable area.
-            LayoutUnit newLogicalTop = adjustForUnsplittableChild(childBox, logicalTop, true);
+            LayoutUnit newLogicalTop = adjustForUnsplittableChild(childBox, floatLogicalLocation.y(), true);
             
             // See if we have a pagination strut that is making us move down further.
             // Note that an unsplittable child can't also have a pagination strut, so this is
@@ -3376,17 +3387,21 @@ bool RenderBlock::positionNewFloats()
                 childBlock->setPaginationStrut(0);
             }
             
-            if (newLogicalTop != logicalTop) {
-                floatingObject->m_paginationStrut = newLogicalTop - logicalTop;
-                logicalTop = newLogicalTop;
-                setLogicalTopForChild(childBox, logicalTop + marginBeforeForChild(childBox));
+            if (newLogicalTop != floatLogicalLocation.y()) {
+                floatingObject->m_paginationStrut = newLogicalTop - floatLogicalLocation.y();
+
+                floatLogicalLocation = computeLogicalLocationForFloat(floatingObject, newLogicalTop, leftOffset, rightOffset, floatLogicalWidth);
+                setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
+                setLogicalLeftForChild(childBox, floatLogicalLocation.x() + childLogicalLeftMargin);
+                setLogicalTopForChild(childBox, floatLogicalLocation.y() + marginBeforeForChild(childBox));
+        
                 if (childBlock)
                     childBlock->setChildNeedsLayout(true, false);
                 childBox->layoutIfNeeded();
             }
         }
 
-        setLogicalTopForFloat(floatingObject, logicalTop);
+        setLogicalTopForFloat(floatingObject, floatLogicalLocation.y());
         setLogicalHeightForFloat(floatingObject, logicalHeightForChild(childBox) + marginBeforeForChild(childBox) + marginAfterForChild(childBox));
 
         m_floatingObjects->addPlacedObject(floatingObject);
index 86817fa..3a4934d 100644 (file)
@@ -555,6 +555,9 @@ private:
             return child->y() + child->renderer()->marginTop();
     }
 
+    LayoutPoint computeLogicalLocationForFloat(const FloatingObject*, LayoutUnit logicalTopOffset, LayoutUnit logicalLeftOffset,
+        LayoutUnit logicalRightOffset, LayoutUnit floatLogicalWidth) const;
+
     // The following functions' implementations are in RenderBlockLineLayout.cpp.
     typedef std::pair<RenderText*, LazyLineBreakIterator> LineBreakIteratorInfo;
     class LineBreaker {