WebCore:
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2009 17:56:41 +0000 (17:56 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2009 17:56:41 +0000 (17:56 +0000)
2009-03-03  David Hyatt  <hyatt@apple.com>

        https://bugs.webkit.org/show_bug.cgi?id=24201, pathologically bad block layout.

        Make sure to factor clear deltas into y position estimates.  Also avoid doing the comparison of
        the final position against the y position estimate until after the clear has happened.  This gets rid
        of some duplicated cut/pasted code and also ensures a layout delta only has to be put in once.

        Reviewed by Dan Bernstein

        Added fast/block/float/nested-clearance.html

        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::collapseMargins):
        (WebCore::RenderBlock::clearFloatsIfNeeded):
        (WebCore::RenderBlock::estimateVerticalPosition):
        (WebCore::RenderBlock::layoutBlockChildren):
        (WebCore::RenderBlock::getClearDelta):
        * rendering/RenderBlock.h:

LayoutTests:

2009-03-03  David Hyatt  <hyatt@apple.com>

        Test case for https://bugs.webkit.org/show_bug.cgi?id=24201

        Reviewed by Dan Bernstein

        * fast/block/float/nested-clearance.html: Added.
        * platform/mac/fast/block/float/nested-clearance-expected.checksum: Added.
        * platform/mac/fast/block/float/nested-clearance-expected.png: Added.
        * platform/mac/fast/block/float/nested-clearance-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/block/float/nested-clearance.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/Element.cpp
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RenderBlock.h

index 1c24e16c348d28619bdb0797e2b78d0e678d2104..058d9675f26ea8d8644d7cb1ca79c6c22defbdf1 100644 (file)
@@ -1,3 +1,14 @@
+2009-03-03  David Hyatt  <hyatt@apple.com>
+
+        Test case for https://bugs.webkit.org/show_bug.cgi?id=24201
+
+        Reviewed by Dan Bernstein
+
+        * fast/block/float/nested-clearance.html: Added.
+        * platform/mac/fast/block/float/nested-clearance-expected.checksum: Added.
+        * platform/mac/fast/block/float/nested-clearance-expected.png: Added.
+        * platform/mac/fast/block/float/nested-clearance-expected.txt: Added.
+
 2009-03-03  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by John Sullivan.
