layout: Fix border widths of table wrapper with collapsed borders (#35097)

For a table wrapper in collapsed-borders mode we were just halving the
border widths from the computed style. However, it needs to actually
receive half of the resulting collapsed border, which can be bigger.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>

components/layout_2020/display_list/mod.rs

@@ -933,35 +933,22 @@ impl<'a> BuilderForBoxFragment<'a> {
                     bottom_border.width,
                     left_border.width,
                 );
-
+                let left_adjustment = if x == 0 {
-                let mut origin = PhysicalPoint::new(column_sum, row_sum);
+                    -border_widths.left / 2
-                let mut size = PhysicalSize::new(*column_size, *row_size);
-                if x == 0 {
-                    origin.x -= table_info.wrapper_border.left;
-                    size.width += table_info.wrapper_border.left;
                 } else {
-                    border_widths.left = Au::zero();
+                    std::mem::take(&mut border_widths.left) / 2
-                    origin.x += left_border.width / 2;
+                };
-                    size.width -= left_border.width / 2;
+                let top_adjustment = if y == 0 {
-                }
+                    -border_widths.top / 2
-                if y == 0 {
-                    origin.y -= table_info.wrapper_border.top;
-                    size.height += table_info.wrapper_border.top;
                 } else {
-                    border_widths.top = Au::zero();
+                    std::mem::take(&mut border_widths.top) / 2
-                    origin.y += top_border.width / 2;
+                };
-                    size.height -= top_border.width / 2;
+                let origin =
-                }
+                    PhysicalPoint::new(column_sum + left_adjustment, row_sum + top_adjustment);
-                if x + 1 == table_info.track_sizes.x.len() {
+                let size = PhysicalSize::new(
-                    size.width += table_info.wrapper_border.right;
+                    *column_size - left_adjustment + border_widths.right / 2,
-                } else {
+                    *row_size - top_adjustment + border_widths.bottom / 2,
-                    size.width += border_widths.right / 2;
+                );
-                }
-                if y + 1 == table_info.track_sizes.y.len() {
-                    size.height += table_info.wrapper_border.bottom;
-                } else {
-                    size.height += border_widths.bottom / 2;
-                }
                 let border_rect = PhysicalRect::new(origin, size)
                     .translate(self.fragment.content_rect.origin.to_vector())
                     .translate(self.containing_block.origin.to_vector())

components/layout_2020/formatting_contexts.rs

@@ -288,7 +288,7 @@ impl IndependentNonReplacedContents {
             IndependentNonReplacedContents::Flow(fc) => fc.layout_style(base),
             IndependentNonReplacedContents::Flex(fc) => fc.layout_style(),
             IndependentNonReplacedContents::Grid(fc) => fc.layout_style(),
-            IndependentNonReplacedContents::Table(fc) => fc.layout_style(),
+            IndependentNonReplacedContents::Table(fc) => fc.layout_style(None),
         }
     }
 

components/layout_2020/style_ext.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use app_units::Au;
-use style::computed_values::border_collapse::T as BorderCollapse;
 use style::computed_values::direction::T as Direction;
 use style::computed_values::mix_blend_mode::T as ComputedMixBlendMode;
 use style::computed_values::position::T as ComputedPosition;
@@ -33,6 +32,7 @@ use crate::geom::{
     AuOrAuto, LengthPercentageOrAuto, LogicalSides, LogicalVec2, PhysicalSides, PhysicalSize,
     PhysicalVec, Size, Sizes,
 };
+use crate::table::TableLayoutStyle;
 use crate::{ContainingBlock, IndefiniteContainingBlock};
 
 #[derive(Clone, Copy, Eq, PartialEq)]
