Increase the max preferred width of tables to 1000000
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2013 22:38:18 +0000 (22:38 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2013 22:38:18 +0000 (22:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110545

Reviewed by Emil A Eklund.

Source/WebCore:

Test: fast/table/large-shrink-wrapped-width.html

The old limit of 15000 was picked arbitrarily to avoid overflow.
No need to pick such a small number.

* rendering/AutoTableLayout.cpp:
(WebCore::AutoTableLayout::computeInstrinsicLogicalWidths):
As best I can tell, this code is just wrong. Removing this makes the
new test pass and doesn't cause any test failure. While we have many
tests that hit this codepath, this code would only
show a difference in the cases where the available container width is
greater then 15000, and I don't think we have any tests that hit that case
other than this new one.
* rendering/TableLayout.h:

LayoutTests:

* fast/table/large-shrink-wrapped-width-expected.txt: Added.
* fast/table/large-shrink-wrapped-width.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/table/large-shrink-wrapped-width-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/large-shrink-wrapped-width.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/AutoTableLayout.cpp
Source/WebCore/rendering/TableLayout.h

index 2f961df..ce32c7f 100644 (file)
@@ -1,3 +1,13 @@
+2013-02-21  Ojan Vafai  <ojan@chromium.org>
+
+        Increase the max preferred width of tables to 1000000
+        https://bugs.webkit.org/show_bug.cgi?id=110545
+
+        Reviewed by Emil A Eklund.
+
+        * fast/table/large-shrink-wrapped-width-expected.txt: Added.
+        * fast/table/large-shrink-wrapped-width.html: Added.
+
 2013-02-22  Ian Vollick  <vollick@chromium.org>
 
         [chromium] Update test expectations for XP after 143554
diff --git a/LayoutTests/fast/table/large-shrink-wrapped-width-expected.txt b/LayoutTests/fast/table/large-shrink-wrapped-width-expected.txt
new file mode 100644 (file)
index 0000000..74c6471
--- /dev/null
@@ -0,0 +1,15 @@
+Content 
+FAIL:
+Expected 2000000 for width, but got 1000010. 
+
+<div class="container">
+    <div style="display:inline-block; border: 5px solid salmon;" data-expected-width="2000000">
+        <table style="width:100%; background-color:green; table-layout:fixed"><tbody><tr><td>Content</td></tr></tbody></table>
+    </div>
+</div>
+PASS
+PASS
+PASS
diff --git a/LayoutTests/fast/table/large-shrink-wrapped-width.html b/LayoutTests/fast/table/large-shrink-wrapped-width.html
new file mode 100644 (file)
index 0000000..618ef95
--- /dev/null
@@ -0,0 +1,62 @@
+<style>
+.container {
+    width: 2000000px;
+}
+.child {
+    display:inline-block;
+    background-color: orange;
+    height: 10px;
+}
+td {
+    padding: 0;
+}
+table {
+    border-spacing: 0;
+    display: inline-table;
+    outline: 5px solid salmon;
+}
+</style>
+
+<!-- The inner div should fill the container. It doesn't right now because we artificially limit the table's maxPreferredLogicalWidth to 1000000.
+    See the related FIXME in TableLayout.h.
+ -->
+<div class="container">
+    <div style="display:inline-block; border: 5px solid salmon;" data-expected-width=2000000>
+        <table style="width:100%; background-color:green; table-layout:fixed"><tr><td>Content</td></tr></table>
+    </div>
+</div>
+
+<div class="container">
+    <!-- The 1 px extra is from the align=right td. -->
+    <table data-expected-width=20001>
+        <td align=right></td>
+        <td width="100%">
+            <div class="child" style="width: 10000px"></div><div class="child" style="width: 10000px"></div>
+        </td>
+    </table>
+</div>
+
+<div class="container">
+    <!-- The 1 px extra is from the align=right td. -->
+    <table data-expected-width=1000001>
+        <td align=right></td>
+        <td width="100%">
+            <div class="child" style="width: 500000px"></div><div class="child" style="width: 500000px"></div>
+        </td>
+    </table>
+</div>
+
+<div class="container">
+    <!-- The 1 px extra is from the align=right td. -->
+    <table data-expected-width=1500001>
+        <td align=right></td>
+        <td width="100%">
+            <div class="child" style="width: 500000px"></div><div class="child" style="width: 500000px"></div><div class="child" style="width: 500000px"></div>
+        </td>
+    </table>
+</div>
+
+<script src="../../resources/check-layout.js"></script>
+<script>
+checkLayout('.container');
+</script>
index 18c7a6c..a21ff44 100644 (file)
@@ -1,3 +1,25 @@
+2013-02-21  Ojan Vafai  <ojan@chromium.org>
+
+        Increase the max preferred width of tables to 1000000
+        https://bugs.webkit.org/show_bug.cgi?id=110545
+
+        Reviewed by Emil A Eklund.
+
+        Test: fast/table/large-shrink-wrapped-width.html
+
+        The old limit of 15000 was picked arbitrarily to avoid overflow.
+        No need to pick such a small number.
+
+        * rendering/AutoTableLayout.cpp:
+        (WebCore::AutoTableLayout::computeInstrinsicLogicalWidths):
+        As best I can tell, this code is just wrong. Removing this makes the
+        new test pass and doesn't cause any test failure. While we have many
+        tests that hit this codepath, this code would only
+        show a difference in the cases where the available container width is
+        greater then 15000, and I don't think we have any tests that hit that case
+        other than this new one.
+        * rendering/TableLayout.h:
+
 2013-02-22  Adam Barth  <abarth@webkit.org>
 
         Threaded HTML parser should pass fast/parser/iframe-sets-parent-to-javascript-url.html
index 96a707b..d40dc5b 100644 (file)
@@ -247,10 +247,6 @@ void AutoTableLayout::computeIntrinsicLogicalWidths(LayoutUnit& minWidth, Layout
     }
 
     maxWidth = max<int>(maxWidth, spanMaxLogicalWidth);
-
-    // If there was no remaining percent, maxWidth is invalid
-    if (!remainingPercent && maxNonPercent)
-        maxWidth = tableMaxWidth;
 }
 
 void AutoTableLayout::applyPreferredLogicalWidthQuirks(LayoutUnit& minWidth, LayoutUnit& maxWidth) const
index 27393d4..67472b9 100644 (file)
@@ -43,7 +43,9 @@ public:
     virtual void layout() = 0;
 
 protected:
-    const static int tableMaxWidth = 15000;
+    // FIXME: Once we enable SATURATED_LAYOUT_ARITHMETHIC, this should just be LayoutUnit::nearlyMax().
+    // Until then though, using nearlyMax causes overflow in some tests, so we just pick a large number.
+    const static int tableMaxWidth = 1000000;
 
     RenderTable* m_table;
 };