Changeset 261924 in webkit

Timestamp:

May 20, 2020 9:05:43 AM (4 years ago)

Author:

Noam Rosenthal

Message:

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

Reviewed by Zalan Bujtas.

Source/WebCore:

Based on previous patch by László Langó <​lango@inf.u-szeged.hu>

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

A table should always be wide enough to contain its content (preferred logical width).
This constraint should be stronger than the table style's specified min-width/width.

The behavior matches the spec, and behavior on Firefox/Chrome.

• rendering/RenderTable.cpp:

(WebCore::RenderTable::updateLogicalWidth):

Max-width should only affect the table's max preferred width.

(WebCore::RenderTable::computePreferredLogicalWidths):

Change the order of constraints so that content constraint is stronger than style width/max-width constraint.

LayoutTests:

Based on a previous patch by László Langó <​lango@inf.u-szeged.hu>

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

Change in behavior, test was expecting buggy behavior.

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

Test the desired behavior with content-constrained tables.

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) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-05-20  Noam Rosenthal  <noam@webkit.org>
+
+        Fix table sizing when 'max-width' is used
+        https://bugs.webkit.org/show_bug.cgi?id=115156
+
+        Reviewed by Zalan Bujtas.
+
+        Based on a previous patch by László Langó  <lango@inf.u-szeged.hu>
+
+        * fast/table/css-table-max-width-expected.txt:
+        * fast/table/css-table-max-width.html:
+                Change in behavior, test was expecting buggy behavior.
+
+        * fast/table/html-table-width-max-width-constrained-expected.txt: Added.
+        * fast/table/html-table-width-max-width-constrained.html: Added.
+                Test the desired behavior with content-constrained tables.
+
 2020-05-20  Philippe Normand  <pnormand@igalia.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 @@
+2020-05-20  Noam Rosenthal  <noam@webkit.org>
+
+        Fix table sizing when 'max-width' is used
+        https://bugs.webkit.org/show_bug.cgi?id=115156
+
+        Reviewed by Zalan Bujtas.
+
+        Based on previous patch by László Langó  <lango@inf.u-szeged.hu>
+
+        Test: fast/table/html-table-width-max-width-constrained.html
+
+        A table should always be wide enough to contain its content (preferred logical width).
+        This constraint should be stronger than the table style's specified min-width/width.
+
+        The behavior matches the spec, and behavior on Firefox/Chrome.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::updateLogicalWidth):
+                Max-width should only affect the table's max preferred width.
+
+        (WebCore::RenderTable::computePreferredLogicalWidths):
+                Change the order of constraints so that content constraint is stronger than style width/max-width constraint.
+
 2020-05-20  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/rendering/RenderTable.cpp

@@ -281 +281 @@
     }
 
-    // Ensure we aren't smaller than our min preferred width.
-    setLogicalWidth(std::max(logicalWidth(), minPreferredLogicalWidth()));
-
-    
     // Ensure we aren't bigger than our max-width style.
     Length styleMaxLogicalWidth = style().logicalMaxWidth();
@@ -291 +287 @@
         setLogicalWidth(std::min(logicalWidth(), computedMaxLogicalWidth));
     }
+
+    // Ensure we aren't smaller than our min preferred width.
+    setLogicalWidth(std::max(logicalWidth(), minPreferredLogicalWidth()));    
 
     // Ensure we aren't smaller than our min-width style.
@@ -828 +827 @@
     if (styleToUse.logicalMaxWidth().isFixed()) {
         m_maxPreferredLogicalWidth = std::min(m_maxPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse.logicalMaxWidth().value()));
-        m_minPreferredLogicalWidth = std::min(m_minPreferredLogicalWidth, adjustContentBoxLogicalWidthForBoxSizing(styleToUse.logicalMaxWidth().value()));
+        m_maxPreferredLogicalWidth = std::max(m_maxPreferredLogicalWidth, m_minPreferredLogicalWidth);
     }