@@ -805,7 +805,7 @@ impl ComputedValuesExt for ComputedValues {
 
 pub(crate) enum LayoutStyle<'a> {
     Default(&'a ComputedValues),
-    Table(&'a ComputedValues),
+    Table(TableLayoutStyle<'a>),
 }
 
 impl LayoutStyle<'_> {
@@ -813,7 +813,7 @@ impl LayoutStyle<'_> {
     pub(crate) fn style(&self) -> &ComputedValues {
         match self {
             Self::Default(style) => style,
-            Self::Table(style) => style,
+            Self::Table(table) => table.style(),
         }
     }
 
@@ -931,10 +931,7 @@ impl LayoutStyle<'_> {
         &self,
         containing_block_writing_mode: WritingMode,
     ) -> LogicalSides<LengthPercentage> {
-        let style = self.style();
+        if matches!(self, Self::Table(table) if table.collapses_borders()) {
-        if matches!(self, Self::Table(_)) &&
-            style.get_inherited_table().border_collapse == BorderCollapse::Collapse
-        {
             // https://drafts.csswg.org/css-tables/#collapsed-style-overrides
             // > The padding of the table-root is ignored (as if it was set to 0px).
             return LogicalSides::zero();
@@ -955,33 +952,24 @@ impl LayoutStyle<'_> {
         &self,
         containing_block_writing_mode: WritingMode,
     ) -> LogicalSides<Au> {
-        let style = self.style();
+        let border_width = match self {
-        let border = style.get_border();
-        if matches!(self, Self::Table(_)) &&
-            style.get_inherited_table().border_collapse == BorderCollapse::Collapse
-        {
             // For tables in collapsed-borders mode we halve the border widths, because
             // > in this model, the width of the table includes half the table border.
             // https://www.w3.org/TR/CSS22/tables.html#collapsing-borders
-            return LogicalSides::from_physical(
+            Self::Table(table) if table.collapses_borders() => table
-                &PhysicalSides::new(
+                .halved_collapsed_border_widths()
-                    border.border_top_width / 2,
+                .to_physical(self.style().writing_mode),
-                    border.border_right_width / 2,
+            _ => {
-                    border.border_bottom_width / 2,
+                let border = self.style().get_border();
-                    border.border_left_width / 2,
+                PhysicalSides::new(
-                ),
-                containing_block_writing_mode,
-            );
-        }
-        LogicalSides::from_physical(
-            &PhysicalSides::new(
                     border.border_top_width,
                     border.border_right_width,
                     border.border_bottom_width,
                     border.border_left_width,
-            ),
-            containing_block_writing_mode,
                 )
+            },
+        };
+        LogicalSides::from_physical(&border_width, containing_block_writing_mode)
     }
 }
 

components/layout_2020/table/layout.rs

@@ -26,7 +26,7 @@ use style::Zero;
 
 use super::{
     ArcRefCell, CollapsedBorder, CollapsedBorderLine, SpecificTableGridInfo, Table, TableCaption,
-    TableSlot, TableSlotCell, TableSlotCoordinates, TableTrack, TableTrackGroup,
+    TableLayoutStyle, TableSlot, TableSlotCell, TableSlotCoordinates, TableTrack, TableTrackGroup,
 };
 use crate::context::LayoutContext;
 use crate::formatting_contexts::{Baselines, IndependentLayout};
@@ -636,7 +636,6 @@ impl<'a> TableLayout<'a> {
         writing_mode: WritingMode,
     ) -> ContentSizes {
         self.compute_track_constrainedness_and_has_originating_cells(writing_mode);
-        self.compute_border_collapse(writing_mode);
         self.compute_cell_measures(layout_context, writing_mode);
         self.compute_column_measures(writing_mode);
 
@@ -1518,7 +1517,12 @@ impl<'a> TableLayout<'a> {
         containing_block_for_table: &ContainingBlock,
     ) -> IndependentLayout {
         let table_writing_mode = containing_block_for_children.style.writing_mode;
-        let layout_style = self.table.layout_style();
+        self.compute_border_collapse(table_writing_mode);
+        let layout_style = self.table.layout_style(Some(&self));
+        let depends_on_block_constraints = layout_style
+            .content_box_sizes_and_padding_border_margin(&containing_block_for_table.into())
+            .depends_on_block_constraints;
+
         self.pbm = layout_style
             .padding_border_margin_with_writing_mode_and_containing_block_inline_size(
                 table_writing_mode,
@@ -1556,10 +1560,6 @@ impl<'a> TableLayout<'a> {
         let offset_from_wrapper = -self.pbm.padding - self.pbm.border;
         let mut current_block_offset = offset_from_wrapper.block_start;
 
-        let depends_on_block_constraints = layout_style
-            .content_box_sizes_and_padding_border_margin(&containing_block_for_table.into())
-            .depends_on_block_constraints;
-
         let mut table_layout = IndependentLayout {
             fragments: Vec::new(),
             content_block_size: Zero::zero(),
@@ -1914,7 +1914,6 @@ impl<'a> TableLayout<'a> {
                 } else {
                     PhysicalVec::new(track_sizes.block, track_sizes.inline)
                 },
-                wrapper_border: self.pbm.border.to_physical(writing_mode),
             }))
         })
     }
@@ -2187,26 +2186,10 @@ impl<'a> TableLayout<'a> {
         let block_start = &collapsed_borders.block[area.block_start];
         let block_end = &collapsed_borders.block[area.block_end];
         Some(LogicalSides {
-            inline_start: if area.inline_start == 0 {
+            inline_start: inline_start.max_width / 2,
-                inline_start.max_width - self.pbm.border.inline_start
+            inline_end: inline_end.max_width / 2,
-            } else {
+            block_start: block_start.max_width / 2,
-                inline_start.max_width / 2
+            block_end: block_end.max_width / 2,
-            },
-            inline_end: if area.inline_end == self.table.size.width {
-                inline_end.max_width - self.pbm.border.inline_end
-            } else {
-                inline_end.max_width / 2
-            },
-            block_start: if area.block_start == 0 {
-                block_start.max_width - self.pbm.border.block_start
-            } else {
-                block_start.max_width / 2
-            },
-            block_end: if area.block_end == self.table.size.height {
-                block_end.max_width - self.pbm.border.block_end
-            } else {
-                block_end.max_width / 2
-            },
         })
     }
 }
@@ -2638,9 +2621,10 @@ impl ComputeInlineContentSizes for Table {
         constraint_space: &ConstraintSpace,
     ) -> InlineContentSizesResult {
         let writing_mode = constraint_space.writing_mode;
-        let layout_style = self.layout_style();
         let mut layout = TableLayout::new(self);
-        layout.pbm = layout_style
+        layout.compute_border_collapse(writing_mode);
+        layout.pbm = self
+            .layout_style(Some(&layout))
             .padding_border_margin_with_writing_mode_and_containing_block_inline_size(
                 writing_mode,
                 Au::zero(),
@@ -2655,6 +2639,7 @@ impl ComputeInlineContentSizes for Table {
             // Padding and border should apply to the table grid, but they will be taken into
             // account when computing the inline content sizes of the table wrapper (our parent), so
             // this code removes their contribution from the inline content size of the caption.
+            let layout_style = self.layout_style(Some(&layout));
             let padding = layout_style
                 .padding(writing_mode)
                 .percentages_relative_to(Au::zero());
@@ -2677,8 +2662,14 @@ impl ComputeInlineContentSizes for Table {
 
 impl Table {
     #[inline]
-    pub(crate) fn layout_style(&self) -> LayoutStyle {
+    pub(crate) fn layout_style<'a>(
-        LayoutStyle::Table(&self.style)
+        &'a self,
+        layout: Option<&'a TableLayout<'a>>,
+    ) -> LayoutStyle<'a> {
+        LayoutStyle::Table(TableLayoutStyle {
+            table: self,
+            layout,
+        })
     }
 
     #[inline]
@@ -2701,6 +2692,37 @@ impl TableTrackGroup {
     }
 }
 
+impl TableLayoutStyle<'_> {
+    #[inline]
+    pub(crate) fn style(&self) -> &ComputedValues {
+        &self.table.style
+    }
+
+    #[inline]
+    pub(crate) fn collapses_borders(&self) -> bool {
+        self.style().get_inherited_table().border_collapse == BorderCollapse::Collapse
+    }
+
+    pub(crate) fn halved_collapsed_border_widths(&self) -> LogicalSides<Au> {
+        debug_assert!(self.collapses_borders());
+        let area = LogicalSides {
+            inline_start: 0,
+            inline_end: self.table.size.width,
+            block_start: 0,
+            block_end: self.table.size.height,
+        };
+        if let Some(layout) = self.layout {
+            layout.get_collapsed_border_widths_for_area(area)
+        } else {
+            // TODO: this should be cached.
+            let mut layout = TableLayout::new(self.table);
+            layout.compute_border_collapse(self.style().writing_mode);
+            layout.get_collapsed_border_widths_for_area(area)
+        }
+        .expect("Collapsed borders should be computed")
+    }
+}
+
 impl TableSlotCell {
     #[inline]
     fn layout_style(&self) -> LayoutStyle {

components/layout_2020/table/mod.rs

@@ -84,9 +84,10 @@ use crate::cell::ArcRefCell;
 use crate::flow::BlockContainer;
 use crate::formatting_contexts::IndependentFormattingContext;
 use crate::fragment_tree::BaseFragmentInfo;
-use crate::geom::{PhysicalSides, PhysicalVec};
+use crate::geom::PhysicalVec;
 use crate::layout_box_base::LayoutBoxBase;
 use crate::style_ext::BorderStyleColor;
+use crate::table::layout::TableLayout;
 
 pub type TableSize = Size2D<usize, UnknownUnit>;
 
@@ -336,5 +337,9 @@ pub(crate) struct CollapsedBorderLine {
 pub(crate) struct SpecificTableGridInfo {
     pub collapsed_borders: PhysicalVec<Vec<CollapsedBorderLine>>,
     pub track_sizes: PhysicalVec<Vec<Au>>,
-    pub wrapper_border: PhysicalSides<Au>,
+}
+
+pub(crate) struct TableLayoutStyle<'a> {
+    table: &'a Table,
+    layout: Option<&'a TableLayout<'a>>,
 }

tests/wpt/meta/MANIFEST.json

@@ -586076,7 +586076,7 @@
       ]
      ],
      "visibility-collapse-rowspan-004-dynamic.html": [
-      "e6f65c450f7cf3da4d8f745306065278f9471036",
+      "c2f1a114fb4333aaef5459989019ee6e8c858bb5",
       [
        null,
        {}

tests/wpt/meta/css/CSS2/borders/border-applies-to-001.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-001.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-applies-to-002.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-002.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-applies-to-003.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-003.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-applies-to-004.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-004.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-applies-to-005.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-005.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-applies-to-007.xht.ini

@@ -1,2 +0,0 @@
-[border-applies-to-007.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-001.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-001.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-002.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-002.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-003.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-003.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-004.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-004.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-005.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-005.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/borders/border-color-applies-to-007.xht.ini

@@ -1,2 +0,0 @@
-[border-color-applies-to-007.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/tables/collapsing-border-model-003.xht.ini

@@ -1,2 +0,0 @@
-[collapsing-border-model-003.xht]
-  expected: FAIL

tests/wpt/meta/css/CSS2/tables/collapsing-border-model-009.xht.ini

@@ -1,2 +0,0 @@
-[collapsing-border-model-009.xht]
-  expected: FAIL

tests/wpt/meta/css/css-tables/border-collapse-dynamic-section.html.ini

@@ -1,2 +0,0 @@
-[border-collapse-dynamic-section.html]
-  expected: FAIL

tests/wpt/tests/css/css-tables/visibility-collapse-rowspan-004-dynamic.html

@@ -70,105 +70,114 @@ table td {
                 tests[i][2]);
         };
     }
+    function width(element) {
+        return element.getBoundingClientRect().width;
+    }
+    function height(element) {
+        return element.getBoundingClientRect().height;
+    }
     document.getElementById("thirdRow").style.visibility = "collapse";
     tests = [
         [
-            document.getElementById('two').offsetWidth,
+            width(document.getElementById('two')),
-            document.getElementById('one').offsetWidth,
+            width(document.getElementById('one')),
             "spanning row visibility:collapse doesn't change table width"
         ],
         [
-            document.getElementById('firstRow').offsetHeight,
+            height(document.getElementById('firstRow')),
-            document.getElementById('firstRowRef').offsetHeight,
+            height(document.getElementById('firstRowRef')),
             "when third row is collapsed, first row stays the same height"
         ],
         [
-            document.getElementById('secondRow').offsetHeight,
+            height(document.getElementById('secondRow')),
-            document.getElementById('secondRowRef').offsetHeight,
+            height(document.getElementById('secondRowRef')),
             "when third row is collapsed, second row stays the same height"
         ],
-        [   document.getElementById('thirdRow').offsetHeight,
+        [
+            height(document.getElementById('thirdRow')),
             0,
             "third row visibility:collapse makes row height 0"
         ],
         [
-               document.getElementById('fourthRow').offsetHeight,
+            height(document.getElementById('fourthRow')),
-            document.getElementById('fourthRowRef').offsetHeight,
+            height(document.getElementById('fourthRowRef')),
             "when third row is collapsed, fourth row stays the same height"
         ],
         [
-            document.getElementById('spanningCell').offsetHeight,
+            height(document.getElementById('spanningCell')),
-            document.getElementById('firstRow').offsetHeight +
+            height(document.getElementById('firstRow')) +
-            document.getElementById('secondRow').offsetHeight +
+            height(document.getElementById('secondRow')) +
-            document.getElementById('fourthRow').offsetHeight +
+            height(document.getElementById('fourthRow')) +
-            document.getElementById('fifthRow').offsetHeight,
+            height(document.getElementById('fifthRow')),
             "spanning cell shrinks to sum of remaining three rows' height"
         ]];
     runTests();
     document.getElementById("thirdRow").style.visibility = "visible";
     tests = [
         [
-            document.getElementById('firstRow').offsetHeight,
+            height(document.getElementById('firstRow')),
-            document.getElementById('firstRowRef').offsetHeight,
+            height(document.getElementById('firstRowRef')),
             "when third row is visible, first row stays the same height"
         ],
         [
-            document.getElementById('secondRow').offsetHeight,
+            height(document.getElementById('secondRow')),
-            document.getElementById('secondRowRef').offsetHeight,
+            height(document.getElementById('secondRowRef')),
             "when third row is visible, second row stays the same height"
         ],
-        [   document.getElementById('thirdRow').offsetHeight,
+        [
-            document.getElementById('secondRowRef').offsetHeight,
+            height(document.getElementById('thirdRow')),
+            height(document.getElementById('secondRowRef')),
             "when third row is visible, third row stays the same height"
         ],
         [
-            document.getElementById('fourthRow').offsetHeight,
+            height(document.getElementById('fourthRow')),
-            document.getElementById('fourthRowRef').offsetHeight,
+            height(document.getElementById('fourthRowRef')),
             "when third row is visible, fourth row stays the same height"
         ],
         [
-            document.getElementById('fifthRow').offsetHeight,
+            height(document.getElementById('fifthRow')),
-            document.getElementById('fifthRowRef').offsetHeight,
+            height(document.getElementById('fifthRowRef')),
             "when third row is visible, fifth row stays the same height"
         ],
         [
-            document.getElementById('spanningCell').offsetHeight,
+            height(document.getElementById('spanningCell')),
-            document.getElementById('spanningCellRef').offsetHeight,
+            height(document.getElementById('spanningCellRef')),
             "when third row is visible, spanning cell stays the same height"
         ]];
     runTests();
     document.getElementById("thirdRow").style.visibility = "collapse";
     tests = [
         [
-            document.getElementById('two').offsetWidth,
+            width(document.getElementById('two')),
-            document.getElementById('one').offsetWidth,
+            width(document.getElementById('one')),
             "(2nd collapse) spanning row visibility:collapse doesn't change table width"
         ],
         [
-            document.getElementById('firstRow').offsetHeight,
+            height(document.getElementById('firstRow')),
-            document.getElementById('firstRowRef').offsetHeight,
+            height(document.getElementById('firstRowRef')),
             "when third row is collapsed again, first row stays the same height"
         ],
         [
-            document.getElementById('secondRow').offsetHeight,
+            height(document.getElementById('secondRow')),
-            document.getElementById('secondRowRef').offsetHeight,
+            height(document.getElementById('secondRowRef')),
             "when third row is collapsed again, second row stays the same height"
         ],
-        [   document.getElementById('thirdRow').offsetHeight,
+        [
+            height(document.getElementById('thirdRow')),
             0,
             "(2nd collapse) third row visibility:collapse makes row height 0"
         ],
         [
-            document.getElementById('fourthRow').offsetHeight,
+            height(document.getElementById('fourthRow')),
-            document.getElementById('fourthRowRef').offsetHeight,
+            height(document.getElementById('fourthRowRef')),
             "when third row is collapsed again, fourth row stays the same height"
         ],
         [
-            document.getElementById('spanningCell').offsetHeight,
+            height(document.getElementById('spanningCell')),
-            document.getElementById('firstRow').offsetHeight +
+            height(document.getElementById('firstRow')) +
-            document.getElementById('secondRow').offsetHeight +
+            height(document.getElementById('secondRow')) +
-            document.getElementById('fourthRow').offsetHeight +
+            height(document.getElementById('fourthRow')) +
-            document.getElementById('fifthRow').offsetHeight,
+            height(document.getElementById('fifthRow')),
             "(2nd collapse) spanning cell shrinks to sum of remaining three rows' height"
         ]];
     runTests();