Changeset 251281 in webkit

Timestamp:

Oct 18, 2019 9:09:41 AM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][BFC] TableFormattingContext::computedIntrinsicWidthConstraints should not expect a valid containing block's width
​https://bugs.webkit.org/show_bug.cgi?id=203131
<rdar://problem/56394676>

Reviewed by Antti Koivisto.

When TableFormattingContext::computedIntrinsicWidthConstraints is called by the preferred width computation (<div style="float: left"><table>)
the containing block's width is not yet set (it gets computed based on the preferred width) so computedIntrinsicWidthConstraints should not be relying
on it. Let's move that logic out to TableFormattingContext::layoutInFlowContent() where it belongs.

• layout/blockformatting/BlockFormattingContextGeometry.cpp:

(WebCore::Layout::BlockFormattingContext::Geometry::inFlowWidthAndMargin):

• layout/tableformatting/TableFormattingContext.cpp:

(WebCore::Layout::TableFormattingContext::layoutInFlowContent):
(WebCore::Layout::TableFormattingContext::computedIntrinsicWidthConstraints):
(WebCore::Layout::TableFormattingContext::ensureTableGrid):
(WebCore::Layout::TableFormattingContext::computeAndDistributeExtraHorizontalSpace):
(WebCore::Layout::TableFormattingContext::computedTableWidth): Deleted.
(WebCore::Layout::TableFormattingContext::distributeExtraHorizontalSpace): Deleted.

• layout/tableformatting/TableFormattingContext.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/blockformatting/BlockFormattingContextGeometry.cpp (modified) (1 diff)
• layout/tableformatting/TableFormattingContext.cpp (modified) (5 diffs)
• layout/tableformatting/TableFormattingContext.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-10-18  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC] TableFormattingContext::computedIntrinsicWidthConstraints should not expect a valid containing block's width
+        https://bugs.webkit.org/show_bug.cgi?id=203131
+        <rdar://problem/56394676>
+
+        Reviewed by Antti Koivisto.
+
+        When TableFormattingContext::computedIntrinsicWidthConstraints is called by the preferred width computation (<div style="float: left"><table>)
+        the containing block's width is not yet set (it gets computed based on the preferred width) so computedIntrinsicWidthConstraints should not be relying
+        on it. Let's move that logic out to TableFormattingContext::layoutInFlowContent() where it belongs. 
+
+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowWidthAndMargin):
+        * layout/tableformatting/TableFormattingContext.cpp:
+        (WebCore::Layout::TableFormattingContext::layoutInFlowContent):
+        (WebCore::Layout::TableFormattingContext::computedIntrinsicWidthConstraints):
+        (WebCore::Layout::TableFormattingContext::ensureTableGrid):
+        (WebCore::Layout::TableFormattingContext::computeAndDistributeExtraHorizontalSpace):
+        (WebCore::Layout::TableFormattingContext::computedTableWidth): Deleted.
+        (WebCore::Layout::TableFormattingContext::distributeExtraHorizontalSpace): Deleted.
+        * layout/tableformatting/TableFormattingContext.h:
+
 2019-10-18  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp

@@ -283 +283 @@
             // This is a special table "fit-content size" behavior handling. Not in the spec though.
             // Table returns its final width as min/max. Use this final width value to computed horizontal margins etc.
-            usedHorizontalValues.width = shrinkToFitWidth(layoutBox, usedHorizontalValues.constraints.width);
+            usedHorizontalValues.width = usedHorizontalValues.width ? usedHorizontalValues.width : shrinkToFitWidth(layoutBox, usedHorizontalValues.constraints.width);
         }
         return inFlowNonReplacedWidthAndMargin(layoutBox, usedHorizontalValues);

trunk/Source/WebCore/layout/tableformatting/TableFormattingContext.cpp

