[LFC][TFC] Non-collapsing row border should not make the table wider/taller
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 13:34:47 +0000 (13:34 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 May 2020 13:34:47 +0000 (13:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212263

Reviewed by Antti Koivisto.

Source/WebCore:

Non-collapsing row border eats into the content box but oddly it does not
constraint the cell boxes, so we can end up with smaller row content box than
the cell box it contains.

Test: fast/layoutformattingcontext/table-simple-row-border.html

* layout/LayoutUnits.h:
(WebCore::Layout::Edges::width const):
(WebCore::Layout::Edges::height const):
(WebCore::Layout::HorizontalEdges::width const): Deleted.
(WebCore::Layout::VerticalEdges::height const): Deleted.
* layout/tableformatting/TableFormattingContext.cpp:
(WebCore::Layout::TableFormattingContext::setUsedGeometryForRows):
* layout/tableformatting/TableFormattingContextGeometry.cpp:
(WebCore::Layout::TableFormattingContext::Geometry::intrinsicWidthConstraintsForCell):

LayoutTests:

* fast/layoutformattingcontext/table-simple-row-border-expected.html: Added.
* fast/layoutformattingcontext/table-simple-row-border.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/layoutformattingcontext/table-simple-row-border-expected.html [new file with mode: 0644]
LayoutTests/fast/layoutformattingcontext/table-simple-row-border.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/layout/LayoutUnits.h
Source/WebCore/layout/Verification.cpp
Source/WebCore/layout/tableformatting/TableFormattingContext.cpp
Source/WebCore/layout/tableformatting/TableFormattingContextGeometry.cpp

index cc04054..7397679 100644 (file)
@@ -1,3 +1,13 @@
+2020-05-23  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][TFC] Non-collapsing row border should not make the table wider/taller
+        https://bugs.webkit.org/show_bug.cgi?id=212263
+
+        Reviewed by Antti Koivisto.
+
+        * fast/layoutformattingcontext/table-simple-row-border-expected.html: Added.
+        * fast/layoutformattingcontext/table-simple-row-border.html: Added.
+
 2020-05-22  Jack Lee  <shihchieh_lee@apple.com>
 
         ASSERTION FAILED: (!s_current || &m_view != &s_current->m_view) in RenderTreeBuilder::RenderTreeBuilder
diff --git a/LayoutTests/fast/layoutformattingcontext/table-simple-row-border-expected.html b/LayoutTests/fast/layoutformattingcontext/table-simple-row-border-expected.html
new file mode 100644 (file)
index 0000000..5c75cfd
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:LayoutFormattingContextEnabled=true internal:LayoutFormattingContextIntegrationEnabled=false ] -->
+<style>
+div {
+    height: 10px;
+    width: 10px;
+    border: 1px solid green;
+    position: absolute;
+}
+</style>
+<div style="top: 10px; left: 10px;"></div>
+<div style="top: 10px; left: 24px;"></div>
+<div style="top: 24px; left: 10px;"></div>
+<div style="top: 24px; left: 24px;"></div>
diff --git a/LayoutTests/fast/layoutformattingcontext/table-simple-row-border.html b/LayoutTests/fast/layoutformattingcontext/table-simple-row-border.html
new file mode 100644 (file)
index 0000000..1084e5f
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:LayoutFormattingContextEnabled=true internal:LayoutFormattingContextIntegrationEnabled=false ] -->
+<style>
+td {
+    width: 10px;
+    height: 10px;
+    padding: 0px;
+    border: 1px solid green;
+}
+
+tr {
+    border: 50px solid red;
+}
+</style>
+<table>
+<tr><td></td><td></td></tr>
+<tr><td></td><td></td></tr>
+</tbody>
+</table>
index eaa03ec..29694a0 100644 (file)
@@ -1,3 +1,26 @@
+2020-05-23  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][TFC] Non-collapsing row border should not make the table wider/taller
+        https://bugs.webkit.org/show_bug.cgi?id=212263
+
+        Reviewed by Antti Koivisto.
+
+        Non-collapsing row border eats into the content box but oddly it does not
+        constraint the cell boxes, so we can end up with smaller row content box than
+        the cell box it contains.
+
+        Test: fast/layoutformattingcontext/table-simple-row-border.html
+
+        * layout/LayoutUnits.h:
+        (WebCore::Layout::Edges::width const):
+        (WebCore::Layout::Edges::height const):
+        (WebCore::Layout::HorizontalEdges::width const): Deleted.
+        (WebCore::Layout::VerticalEdges::height const): Deleted.
+        * layout/tableformatting/TableFormattingContext.cpp:
+        (WebCore::Layout::TableFormattingContext::setUsedGeometryForRows):
+        * layout/tableformatting/TableFormattingContextGeometry.cpp:
+        (WebCore::Layout::TableFormattingContext::Geometry::intrinsicWidthConstraintsForCell):
+
 2020-05-22  Jack Lee  <shihchieh_lee@apple.com>
 
         ASSERTION FAILED: (!s_current || &m_view != &s_current->m_view) in RenderTreeBuilder::RenderTreeBuilder
