Title: [136241] branches/chromium/1312
Revision
136241
Author
joc...@chromium.org
Date
2012-11-30 07:32:36 -0800 (Fri, 30 Nov 2012)

Log Message

Merge 135578 - REGRESSION (r128633): td changes size during re-layout of table although it shouldn't
https://bugs.webkit.org/show_bug.cgi?id=102802

Patch by Julian Pastarmov <pastarm...@chromium.org> on 2012-11-23
Reviewed by Ojan Vafai.

Source/WebCore:

The bug was caused by incorrectly rewriting a nested condition which caused the else
clause to trigger in wrong cases.

Test: fast/table/nested-tables-with-div-offset.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computePercentageLogicalHeight):
Reverted wrong combination of nested if statements.

LayoutTests:

This test checks that table layout of some nested tables in quirks mode is computed correctly.

* fast/table/nested-tables-with-div-offset-expected.txt: Added.
* fast/table/nested-tables-with-div-offset.html: Added.

TBR=commit-qu...@webkit.org
Review URL: https://codereview.chromium.org/11414259

Modified Paths

Added Paths

Diff

Copied: branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset-expected.txt (from rev 135578, trunk/LayoutTests/fast/table/nested-tables-with-div-offset-expected.txt) (0 => 136241)


--- branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset-expected.txt	                        (rev 0)
+++ branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset-expected.txt	2012-11-30 15:32:36 UTC (rev 136241)
@@ -0,0 +1 @@
+PASS

Copied: branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset.html (from rev 135578, trunk/LayoutTests/fast/table/nested-tables-with-div-offset.html) (0 => 136241)


--- branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset.html	                        (rev 0)
+++ branches/chromium/1312/LayoutTests/fast/table/nested-tables-with-div-offset.html	2012-11-30 15:32:36 UTC (rev 136241)
@@ -0,0 +1,34 @@
+<script src=""
+<body _onload_="checkLayout('.quirky-box');">
+<table height="100%">
+    <tbody>
+        <tr>
+            <td>
+                <table class="quirky-box" height="100%" data-offset-y="1">
+                    <tbody>
+                        <tr>
+                            <td style="height: 100px;"></td>
+                        </tr>
+                        <tr>
+                            <td>
+                                <div style="height:350px;"></div>
+                            </td>
+                            <td style="height: 100%; vertical-align: top; width: 100%;">
+                                <div>
+                                    <table height="100%">
+                                        <tbody>
+                                            <tr>
+                                            <td height="100%" valign="top"></td>
+                                            </tr>
+                                        </tbody>
+                                    </table>
+                                </div>
+                            </td>
+                        </tr>
+                    </tbody>
+                </table>
+            </td>
+        </tr>
+    </tbody>
+</table>
+</body>

Modified: branches/chromium/1312/Source/WebCore/rendering/RenderBox.cpp (136240 => 136241)


--- branches/chromium/1312/Source/WebCore/rendering/RenderBox.cpp	2012-11-30 15:25:27 UTC (rev 136240)
+++ branches/chromium/1312/Source/WebCore/rendering/RenderBox.cpp	2012-11-30 15:32:36 UTC (rev 136241)
@@ -2181,25 +2181,27 @@
 
     if (isHorizontalWritingMode() != cb->isHorizontalWritingMode())
         availableHeight = cb->contentLogicalWidth();
-    else if (cb->isTableCell() && !skippedAutoHeightContainingBlock) {
-        // Table cells violate what the CSS spec says to do with heights. Basically we
-        // don't care if the cell specified a height or not. We just always make ourselves
-        // be a percentage of the cell's current content height.
-        if (!cb->hasOverrideHeight()) {
-            // Normally we would let the cell size intrinsically, but scrolling overflow has to be
-            // treated differently, since WinIE lets scrolled overflow regions shrink as needed.
-            // While we can't get all cases right, we can at least detect when the cell has a specified
-            // height or when the table has a specified height. In these cases we want to initially have
-            // no size and allow the flexing of the table or the cell to its specified height to cause us
-            // to grow to fill the space. This could end up being wrong in some cases, but it is
-            // preferable to the alternative (sizing intrinsically and making the row end up too big).
-            RenderTableCell* cell = toRenderTableCell(cb);
-            if (scrollsOverflowY() && (!cell->style()->logicalHeight().isAuto() || !cell->table()->style()->logicalHeight().isAuto()))
-                return 0;
-            return -1;
+    else if (cb->isTableCell()) {
+        if (!skippedAutoHeightContainingBlock) {
+            // Table cells violate what the CSS spec says to do with heights. Basically we
+            // don't care if the cell specified a height or not. We just always make ourselves
+            // be a percentage of the cell's current content height.
+            if (!cb->hasOverrideHeight()) {
+                // Normally we would let the cell size intrinsically, but scrolling overflow has to be
+                // treated differently, since WinIE lets scrolled overflow regions shrink as needed.
+                // While we can't get all cases right, we can at least detect when the cell has a specified
+                // height or when the table has a specified height. In these cases we want to initially have
+                // no size and allow the flexing of the table or the cell to its specified height to cause us
+                // to grow to fill the space. This could end up being wrong in some cases, but it is
+                // preferable to the alternative (sizing intrinsically and making the row end up too big).
+                RenderTableCell* cell = toRenderTableCell(cb);
+                if (scrollsOverflowY() && (!cell->style()->logicalHeight().isAuto() || !cell->table()->style()->logicalHeight().isAuto()))
+                    return 0;
+                return -1;
+            }
+            availableHeight = cb->overrideLogicalContentHeight();
+            includeBorderPadding = true;
         }
-        availableHeight = cb->overrideLogicalContentHeight();
-        includeBorderPadding = true;
     } else if (cbstyle->logicalHeight().isFixed()) {
         LayoutUnit contentBoxHeightWithScrollbar = cb->adjustContentBoxLogicalHeightForBoxSizing(cbstyle->logicalHeight().value());
         availableHeight = max<LayoutUnit>(0, contentBoxHeightWithScrollbar - cb->scrollbarLogicalHeight());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to