Fix table layout to properly handle visibility:collapse on columns

Ahmad-S792 · Ahmad Saleem · commit 3ea149a5173a · 2026-01-21T23:13:37.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=305905 rdar://168556786 Reviewed by Alan Baradlay. This patch aligns WebKit with Gecko / Firefox and Blink / Chromium. Columns with visibility:collapse should not contribute to table width calculations. This patch modifies AutoTableLayout to: 1. Skip collapsed columns in recalcColumn() by setting their min/max widths to 0 early. 2. Skip collapsed columns throughout layout() when distributing available width to columns. 3. Skip adding border-spacing after collapsed columns when calculating column positions. Previously, collapsed columns were treated as normal columns and received width allocation, causing tables to be wider than expected. * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-001-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-002-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-003-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-004-dynamic-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-005-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-colspan-002-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-001-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-002-expected.txt: * LayoutTests/platform/glib/tables/mozilla_expected_failures/other/test4-expected.txt: * LayoutTests/platform/ios/tables/mozilla_expected_failures/other/test4-expected.txt: * LayoutTests/platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt: * Source/WebCore/rendering/AutoTableLayout.cpp: (WebCore::AutoTableLayout::recalcColumn): (WebCore::AutoTableLayout::layout): Canonical link: https://commits.webkit.org/305997@main
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -7883,8 +7883,6 @@ http/tests/iframe-memory-monitor/video-iframe.html [ Skip ]
 # css/CSS2/tables failures
 webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-collapse-006.html [ ImageOnlyFailure ]
 webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-collapse-dynamic-column-002.xht [ ImageOnlyFailure ]
-webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-collapse-visibility-collapse-001.tentative.html [ ImageOnlyFailure ]
-webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-collapse-visibility-collapse-002.tentative.html [ ImageOnlyFailure ]
 webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-conflict-element-001d.xht [ ImageOnlyFailure ]
 webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/border-conflict-element-001e.xht [ ImageOnlyFailure ]
 webkit.org/b/288957 imported/w3c/web-platform-tests/css/CSS2/tables/caption-position-001.xht [ ImageOnlyFailure ]
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-001-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-001-expected.txt
@@ -12,5 +12,5 @@ row 2
 row 3	
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: expected 116 but got 222
+PASS col visibility:collapse changes table width
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-002-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-002-expected.txt
@@ -12,5 +12,5 @@ row 2
 row 3	
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 10 but got 222
+PASS col visibility:collapse changes table width
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-003-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-003-expected.txt
@@ -12,5 +12,5 @@ row 2
 row 3	
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 116 but got 222
+PASS col visibility:collapse changes table width
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-004-dynamic-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-004-dynamic-expected.txt
@@ -8,5 +8,5 @@ row 2
 row 3	
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 122 but got 232
+FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 122 but got 124
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-005-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-col-005-expected.txt
@@ -5,5 +5,5 @@ Setting a spanning column to visibility:collapse changes table width but not hei
 
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 100 but got 111
+PASS col visibility:collapse changes table width
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-colspan-002-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-colspan-002-expected.txt
@@ -5,5 +5,5 @@ Setting a spanning column to visibility:collapse changes table width but not hei
 
 
 PASS col visibility:collapse doesn't change table height
-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 50 but got 60
+PASS col visibility:collapse changes table width
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-001-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-001-expected.txt
@@ -4,6 +4,6 @@ Spec
 Setting row and column to visibility:collapse changes both height and width. The collapsed result should have whitespace in the bottom right corner.
 
 
-FAIL spanning col visibility:collapse changes table width assert_equals: spanning col visibility:collapse changes table width expected 112 but got 148
+FAIL spanning col visibility:collapse changes table width assert_equals: spanning col visibility:collapse changes table width expected 112 but got 114
 PASS spanning row visibility:collapse changes height
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-002-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowcol-002-expected.txt
@@ -4,6 +4,6 @@ Spec
 Setting row and spanning column to visibility:collapse changes height and width.
 
 
-FAIL spanning row visibility:collapse doesn't change table width assert_equals: spanning row visibility:collapse doesn't change table width expected 45 but got 130
+PASS spanning row visibility:collapse doesn't change table width
 PASS spanning row visibility:collapse doesn't change height in this case
 