index 58c096d..d7c81b5 100644 (file)
@@ -115,20 +115,19 @@ inline void Point::moveBy(LayoutPoint offset)
 struct HorizontalEdges {
     LayoutUnit left;
     LayoutUnit right;
-
-    LayoutUnit width() const { return left + right; }
 };
 
 struct VerticalEdges {
     LayoutUnit top;
     LayoutUnit bottom;
-
-    LayoutUnit height() const { return top + bottom; }
 };
 
 struct Edges {
     HorizontalEdges horizontal;
     VerticalEdges vertical;
+
+    LayoutUnit width() const { return horizontal.left + horizontal.right; }
+    LayoutUnit height() const { return vertical.top + vertical.bottom; }
 };
 
 inline Edges operator/(const Edges& edge, size_t value)
index dfd4602..5e372ac 100644 (file)
@@ -274,12 +274,16 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
         return true;
     }
 
-    if (!areEssentiallyEqual(renderer.paddingBoxRect(), displayBox.paddingBox())) {
+    // When the table row border overflows the row, padding box becomes negative and content box is incorrect.
+    auto shouldCheckPaddingAndContentBox = !is<RenderTableRow>(renderer) || renderer.paddingBoxRect().width() >= 0;
+    if (shouldCheckPaddingAndContentBox && !areEssentiallyEqual(renderer.paddingBoxRect(), displayBox.paddingBox())) {
         outputRect("paddingBox", renderer.paddingBoxRect(), displayBox.paddingBox());
         return true;
     }
 
     auto shouldCheckContentBox = [&] {
+        if (!shouldCheckPaddingAndContentBox)
+            return false;
         // FIXME: Figure out why trunk/rendering comes back with odd values for <tbody> and <td> content box.
         if (is<RenderTableCell>(renderer) || is<RenderTableSection>(renderer))
             return false;
index f3b2854..f51ed0f 100644 (file)
@@ -113,13 +113,19 @@ void TableFormattingContext::setUsedGeometryForRows(LayoutUnit availableHorizont
     auto& grid = formattingState().tableGrid();
     auto& rows = grid.rows().list();
 
-    auto rowWidth = grid.columns().logicalWidth() + 2 * grid.horizontalSpacing();
     auto rowLogicalTop = grid.verticalSpacing();
     for (size_t rowIndex = 0; rowIndex < rows.size(); ++rowIndex) {
         auto& row = rows[rowIndex];
         auto& rowBox = row.box();
         auto& rowDisplayBox = formattingState().displayBox(rowBox);
 
+        rowDisplayBox.setPadding(geometry().computedPadding(rowBox, availableHorizontalSpace));
+        // Internal table elements do not have margins.
+        rowDisplayBox.setHorizontalMargin({ });
+        rowDisplayBox.setHorizontalComputedMargin({ });
+        rowDisplayBox.setVerticalMargin({ { }, { } });
+
+
         auto computedRowBorder = [&] {
             auto border = geometry().computedBorder(rowBox);
             if (!grid.collapsedBorder())
@@ -132,15 +138,28 @@ void TableFormattingContext::setUsedGeometryForRows(LayoutUnit availableHorizont
                 border.vertical.bottom = { };
             return border;
         }();
+        if (computedRowBorder.height() > row.logicalHeight()) {
+            // FIXME: This is an odd quirk when the row border overflows the row.
+            // We don't paint row borders so it does not matter too much, but if we don't
+            // set this fake border value, than we either end up with a negative content box
+            // or with a wide frame box.
+            // If it happens to cause issues in the display tree, we could also consider
+            // a special frame box override, where padding box + border != frame box.
+            computedRowBorder.vertical.top = { };
+            computedRowBorder.vertical.bottom = { };
+        }
+        rowDisplayBox.setContentBoxHeight(row.logicalHeight() - computedRowBorder.height());
+
+        auto rowLogicalWidth = grid.columns().logicalWidth() + 2 * grid.horizontalSpacing();
+        if (computedRowBorder.width() > rowLogicalWidth) {
+            // See comment above.
+            computedRowBorder.horizontal.left = { };
+            computedRowBorder.horizontal.right = { };
+        }
+        rowDisplayBox.setContentBoxWidth(rowLogicalWidth - computedRowBorder.width());
+
         rowDisplayBox.setBorder(computedRowBorder);
-        rowDisplayBox.setPadding(geometry().computedPadding(rowBox, availableHorizontalSpace));
-        // Internal table elements do not have margins.
-        rowDisplayBox.setHorizontalMargin({ });
-        rowDisplayBox.setHorizontalComputedMargin({ });
-        rowDisplayBox.setVerticalMargin({ { }, { } });
 
-        rowDisplayBox.setContentBoxHeight(row.logicalHeight());
-        rowDisplayBox.setContentBoxWidth(rowWidth);
         rowDisplayBox.setTop(rowLogicalTop);
         rowDisplayBox.setLeft({ });
 
index ced5792..c76b1db 100644 (file)
@@ -110,7 +110,7 @@ FormattingContext::IntrinsicWidthConstraints TableFormattingContext::Geometry::i
     // FIXME Check for box-sizing: border-box;
     auto intrinsicWidthConstraints = constrainByMinMaxWidth(cellBox, computedIntrinsicWidthConstraints());
     // Expand with border
-    intrinsicWidthConstraints.expand(computedCellBorder(cell).horizontal.width());
+    intrinsicWidthConstraints.expand(computedCellBorder(cell).width());
     // padding
     intrinsicWidthConstraints.expand(fixedValue(style.paddingLeft()).valueOr(0) + fixedValue(style.paddingRight()).valueOr(0));
     // and margin