@@ -61 +61 @@
 {
     auto& grid = formattingState().tableGrid();
+    auto& columnsContext = grid.columnsContext();
+
+    computeAndDistributeExtraHorizontalSpace();
+    // 1. Position each column.
+    // FIXME: This should also deal with collapsing borders etc.
+    auto horizontalSpacing = grid.horizontalSpacing();
+    auto columnLogicalLeft = horizontalSpacing;
+    for (auto& column : columnsContext.columns()) {
+        column.setLogicalLeft(columnLogicalLeft);
+        columnLogicalLeft += (column.logicalWidth() + horizontalSpacing);
+    }
+
+    // 2. Layout each table cell (and compute row height as well).
+    auto& columnList = columnsContext.columns();
     auto& cellList = grid.cells();
-    auto& columnList = grid.columnsContext().columns();
     ASSERT(!cellList.isEmpty());
-    // Layout and position each table cell (and compute row height as well).
     for (auto& cell : cellList) {
         auto& cellLayoutBox = cell->tableCellBox;
@@ -79 +91 @@
     }
 
-    // Finalize size and position.
+    // 3. Finalize size and position.
     positionTableCells();
     setComputedGeometryForSections();
@@ -153 +165 @@
     // 1. Ensure each cell slot is occupied by at least one cell.
     ensureTableGrid();
-    // 2. Compute the minimum width of each column.
+    // 2. Compute the minimum/maximum width of each column.
     computePreferredWidthForColumns();
-    // 3. Compute the width of the table.
-    auto width = computedTableWidth();
-    // This is the actual computed table width that we want to present as min/max width.
-    formattingState().setIntrinsicWidthConstraints({ width, width });
-    return { width, width };
+    // 3. Compute the minimum/maximum width of the table box.
+    auto& grid = formattingState().tableGrid();
+    auto tableWidthConstraints = grid.widthConstraints();
+    tableWidthConstraints.expand(grid.totalHorizontalSpacing());
+    return tableWidthConstraints;
 }
 
 void TableFormattingContext::ensureTableGrid()
 {
-    auto& tableWrapperBox = root();
+    auto& tableBox = root();
     auto& tableGrid = formattingState().tableGrid();
-    tableGrid.setHorizontalSpacing(LayoutUnit { tableWrapperBox.style().horizontalBorderSpacing() });
-    tableGrid.setVerticalSpacing(LayoutUnit { tableWrapperBox.style().verticalBorderSpacing() });
-
-    auto* firstChild = tableWrapperBox.firstChild();
+    tableGrid.setHorizontalSpacing(LayoutUnit { tableBox.style().horizontalBorderSpacing() });
+    tableGrid.setVerticalSpacing(LayoutUnit { tableBox.style().verticalBorderSpacing() });
+
+    auto* firstChild = tableBox.firstChild();
     const Box* tableCaption = nullptr;
     const Box* colgroup = nullptr;
@@ -261 +273 @@
 }
 