diff --git a/LayoutTests/platform/glib/tables/mozilla_expected_failures/other/test4-expected.txt b/LayoutTests/platform/glib/tables/mozilla_expected_failures/other/test4-expected.txt
diff --git a/LayoutTests/platform/ios/tables/mozilla_expected_failures/other/test4-expected.txt b/LayoutTests/platform/ios/tables/mozilla_expected_failures/other/test4-expected.txt
diff --git a/LayoutTests/platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt b/LayoutTests/platform/mac/tables/mozilla_expected_failures/other/test4-expected.txt
diff --git a/Source/WebCore/rendering/AutoTableLayout.cpp b/Source/WebCore/rendering/AutoTableLayout.cpp
@@ -49,6 +49,17 @@ void AutoTableLayout::recalcColumn(unsigned effCol)
 {
     Layout& columnLayout = m_layoutStruct[effCol];
 
+    // Check if this column is collapsed.
+    if (CheckedPtr colElement = m_table->colElement(effCol); colElement && colElement->style().visibility() == Visibility::Collapse) {
+        columnLayout.effectiveLogicalWidth = CSS::Keyword::Auto { };
+        columnLayout.effectiveMinLogicalWidth = 0;
+        columnLayout.effectiveMaxLogicalWidth = 0;
+        columnLayout.minLogicalWidth = 0;
+        columnLayout.maxLogicalWidth = 0;
+        columnLayout.logicalWidth = CSS::Keyword::Auto { };
+        return;
+    }
+
     RenderTableCell* fixedContributor = nullptr;
     RenderTableCell* maxContributor = nullptr;
 
@@ -599,6 +610,12 @@ void AutoTableLayout::layout()
 
     // fill up every cell with its minWidth
     for (size_t i = 0; i < nEffCols; ++i) {
+        // Check if this column is collapsed
+        if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse) {
+            m_layoutStruct[i].computedLogicalWidth = 0;
+            continue;
+        }
+
         float cellLogicalWidth = m_layoutStruct[i].effectiveMinLogicalWidth;
         m_layoutStruct[i].computedLogicalWidth = cellLogicalWidth;
         available -= cellLogicalWidth;
@@ -627,6 +644,9 @@ void AutoTableLayout::layout()
     // allocate width to percent cols
     if (available > 0 && havePercent) {
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isPercentOrCalculated()) {
                 float cellLogicalWidth = std::max<float>(m_layoutStruct[i].effectiveMinLogicalWidth, Style::evaluateMinimum<float>(logicalWidth, tableLogicalWidth, Style::ZoomFactor { m_layoutStruct[i].usedZoom }));
@@ -639,6 +659,9 @@ void AutoTableLayout::layout()
             float excess = tableLogicalWidth * (totalPercent - 100) / 100;
             for (unsigned i = nEffCols; i; ) {
                 --i;
+                if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                    continue;
+
                 if (m_layoutStruct[i].effectiveLogicalWidth.isPercentOrCalculated()) {
                     float cellLogicalWidth = m_layoutStruct[i].computedLogicalWidth;
                     float reduction = std::min(cellLogicalWidth,  excess);
@@ -655,6 +678,9 @@ void AutoTableLayout::layout()
     // then allocate width to fixed cols
     if (available > 0) {
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             auto usedZoom = m_layoutStruct[i].usedZoom;
             if (auto fixedLogicalWidth = logicalWidth.tryFixed(); fixedLogicalWidth && fixedLogicalWidth->resolveZoom(Style::ZoomFactor { usedZoom }) > m_layoutStruct[i].computedLogicalWidth) {
@@ -674,6 +700,9 @@ void AutoTableLayout::layout()
             equalWidthForZeroLengthColumns = available / numberOfNonEmptyAuto;
         }
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& column = m_layoutStruct[i];
             if (!column.effectiveLogicalWidth.isAuto() || column.emptyCellsOnly)
                 continue;
@@ -691,6 +720,9 @@ void AutoTableLayout::layout()
     // spread over fixed columns
     if (available > 0 && numFixed) {
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isFixed()) {
                 float cellLogicalWidth = available * m_layoutStruct[i].effectiveMaxLogicalWidth / totalFixed;
@@ -704,6 +736,9 @@ void AutoTableLayout::layout()
     // spread over percent columns
     if (available > 0 && m_hasPercent && totalPercent < 100) {
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (auto percentageLogicalWidth = logicalWidth.tryPercentage()) {
                 float cellLogicalWidth = available * percentageLogicalWidth->value / totalPercent;
@@ -719,9 +754,20 @@ void AutoTableLayout::layout()
     // spread over the rest
     if (available > 0 && nEffCols > numAutoEmptyCellsOnly) {
         unsigned total = nEffCols - numAutoEmptyCellsOnly;
+        // Count collapsed columns to subtract from total
+        unsigned numCollapsed = 0;
+        for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                numCollapsed++;
+        }
+        total -= numCollapsed;
+
         // still have some width to spread
-        for (unsigned i = nEffCols; i; ) {
+        for (unsigned i = nEffCols; i && total > 0; ) {
             --i;
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             // variable columns with empty cells only don't get any width
             if (m_layoutStruct[i].effectiveLogicalWidth.isAuto() && m_layoutStruct[i].emptyCellsOnly)
                 continue;
@@ -736,6 +782,9 @@ void AutoTableLayout::layout()
         // All columns in this table are empty with 'width: auto'.
         auto equalWidthForColumns = available / numAutoEmptyCellsOnly;
         for (size_t i = 0; i < nEffCols; ++i) {
+            if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse)
+                continue;
+
             auto& column = m_layoutStruct[i];
             column.computedLogicalWidth = equalWidthForColumns;
             available -= column.computedLogicalWidth;
@@ -756,6 +805,12 @@ void AutoTableLayout::layout()
     LayoutUnit pos;
     for (size_t i = 0; i < nEffCols; ++i) {
         m_table->setColumnPosition(i, pos);
+
+        if (CheckedPtr colElement = m_table->colElement(i); colElement && colElement->style().visibility() == Visibility::Collapse) {
+            // Don't add width or spacing for collapsed columns.
+            continue;
+        }
+
         pos += LayoutUnit::fromFloatCeil(m_layoutStruct[i].computedLogicalWidth) + m_table->hBorderSpacing();
     }
     m_table->setColumnPosition(m_table->columnPositions().size() - 1, pos);

Original file line number	Diff line number	Diff line change
`@@ -5,5 +5,5 @@ Setting a spanning column to visibility:collapse changes table width but not hei`
`5`	`5`
`6`	`6`
`7`	`7`	`PASS col visibility:collapse doesn't change table height`
`8`		`-FAIL col visibility:collapse changes table width assert_equals: col visibility:collapse changes table width expected 100 but got 111`
	`8`	`+PASS col visibility:collapse changes table width`
`9`	`9`