diff --git a/LayoutTests/fast/block/float/nested-clearance.html b/LayoutTests/fast/block/float/nested-clearance.html
new file mode 100644 (file)
index 0000000..12a6974
--- /dev/null
@@ -0,0 +1,91 @@
+<!DOCTYPE html>
+<html> <body>
+  <div style="float:left">
+    <img height='64' width='64'>
+  </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+  <div style="clear:both;">
+    <div style="float:left">
+      <img height='64' width='64'>
+    </div>
+
+</body> </html>
diff --git a/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.checksum b/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.checksum
new file mode 100644 (file)
index 0000000..5b184e5
--- /dev/null
@@ -0,0 +1 @@
+c15ec04c85c42ed8ce61307a8eb0c3e1
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.png b/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.png
new file mode 100644 (file)
index 0000000..e2d77a2
Binary files /dev/null and b/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.txt b/LayoutTests/platform/mac/fast/block/float/nested-clearance-expected.txt
new file mode 100644 (file)
index 0000000..48aca7c
--- /dev/null
@@ -0,0 +1,92 @@
+layer at (0,0) size 785x1504
+  RenderView at (0,0) size 785x600
+layer at (0,0) size 785x1504
+  RenderBlock {HTML} at (0,0) size 785x1444
+    RenderBody {BODY} at (8,8) size 769x1428
+      RenderBlock (floating) {DIV} at (0,0) size 64x68
+        RenderImage {IMG} at (0,0) size 64x64
+        RenderText {#text} at (0,0) size 0x0
+      RenderBlock {DIV} at (0,68) size 769x1360
+        RenderBlock (floating) {DIV} at (0,0) size 64x68
+          RenderImage {IMG} at (0,0) size 64x64
+          RenderText {#text} at (0,0) size 0x0
+        RenderBlock {DIV} at (0,68) size 769x1292
+          RenderBlock (floating) {DIV} at (0,0) size 64x68
+            RenderImage {IMG} at (0,0) size 64x64
+            RenderText {#text} at (0,0) size 0x0
+          RenderBlock {DIV} at (0,68) size 769x1224
+            RenderBlock (floating) {DIV} at (0,0) size 64x68
+              RenderImage {IMG} at (0,0) size 64x64
+              RenderText {#text} at (0,0) size 0x0
+            RenderBlock {DIV} at (0,68) size 769x1156
+              RenderBlock (floating) {DIV} at (0,0) size 64x68
+                RenderImage {IMG} at (0,0) size 64x64
+                RenderText {#text} at (0,0) size 0x0
+              RenderBlock {DIV} at (0,68) size 769x1088
+                RenderBlock (floating) {DIV} at (0,0) size 64x68
+                  RenderImage {IMG} at (0,0) size 64x64
+                  RenderText {#text} at (0,0) size 0x0
+                RenderBlock {DIV} at (0,68) size 769x1020
+                  RenderBlock (floating) {DIV} at (0,0) size 64x68
+                    RenderImage {IMG} at (0,0) size 64x64
+                    RenderText {#text} at (0,0) size 0x0
+                  RenderBlock {DIV} at (0,68) size 769x952
+                    RenderBlock (floating) {DIV} at (0,0) size 64x68
+                      RenderImage {IMG} at (0,0) size 64x64
+                      RenderText {#text} at (0,0) size 0x0
+                    RenderBlock {DIV} at (0,68) size 769x884
+                      RenderBlock (floating) {DIV} at (0,0) size 64x68
+                        RenderImage {IMG} at (0,0) size 64x64
+                        RenderText {#text} at (0,0) size 0x0
+                      RenderBlock {DIV} at (0,68) size 769x816
+                        RenderBlock (floating) {DIV} at (0,0) size 64x68
+                          RenderImage {IMG} at (0,0) size 64x64
+                          RenderText {#text} at (0,0) size 0x0
+                        RenderBlock {DIV} at (0,68) size 769x748
+                          RenderBlock (floating) {DIV} at (0,0) size 64x68
+                            RenderImage {IMG} at (0,0) size 64x64
+                            RenderText {#text} at (0,0) size 0x0
+                          RenderBlock {DIV} at (0,68) size 769x680
+                            RenderBlock (floating) {DIV} at (0,0) size 64x68
+                              RenderImage {IMG} at (0,0) size 64x64
+                              RenderText {#text} at (0,0) size 0x0
+                            RenderBlock {DIV} at (0,68) size 769x612
+                              RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                RenderImage {IMG} at (0,0) size 64x64
+                                RenderText {#text} at (0,0) size 0x0
+                              RenderBlock {DIV} at (0,68) size 769x544
+                                RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                  RenderImage {IMG} at (0,0) size 64x64
+                                  RenderText {#text} at (0,0) size 0x0
+                                RenderBlock {DIV} at (0,68) size 769x476
+                                  RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                    RenderImage {IMG} at (0,0) size 64x64
+                                    RenderText {#text} at (0,0) size 0x0
+                                  RenderBlock {DIV} at (0,68) size 769x408
+                                    RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                      RenderImage {IMG} at (0,0) size 64x64
+                                      RenderText {#text} at (0,0) size 0x0
+                                    RenderBlock {DIV} at (0,68) size 769x340
+                                      RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                        RenderImage {IMG} at (0,0) size 64x64
+                                        RenderText {#text} at (0,0) size 0x0
+                                      RenderBlock {DIV} at (0,68) size 769x272
+                                        RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                          RenderImage {IMG} at (0,0) size 64x64
+                                          RenderText {#text} at (0,0) size 0x0
+                                        RenderBlock {DIV} at (0,68) size 769x204
+                                          RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                            RenderImage {IMG} at (0,0) size 64x64
+                                            RenderText {#text} at (0,0) size 0x0
+                                          RenderBlock {DIV} at (0,68) size 769x136
+                                            RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                              RenderImage {IMG} at (0,0) size 64x64
+                                              RenderText {#text} at (0,0) size 0x0
+                                            RenderBlock {DIV} at (0,68) size 769x68
+                                              RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                                RenderImage {IMG} at (0,0) size 64x64
+                                                RenderText {#text} at (0,0) size 0x0
+                                              RenderBlock {DIV} at (0,68) size 769x0
+                                                RenderBlock (floating) {DIV} at (0,0) size 64x68
+                                                  RenderImage {IMG} at (0,0) size 64x64
+                                                  RenderText {#text} at (0,0) size 0x0
index df22d4a97ffd10fc3e11dd6213369e868a940fb3..c47f733826cd826da98c83911de2e973419681b0 100644 (file)
@@ -1,3 +1,23 @@
+2009-03-03  David Hyatt  <hyatt@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=24201, pathologically bad block layout.
+
+        Make sure to factor clear deltas into y position estimates.  Also avoid doing the comparison of
+        the final position against the y position estimate until after the clear has happened.  This gets rid
+        of some duplicated cut/pasted code and also ensures a layout delta only has to be put in once.
+
+        Reviewed by Dan Bernstein
+
+        Added fast/block/float/nested-clearance.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::collapseMargins):
+        (WebCore::RenderBlock::clearFloatsIfNeeded):
+        (WebCore::RenderBlock::estimateVerticalPosition):
+        (WebCore::RenderBlock::layoutBlockChildren):
+        (WebCore::RenderBlock::getClearDelta):
+        * rendering/RenderBlock.h:
+
 2009-03-02  Kim Grönholm  <kim.gronholm@nomovok.com>
 
         Reviewed by Simon Hausmann.
index 2ebe446f8f9ed700a56d37d04644c21ebc4d386e..780130cce76a49a1b2ad2745d38c653fe00beca3 100644 (file)
@@ -346,7 +346,7 @@ int Element::clientWidth()
     if ((!inCompatMode && document()->documentElement() == this) ||
         (inCompatMode && isHTMLElement() && document()->body() == this)) {
         if (FrameView* view = document()->view())
-            return view->layoutWidth();
+            return adjustForAbsoluteZoom(view->layoutWidth(), document()->renderer());
     }
     
     if (RenderBox* rend = renderBox())
@@ -365,7 +365,7 @@ int Element::clientHeight()
     if ((!inCompatMode && document()->documentElement() == this) ||
         (inCompatMode && isHTMLElement() && document()->body() == this)) {
         if (FrameView* view = document()->view())
-            return view->layoutHeight();
+            return adjustForAbsoluteZoom(view->layoutHeight(), document()->renderer());
     }
     
     if (RenderBox* rend = renderBox())
index 45bcb273493b25981cc57602318386c3e6d6a06e..20ae142dfd2f903cf6143936c328da9f7953f114 100644 (file)
@@ -1022,7 +1022,7 @@ RenderBox* RenderBlock::handleRunInChild(RenderBox* child, bool& handled)
     return 0;
 }
 
-void RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo, int yPosEstimate)
+int RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo)
 {
     // Get our max pos and neg top margins.
     int posTop = child->maxTopMargin(true);
@@ -1110,34 +1110,15 @@ void RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo, int
 
         marginInfo.setSelfCollapsingBlockClearedFloat(false);
     }
-
-    view()->addLayoutDelta(IntSize(0, yPosEstimate - ypos));
-    child->setLocation(child->x(), ypos);
-    if (ypos != yPosEstimate) {
-        if (child->shrinkToAvoidFloats())
-            // The child's width depends on the line width.
-            // When the child shifts to clear an item, its width can
-            // change (because it has more available line width).
-            // So go ahead and mark the item as dirty.
-            child->setChildNeedsLayout(true, false);
-
-        if (!child->avoidsFloats() && child->isBlockFlow() && toRenderBlock(child)->containsFloats())
-            toRenderBlock(child)->markAllDescendantsWithFloatsForLayout();
-
-        // Our guess was wrong. Make the child lay itself out again.
-        child->layoutIfNeeded();
-    }
+    
+    return ypos;
 }
 
-void RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& marginInfo, int oldTopPosMargin, int oldTopNegMargin)
+int RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& marginInfo, int oldTopPosMargin, int oldTopNegMargin, int yPos)
 {
-    int heightIncrease = getClearDelta(child);
+    int heightIncrease = getClearDelta(child, yPos);
     if (!heightIncrease)
-        return;
-
-    // The child needs to be lowered.  Move the child so that it just clears the float.
-    view()->addLayoutDelta(IntSize(0, -heightIncrease));
-    child->setLocation(child->x(), child->y() + heightIncrease);
+        return yPos;
 
     if (child->isSelfCollapsingBlock()) {
         // For self-collapsing blocks that clear, they can still collapse their
@@ -1166,19 +1147,8 @@ void RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& marginInfo,
         setMaxTopMargins(oldTopPosMargin, oldTopNegMargin);
         marginInfo.setAtTopOfBlock(false);
     }
-
-    // If our value of clear caused us to be repositioned vertically to be
-    // underneath a float, we might have to do another layout to take into account
-    // the extra space we now have available.
-    if (child->shrinkToAvoidFloats())
-        // The child's width depends on the line width.
-        // When the child shifts to clear an item, its width can
-        // change (because it has more available line width).
-        // So go ahead and mark the item as dirty.
-        child->setChildNeedsLayout(true, false);
-    if (!child->avoidsFloats() && child->isBlockFlow() && toRenderBlock(child)->containsFloats())
-        toRenderBlock(child)->markAllDescendantsWithFloatsForLayout();
-    child->layoutIfNeeded();
+    
+    return yPos + heightIncrease;
 }
 
 int RenderBlock::estimateVerticalPosition(RenderBox* child, const MarginInfo& marginInfo)
@@ -1190,6 +1160,7 @@ int RenderBlock::estimateVerticalPosition(RenderBox* child, const MarginInfo& ma
         int childMarginTop = child->selfNeedsLayout() ? child->marginTop() : child->collapsedMarginTop();
         yPosEstimate += max(marginInfo.margin(), childMarginTop);
     }
+    yPosEstimate += getClearDelta(child, yPosEstimate);
     return yPosEstimate;
 }
 
@@ -1392,7 +1363,7 @@ void RenderBlock::layoutBlockChildren(bool relayoutChildren, int& maxFloatBottom
             // If an element might be affected by the presence of floats, then always mark it for
             // layout.
             int fb = max(previousFloatBottom, floatBottom());
-            if (fb > height() || fb > yPosEstimate)
+            if (fb > yPosEstimate)
                 markDescendantsWithFloats = true;
         }
 
@@ -1410,10 +1381,28 @@ void RenderBlock::layoutBlockChildren(bool relayoutChildren, int& maxFloatBottom
 
         // Now determine the correct ypos based off examination of collapsing margin
         // values.
-        collapseMargins(child, marginInfo, yPosEstimate);
+        int yBeforeClear = collapseMargins(child, marginInfo);
 
         // Now check for clear.
-        clearFloatsIfNeeded(child, marginInfo, oldTopPosMargin, oldTopNegMargin);
+        int yAfterClear = clearFloatsIfNeeded(child, marginInfo, oldTopPosMargin, oldTopNegMargin, yBeforeClear);
+        
+        view()->addLayoutDelta(IntSize(0, yPosEstimate - yAfterClear));
+        child->setLocation(child->x(), yAfterClear);
+    
+        // Now we have a final y position.  See if it really does end up being different from our estimate.
+        if (yAfterClear != yPosEstimate) {
+            if (child->shrinkToAvoidFloats()) {
+                // The child's width depends on the line width.
+                // When the child shifts to clear an item, its width can
+                // change (because it has more available line width).
+                // So go ahead and mark the item as dirty.
+                child->setChildNeedsLayout(true, false);
+            }
+            if (!child->avoidsFloats() && child->isBlockFlow() && toRenderBlock(child)->containsFloats())
+                toRenderBlock(child)->markAllDescendantsWithFloatsForLayout();
+            // Our guess was wrong. Make the child lay itself out again.
+            child->layoutIfNeeded();
+        }
 
         // We are no longer at the top of the block if we encounter a non-empty child.  
         // This has to be done after checking for clear, so that margins can be reset if a clear occurred.
@@ -3178,7 +3167,7 @@ void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove
     }
 }
 
-int RenderBlock::getClearDelta(RenderBox* child)
+int RenderBlock::getClearDelta(RenderBox* child, int yPos)
 {
     // There is no need to compute clearance if we have no floats.
     if (!containsFloats())
@@ -3206,11 +3195,11 @@ int RenderBlock::getClearDelta(RenderBox* child)
     // to fit) and not all (we should be using nextFloatBottomBelow and looping).
     // Do not allow tables to wrap in quirks or even in almost strict mode 
     // (ebay on the PLT, finance.yahoo.com in the real world, versiontracker.com forces even almost strict mode not to work)
-    int result = clearSet ? max(0, bottom - child->y()) : 0;
+    int result = clearSet ? max(0, bottom - yPos) : 0;
     if (!result && child->avoidsFloats() && child->style()->width().isFixed() && 
-        child->minPrefWidth() > lineWidth(child->y(), false) && child->minPrefWidth() <= availableWidth() && 
+        child->minPrefWidth() > lineWidth(yPos, false) && child->minPrefWidth() <= availableWidth() && 
         document()->inStrictMode())   
-        result = max(0, floatBottom() - child->y());
+        result = max(0, floatBottom() - yPos);
     return result;
 }
 
index 4d8f38682b4764a21b9905bf759c19e119202d32..a8c7fc7cffba1e33fdc5af918dd7b3fbdf003bf6 100644 (file)
@@ -194,7 +194,7 @@ public:
     // Returns ture if and only if it has positioned any floats.
     bool positionNewFloats();
     void clearFloats();
-    int getClearDelta(RenderBox* child);
+    int getClearDelta(RenderBox* child, int yPos);
     void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0, bool inLayout = true);
     void markPositionedObjectsForLayout();
 
@@ -454,8 +454,8 @@ protected:
     RenderBox* handleFloatingChild(RenderBox* child, const MarginInfo&, bool& handled);
     RenderBox* handlePositionedChild(RenderBox* child, const MarginInfo&, bool& handled);
     RenderBox* handleRunInChild(RenderBox* child, bool& handled);
-    void collapseMargins(RenderBox* child, MarginInfo&, int yPosEstimate);
-    void clearFloatsIfNeeded(RenderBox* child, MarginInfo&, int oldTopPosMargin, int oldTopNegMargin);
+    int collapseMargins(RenderBox* child, MarginInfo&);
+    int clearFloatsIfNeeded(RenderBox* child, MarginInfo&, int oldTopPosMargin, int oldTopNegMargin, int yPos);
     int estimateVerticalPosition(RenderBox* child, const MarginInfo&);
     void determineHorizontalPosition(RenderBox* child);
     void handleBottomOfBlock(int top, int bottom, MarginInfo&);