-LayoutUnit TableFormattingContext::computedTableWidth()
-{
+void TableFormattingContext::computeAndDistributeExtraHorizontalSpace()
+{
+    auto& grid = formattingState().tableGrid();
+    auto tableWidthConstraints = grid.widthConstraints();
+    auto tableMinimumContentWidth = tableWidthConstraints.minimum - grid.totalHorizontalSpacing();
+
     // Column and caption widths influence the final table width as follows:
     // If the 'table' or 'inline-table' element's 'width' property has a computed value (W) other than 'auto', the used width is the greater of
@@ -270 +286 @@
     // CAPMIN, and MIN. However, if either CAPMIN or the maximum width required by the columns plus cell spacing or borders (MAX) is
     // less than that of the containing block, use max(MAX, CAPMIN).
-
-    // FIXME: This kind of code usually lives in *FormattingContextGeometry class.
-    auto& tableWrapperBox = root();
-    auto& style = tableWrapperBox.style();
-    auto& containingBlock = *tableWrapperBox.containingBlock();
-    auto containingBlockWidth = geometryForBox(containingBlock, EscapeType::TableFormattingContextAccessParentTableWrapperBlockFormattingContext).contentBoxWidth();
-
-    auto& grid = formattingState().tableGrid();
-    auto& columnsContext = grid.columnsContext();
-    auto tableWidthConstraints = grid.widthConstraints();
-    auto totalHorizontalSpacing = grid.totalHorizontalSpacing();
-
-    auto width = geometry().computedValueIfNotAuto(style.width(), containingBlockWidth);
-    LayoutUnit usedWidth;
+    auto distributeExtraHorizontalSpace = [&](auto extraHorizontalSpace) {
+        auto& columns = grid.columnsContext().columns();
+        ASSERT(!columns.isEmpty());
+
+        auto adjustabledHorizontalSpace = tableMinimumContentWidth;
+        auto numberOfColumns = columns.size();
+        // Fixed width columns don't participate in available space distribution.
+        for (auto& column : columns) {
+            if (!column.hasFixedWidth())
+                continue;
+            auto columnFixedWidth = *column.columnBox()->columnWidth();
+            column.setLogicalWidth(columnFixedWidth);
+
+            --numberOfColumns;
+            adjustabledHorizontalSpace -= columnFixedWidth;
+        }
+        if (!numberOfColumns || !adjustabledHorizontalSpace)
+            return;
+        // FIXME: Right now just distribute the extra space equaly among the columns using the minimum width.
+        ASSERT(adjustabledHorizontalSpace > 0);
+        for (auto& column : columns) {
+            if (column.hasFixedWidth())
+                continue;
+            auto columnExtraSpace = extraHorizontalSpace / adjustabledHorizontalSpace * column.widthConstraints().minimum;
+            column.setLogicalWidth(column.widthConstraints().minimum + columnExtraSpace);
+        }
+    };
+
+    auto containingBlockWidth = geometryForBox(*root().containingBlock(), EscapeType::TableFormattingContextAccessParentTableWrapperBlockFormattingContext).contentBoxWidth();
+    auto width = geometry().computedValueIfNotAuto(root().style().width(), containingBlockWidth);
     if (width) {
-        if (*width > tableWidthConstraints.minimum) {
-            distributeExtraHorizontalSpace(*width - totalHorizontalSpacing, tableWidthConstraints.minimum - totalHorizontalSpacing);
-            usedWidth = *width;
-        } else {
-            usedWidth = tableWidthConstraints.minimum;
+        if (*width > tableMinimumContentWidth)
+            distributeExtraHorizontalSpace(*width - tableMinimumContentWidth);
+        else
             useAsContentLogicalWidth(WidthConstraintsType::Minimum);
-        }
     } else {
-        if (tableWidthConstraints.minimum > containingBlockWidth) {
-            usedWidth = tableWidthConstraints.minimum;
+        if (tableMinimumContentWidth > containingBlockWidth)
             useAsContentLogicalWidth(WidthConstraintsType::Minimum);
-        } else if (tableWidthConstraints.maximum < containingBlockWidth) {
-            usedWidth = tableWidthConstraints.maximum;
+        else if (tableWidthConstraints.maximum < containingBlockWidth)
             useAsContentLogicalWidth(WidthConstraintsType::Maximum);
-        } else {
-            usedWidth = containingBlockWidth;
-            distributeExtraHorizontalSpace(usedWidth - totalHorizontalSpacing, tableWidthConstraints.minimum - totalHorizontalSpacing);
-        }
-    }
-    // FIXME: This should also deal with collapsing borders etc.
-    auto horizontalSpacing = grid.horizontalSpacing();
-    auto columnLogicalLeft = horizontalSpacing;
-    for (auto& column : columnsContext.columns()) {
-        column.setLogicalLeft(columnLogicalLeft);
-        columnLogicalLeft += (column.logicalWidth() + horizontalSpacing);
-    }
-    return usedWidth;
-}
-
-void TableFormattingContext::distributeExtraHorizontalSpace(LayoutUnit availableContentWidth, LayoutUnit tableMinimumContentWidth)
-{
-    ASSERT(availableContentWidth >= tableMinimumContentWidth);
-    auto& grid = formattingState().tableGrid();
-    auto& columns = grid.columnsContext().columns();
-    ASSERT(!columns.isEmpty());
-
-    auto extraHorizontalSpace = availableContentWidth - tableMinimumContentWidth;
-    auto adjustabledHorizontalSpace = tableMinimumContentWidth;
-    auto numberOfColumns = columns.size();
-    // Fixed width columns don't participate in available space distribution.
-    for (auto& column : columns) {
-        if (!column.hasFixedWidth())
-            continue;
-        auto columnFixedWidth = *column.columnBox()->columnWidth();
-        column.setLogicalWidth(columnFixedWidth);
-
-        --numberOfColumns;
-        adjustabledHorizontalSpace -= columnFixedWidth;
-    }
-    if (!numberOfColumns || !adjustabledHorizontalSpace)
-        return;
-    // FIXME: Right now just distribute the extra space equaly among the columns using the minimum width.
-    ASSERT(adjustabledHorizontalSpace > 0);
-    for (auto& column : columns) {
-        if (column.hasFixedWidth())
-            continue;
-        auto columnExtraSpace = extraHorizontalSpace / adjustabledHorizontalSpace * column.widthConstraints().minimum;
-        column.setLogicalWidth(column.widthConstraints().minimum + columnExtraSpace);
+        else
+            distributeExtraHorizontalSpace(containingBlockWidth - tableMinimumContentWidth);
     }
 }

trunk/Source/WebCore/layout/tableformatting/TableFormattingContext.h

@@ -59 +59 @@
 
     IntrinsicWidthConstraints computedIntrinsicWidthConstraints() override;
-    LayoutUnit computedTableWidth();
     void layoutTableCellBox(const Box& cellLayoutBox, const TableGrid::Column&);
     void positionTableCells();
@@ -67 +66 @@
     void ensureTableGrid();
     void computePreferredWidthForColumns();
-    void distributeExtraHorizontalSpace(LayoutUnit availableContentWidth, LayoutUnit tableMinimumContentWidth);
+    void computeAndDistributeExtraHorizontalSpace();
     enum class WidthConstraintsType { Minimum, Maximum };
     void useAsContentLogicalWidth(WidthConstraintsType);