Changeset 163165 in webkit

Timestamp:

Jan 31, 2014 12:09:07 AM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Fix table sizing when 'max-width' is used.
​https://bugs.webkit.org/show_bug.cgi?id=115156

Patch by László Langó <​lango@inf.u-szeged.hu> on 2014-01-31
Reviewed by Andreas Kling.

Source/WebCore:

r143534 make <table> abide by 'max-width' all the time which is wrong.
Per the CSS specification, a table should be wide enough to fit its
content, regardless of 'max-width'.

r140479 fixed part of the regression from that change but made the
same fatal mistake by constraining min-content to fit 'max-width'.

The fix is to avoid constraining min-content and ensure that the table
logical width is at least its min-content size.

Backported from Blink:
​https://chromium.googlesource.com/chromium/blink/+/0bca0dec4895aeeb2054ba36316e984e4ebed06f

Test: fast/table/html-table-width-max-width-constrained.html

• rendering/RenderTable.cpp:

(WebCore::RenderTable::updateLogicalWidth):
(WebCore::RenderTable::computePreferredLogicalWidths):

LayoutTests:

• fast/table/css-table-max-width-expected.txt:
• fast/table/css-table-max-width.html:
• fast/table/html-table-width-max-width-constrained-expected.txt: Added.
• fast/table/html-table-width-max-width-constrained.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/table/css-table-max-width-expected.txt (modified) (1 diff)
• LayoutTests/fast/table/css-table-max-width.html (modified) (3 diffs)
• LayoutTests/fast/table/html-table-width-max-width-constrained-expected.txt (added)
• LayoutTests/fast/table/html-table-width-max-width-constrained.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderTable.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-01-31  László Langó  <lango@inf.u-szeged.hu>
+
+        Fix table sizing when 'max-width' is used.
+        https://bugs.webkit.org/show_bug.cgi?id=115156
+
+        Reviewed by Andreas Kling.
+
+        * fast/table/css-table-max-width-expected.txt:
+        * fast/table/css-table-max-width.html:
+        * fast/table/html-table-width-max-width-constrained-expected.txt: Added.
+        * fast/table/html-table-width-max-width-constrained.html: Added.
+
 2014-01-28  Timothy Hatcher  <timothy@apple.com>
 

trunk/LayoutTests/fast/table/css-table-max-width-expected.txt

@@ -11 +11 @@
 PASS minGreatThanMaxWidthAutoLayout.getBoundingClientRect().width is 202
 PASS onlyMaxWidthAutoLayout.getBoundingClientRect().width is 202
-PASS maxWidthZeroAutoLayout.getBoundingClientRect().width is 0
+PASS maxWidthZeroAutoLayout.getBoundingClientRect().width is 182
 PASS maxGreatThanMinWidthFixedLayout.getBoundingClientRect().width is 202
 PASS minGreatThanMaxWidthFixedLayout.getBoundingClientRect().width is 202
 PASS onlyMaxWidthFixedLayout.getBoundingClientRect().width is 202
-PASS maxWidthZeroFixedLayout.getBoundingClientRect().width is 0
+PASS maxWidthZeroFixedLayout.getBoundingClientRect().width is 2
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/table/css-table-max-width.html

@@ -43 +43 @@
         qui officia deserunt mollit anim id est laborum.
     </div>
-    <div id="maxWidthZeroAutoLayout" class="child" style="display:table; max-width:0; width:100%;">
+    <div id="maxWidthZeroAutoLayout" class="child" style="display:table; max-width:0; width:100%; font: 10px/1 Ahem;">
         Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod 
         tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, 
@@ -98 +98 @@
 shouldBe("onlyMaxWidthAutoLayout.getBoundingClientRect().width","202");
 maxWidthZeroAutoLayout = document.getElementById("maxWidthZeroAutoLayout");
-shouldBe("maxWidthZeroAutoLayout.getBoundingClientRect().width","0");
+shouldBe("maxWidthZeroAutoLayout.getBoundingClientRect().width","182");
 maxGreatThanMinWidthFixedLayout = document.getElementById("maxGreatThanMinWidthFixedLayout");
 shouldBe("maxGreatThanMinWidthFixedLayout.getBoundingClientRect().width","202");
@@ -106 +106 @@
 shouldBe("onlyMaxWidthFixedLayout.getBoundingClientRect().width","202");
 maxWidthZeroFixedLayout = document.getElementById("maxWidthZeroFixedLayout");
-shouldBe("maxWidthZeroFixedLayout.getBoundingClientRect().width","0");
+shouldBe("maxWidthZeroFixedLayout.getBoundingClientRect().width","2");
 
 document.body.removeChild(document.getElementById('container'));

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-31  László Langó  <lango@inf.u-szeged.hu>
+
+        Fix table sizing when 'max-width' is used.
+        https://bugs.webkit.org/show_bug.cgi?id=115156
+
+        Reviewed by Andreas Kling.
+
+        r143534 make <table> abide by 'max-width' all the time which is wrong.
+        Per the CSS specification, a table should be wide enough to fit its
+        content, regardless of 'max-width'.
+
+        r140479 fixed part of the regression from that change but made the
+        same fatal mistake by constraining min-content to fit 'max-width'.
+
+        The fix is to avoid constraining min-content and ensure that the table
+        logical width is at least its min-content size.
+
+        Backported from Blink:
+        https://chromium.googlesource.com/chromium/blink/+/0bca0dec4895aeeb2054ba36316e984e4ebed06f
+
+        Test: fast/table/html-table-width-max-width-constrained.html
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::updateLogicalWidth):
+        (WebCore::RenderTable::computePreferredLogicalWidths):
+
 2014-01-30  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/rendering/RenderTable.cpp

@@ -293 +293 @@
     }
 
-    // Ensure we aren't smaller than our min preferred width.
-    setLogicalWidth(std::max<int>(logicalWidth(), minPreferredLogicalWidth()));
-
-    
     // Ensure we aren't bigger than our max-width style.
     Length styleMaxLogicalWidth = style().logicalMaxWidth();
@@ -303 +299 @@
         setLogicalWidth(std::min<int>(logicalWidth(), computedMaxLogicalWidth));
     }
+    
+    // Ensure we aren't smaller than our min preferred width. This MUST be done after 'max-width' as
+    // we ignore it if it means we wouldn't accomodate our content.
+    setLogicalWidth(std::max<int>(logicalWidth(), minPreferredLogicalWidth()));
 
     // Ensure we aren't smaller than our min-width style.
@@ -329 +329 @@
         setMarginEnd(minimumValueForLength(style().marginEnd(), availableLogicalWidth));
     }
+
+    // We should NEVER shrink the table below the min-content logical width, or else the table can't accomodate
+    // its own content which doesn't match CSS nor what authors expect.
+    ASSERT(logicalWidth() >= minPreferredLogicalWidth());
 }
 
@@ -779 +783 @@
     // FIXME: This should probably be checking for isSpecified since you should be able to use percentage, calc or viewport relative values for maxWidth.
     if (styleToUse.logicalMaxWidth().isFixed()) {
+        // We don't constrain m_minPreferredLogicalWidth as the table should be at least the size of its min-content, regardless of 'max-width'.
         m_maxPreferredLogicalWidth = std::min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse.logicalMaxWidth().value()));
-        m_minPreferredLogicalWidth = std::min(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse.logicalMaxWidth().value()));
+        m_minPreferredLogicalWidth = std::max(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
     }