- Revision
- 163165
- Author
- [email protected]
- Date
- 2014-01-31 00:09:07 -0800 (Fri, 31 Jan 2014)
Log Message
Fix table sizing when 'max-width' is used.
https://bugs.webkit.org/show_bug.cgi?id=115156
Patch by László Langó <[email protected]> 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (163164 => 163165)
--- trunk/LayoutTests/ChangeLog 2014-01-31 07:51:17 UTC (rev 163164)
+++ trunk/LayoutTests/ChangeLog 2014-01-31 08:09:07 UTC (rev 163165)
@@ -1,3 +1,15 @@
+2014-01-31 László Langó <[email protected]>
+
+ 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 <[email protected]>
Add column number and call timing support to LegacyProfiler.
Modified: trunk/LayoutTests/fast/table/css-table-max-width-expected.txt (163164 => 163165)
--- trunk/LayoutTests/fast/table/css-table-max-width-expected.txt 2014-01-31 07:51:17 UTC (rev 163164)
+++ trunk/LayoutTests/fast/table/css-table-max-width-expected.txt 2014-01-31 08:09:07 UTC (rev 163165)
@@ -10,11 +10,11 @@
PASS maxGreatThanMinWidthAutoLayout.getBoundingClientRect().width is 202
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
TEST COMPLETE
Modified: trunk/LayoutTests/fast/table/css-table-max-width.html (163164 => 163165)
--- trunk/LayoutTests/fast/table/css-table-max-width.html 2014-01-31 07:51:17 UTC (rev 163164)
+++ trunk/LayoutTests/fast/table/css-table-max-width.html 2014-01-31 08:09:07 UTC (rev 163165)
@@ -42,7 +42,7 @@
fugiat nulla pariatur.Excepteur sint occaecat cupidatat non proident, sunt in culpa
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,
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
@@ -97,7 +97,7 @@
_onlyMaxWidthAutoLayout_ = document.getElementById("onlyMaxWidthAutoLayout");
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");
minGreatThanMaxWidthFixedLayout = document.getElementById("minGreatThanMaxWidthFixedLayout");
@@ -105,7 +105,7 @@
_onlyMaxWidthFixedLayout_ = document.getElementById("onlyMaxWidthFixedLayout");
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'));
</script>
Added: trunk/LayoutTests/fast/table/html-table-width-max-width-constrained-expected.txt (0 => 163165)
--- trunk/LayoutTests/fast/table/html-table-width-max-width-constrained-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/html-table-width-max-width-constrained-expected.txt 2014-01-31 08:09:07 UTC (rev 163165)
@@ -0,0 +1,4 @@
+This test checks that a fixed table layout with max-width doesn't over-constraint the cell (ie the content width still wins over max-width per the specification).
+For this test to pass, the second cell shouldn't bleed out of the table.
+Cell text Cell text text text text
+PASS
Added: trunk/LayoutTests/fast/table/html-table-width-max-width-constrained.html (0 => 163165)
--- trunk/LayoutTests/fast/table/html-table-width-max-width-constrained.html (rev 0)
+++ trunk/LayoutTests/fast/table/html-table-width-max-width-constrained.html 2014-01-31 08:09:07 UTC (rev 163165)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {
+ font: 20px/1 Ahem;
+ border-collapse: separate;
+ table-layout: fixed;
+ max-width: 300px;
+ width: 300px;
+ border: 1px solid #dddddd;
+ border-spacing: 0px;
+}
+
+td {
+ padding: 0px;
+ width: 200px;
+ border-left: 1px solid #dddddd;
+}
+</style>
+<script src=""
+</head>
+<body _onload_="checkLayout('table')">
+ <div>This test checks that a fixed table layout with max-width doesn't over-constraint the cell (ie the content width still wins over max-width per the specification).</div>
+ <div>For this test to pass, the second cell shouldn't bleed out of the table.</div>
+ <table data-expected-width="404">
+ <tbody>
+ <tr>
+ <td>Cell text</td>
+ <td>Cell text text text text</td>
+ </tr>
+ </tbody>
+ </table>
+</body></html>
Modified: trunk/Source/WebCore/ChangeLog (163164 => 163165)
--- trunk/Source/WebCore/ChangeLog 2014-01-31 07:51:17 UTC (rev 163164)
+++ trunk/Source/WebCore/ChangeLog 2014-01-31 08:09:07 UTC (rev 163165)
@@ -1,3 +1,29 @@
+2014-01-31 László Langó <[email protected]>
+
+ 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 <[email protected]>
Fix iOS build after r163156.
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (163164 => 163165)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2014-01-31 07:51:17 UTC (rev 163164)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2014-01-31 08:09:07 UTC (rev 163165)
@@ -292,16 +292,16 @@
setLogicalWidth(std::min<int>(availableContentLogicalWidth, maxPreferredLogicalWidth()));
}
- // 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();
if ((styleMaxLogicalWidth.isSpecified() && !styleMaxLogicalWidth.isNegative()) || styleMaxLogicalWidth.isIntrinsic()) {
LayoutUnit computedMaxLogicalWidth = convertStyleLogicalWidthToComputedWidth(styleMaxLogicalWidth, availableLogicalWidth);
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.
Length styleMinLogicalWidth = style().logicalMinWidth();
@@ -328,6 +328,10 @@
setMarginStart(minimumValueForLength(style().marginStart(), availableLogicalWidth));
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());
}
// This method takes a RenderStyle's logical width, min-width, or max-width length and computes its actual value.
@@ -778,8 +782,9 @@
// 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);
}
// FIXME: We should be adding borderAndPaddingLogicalWidth here, but m_tableLayout->computePreferredLogicalWidths already does,
