layout: Make bottom table captions obey relative positioning offsets (#39388)

#33426 only added support for relative positioning on captions with
`caption-side: top`, but forgot about `caption-side: bottom`. This
unifies the logic for both kinds of captions to avoid divergences.

Testing: Modifying an existing test to also cover this case.
Fixes: #39386

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

Cargo.lock

@@ -4881,6 +4881,7 @@ dependencies = [
  "servo_malloc_size_of",
  "servo_url",
  "smallvec",
+ "strum",
  "stylo",
  "stylo_atoms",
  "stylo_traits",

components/layout/Cargo.toml

@@ -54,6 +54,7 @@ servo_config = { path = "../config" }
 servo_geometry = { path = "../geometry" }
 servo_url = { path = "../url" }
 smallvec = { workspace = true }
+strum = { workspace = true }
 stylo = { workspace = true }
 stylo_atoms = { workspace = true }
 stylo_traits = { workspace = true }

components/layout/table/layout.rs

@@ -11,6 +11,7 @@ use atomic_refcell::AtomicRef;
 use log::warn;
 use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator};
 use servo_arc::Arc;
+use strum::{EnumIter, IntoEnumIterator};
 use style::Zero;
 use style::computed_values::border_collapse::T as BorderCollapse;
 use style::computed_values::box_sizing::T as BoxSizing;
@@ -1593,155 +1594,129 @@ impl<'a> TableLayout<'a> {
             collapsible_margins_in_children: CollapsedBlockMargins::zero(),
         };
 
-        table_layout
+        #[derive(EnumIter, PartialEq)]
-            .fragments
+        enum TableWrapperSection {
-            .extend(self.table.captions.iter().filter_map(|caption| {
+            TopCaptions,
-                let caption = caption.borrow();
+            Grid,
-                if caption.context.style().clone_caption_side() != CaptionSide::Top {
+            BottomCaptions,
-                    return None;
+        }
+        impl TableWrapperSection {
+            fn accepts_caption(&self, caption: &TableCaption) -> bool {
+                match caption.context.style().clone_caption_side() {
+                    CaptionSide::Top => *self == TableWrapperSection::TopCaptions,
+                    CaptionSide::Bottom => *self == TableWrapperSection::BottomCaptions,
                 }
+            }
+        }
 
+        for section in TableWrapperSection::iter() {
+            if section == TableWrapperSection::Grid {
                 let original_positioning_context_length = positioning_context.len();
-                let mut caption_fragment =
+                let mut grid_fragment = self.layout_grid(
-                    self.layout_caption(&caption, layout_context, positioning_context);
+                    layout_context,
+                    positioning_context,
+                    &containing_block_for_logical_conversion,
+                    containing_block_for_children,
+                );
 
-                // The caption is not placed yet. Construct a rectangle for it in the adjusted containing block
+                // Take the baseline of the grid fragment, after adjusting it to be in the coordinate system
-                // for the table children and only then convert the result to physical geometry.
+                // of the table wrapper.
-                let caption_pbm = caption_fragment
+                let logical_grid_content_rect = grid_fragment
+                    .content_rect
+                    .to_logical(&containing_block_for_logical_conversion);
+                let grid_pbm = grid_fragment
                     .padding_border_margin()
                     .to_logical(table_writing_mode);
+                table_layout.baselines = grid_fragment.baselines(table_writing_mode).offset(
+                    current_block_offset +
+                        logical_grid_content_rect.start_corner.block +
+                        grid_pbm.block_start,
+                );
 
-                let caption_relative_offset = match caption_fragment.style.clone_position() {
+                grid_fragment.content_rect = LogicalRect {
-                    Position::Relative => {
-                        relative_adjustement(&caption_fragment.style, containing_block_for_children)
-                    },
-                    _ => LogicalVec2::zero(),
-                };
-
-                caption_fragment.content_rect = LogicalRect {
                     start_corner: LogicalVec2 {
-                        inline: offset_from_wrapper.inline_start + caption_pbm.inline_start,
+                        inline: offset_from_wrapper.inline_start + grid_pbm.inline_start,
-                        block: current_block_offset + caption_pbm.block_start,
+                        block: current_block_offset + grid_pbm.block_start,
-                    } + caption_relative_offset,
+                    },
-                    size: caption_fragment
+                    size: grid_fragment
                         .content_rect
                         .size
                         .to_logical(table_writing_mode),
                 }
                 .as_physical(Some(&containing_block_for_logical_conversion));
 
-                current_block_offset += caption_fragment
+                current_block_offset += grid_fragment
-                    .margin_rect()
+                    .border_rect()
                     .size
                     .to_logical(table_writing_mode)
                     .block;
-
+                if logical_grid_content_rect.size.inline < self.table_width {
-                let caption_fragment = Fragment::Box(ArcRefCell::new(caption_fragment));
+                    // This can happen when collapsing columns
-                positioning_context.adjust_static_position_of_hoisted_fragments(
+                    table_layout.content_inline_size_for_table =
-                    &caption_fragment,
+                        Some(logical_grid_content_rect.size.inline);
-                    original_positioning_context_length,
-                );
-
-                caption.context.base.set_fragment(caption_fragment.clone());
-                Some(caption_fragment)
-            }));
-
-        let original_positioning_context_length = positioning_context.len();
-        let mut grid_fragment = self.layout_grid(
-            layout_context,
-            positioning_context,
-            &containing_block_for_logical_conversion,
-            containing_block_for_children,
-        );
-
-        // Take the baseline of the grid fragment, after adjusting it to be in the coordinate system
-        // of the table wrapper.
-        let logical_grid_content_rect = grid_fragment
-            .content_rect
-            .to_logical(&containing_block_for_logical_conversion);
-        let grid_pbm = grid_fragment
-            .padding_border_margin()
-            .to_logical(table_writing_mode);
-        table_layout.baselines = grid_fragment.baselines(table_writing_mode).offset(
-            current_block_offset +
-                logical_grid_content_rect.start_corner.block +
-                grid_pbm.block_start,
-        );
-
-        grid_fragment.content_rect = LogicalRect {
-            start_corner: LogicalVec2 {
-                inline: offset_from_wrapper.inline_start + grid_pbm.inline_start,
-                block: current_block_offset + grid_pbm.block_start,
-            },
-            size: grid_fragment
-                .content_rect
-                .size
-                .to_logical(table_writing_mode),
-        }
-        .as_physical(Some(&containing_block_for_logical_conversion));
-
-        current_block_offset += grid_fragment
-            .border_rect()
-            .size
-            .to_logical(table_writing_mode)
-            .block;
-        if logical_grid_content_rect.size.inline < self.table_width {
-            // This can happen when collapsing columns
-            table_layout.content_inline_size_for_table =
-                Some(logical_grid_content_rect.size.inline);
-        }
-
-        let grid_fragment = Fragment::Box(ArcRefCell::new(grid_fragment));
-        positioning_context.adjust_static_position_of_hoisted_fragments(
-            &grid_fragment,
-            original_positioning_context_length,
-        );
-        table_layout.fragments.push(grid_fragment);
-
-        table_layout
-            .fragments
-            .extend(self.table.captions.iter().filter_map(|caption| {
-                let caption = caption.borrow();
-                if caption.context.style().clone_caption_side() != CaptionSide::Bottom {
-                    return None;
                 }
 
-                let original_positioning_context_length = positioning_context.len();
+                let grid_fragment = Fragment::Box(ArcRefCell::new(grid_fragment));
-                let mut caption_fragment =
+                positioning_context.adjust_static_position_of_hoisted_fragments(
-                    self.layout_caption(&caption, layout_context, positioning_context);
+                    &grid_fragment,
+                    original_positioning_context_length,
+                );
+                table_layout.fragments.push(grid_fragment);
+            } else {
+                let caption_fragments = self.table.captions.iter().filter_map(|caption| {
+                    let caption = caption.borrow();
+                    if !section.accepts_caption(&caption) {
+                        return None;
+                    }
 
-                // The caption is not placed yet. Construct a rectangle for it in the adjusted containing block
+                    let original_positioning_context_length = positioning_context.len();
-                // for the table children and only then convert the result to physical geometry.
+                    let mut caption_fragment =
-                let caption_pbm = caption_fragment
+                        self.layout_caption(&caption, layout_context, positioning_context);
-                    .padding_border_margin()
+
-                    .to_logical(table_writing_mode);
+                    // The caption is not placed yet. Construct a rectangle for it in the adjusted containing block
-                caption_fragment.content_rect = LogicalRect {
+                    // for the table children and only then convert the result to physical geometry.
-                    start_corner: LogicalVec2 {
+                    let caption_pbm = caption_fragment
-                        inline: offset_from_wrapper.inline_start + caption_pbm.inline_start,
+                        .padding_border_margin()
-                        block: current_block_offset + caption_pbm.block_start,
+                        .to_logical(table_writing_mode);
-                    },
+
-                    size: caption_fragment
+                    let caption_relative_offset = match caption_fragment.style.clone_position() {
-                        .content_rect
+                        Position::Relative => relative_adjustement(
+                            &caption_fragment.style,
+                            containing_block_for_children,
+                        ),
+                        _ => LogicalVec2::zero(),
+                    };
+
+                    caption_fragment.content_rect = LogicalRect {
+                        start_corner: LogicalVec2 {
+                            inline: offset_from_wrapper.inline_start + caption_pbm.inline_start,
+                            block: current_block_offset + caption_pbm.block_start,
+                        } + caption_relative_offset,
+                        size: caption_fragment
+                            .content_rect
+                            .size
+                            .to_logical(table_writing_mode),
+                    }
+                    .as_physical(Some(&containing_block_for_logical_conversion));
+
+                    current_block_offset += caption_fragment
+                        .margin_rect()
                         .size
-                        .to_logical(table_writing_mode),
+                        .to_logical(table_writing_mode)
-                }
+                        .block;
-                .as_physical(Some(&containing_block_for_logical_conversion));
 
-                current_block_offset += caption_fragment
+                    let caption_fragment = Fragment::Box(ArcRefCell::new(caption_fragment));
-                    .margin_rect()
+                    positioning_context.adjust_static_position_of_hoisted_fragments(
-                    .size
+                        &caption_fragment,
-                    .to_logical(table_writing_mode)
+                        original_positioning_context_length,
-                    .block;
+                    );
 
-                let caption_fragment = Fragment::Box(ArcRefCell::new(caption_fragment));
+                    caption.context.base.set_fragment(caption_fragment.clone());
-                positioning_context.adjust_static_position_of_hoisted_fragments(
+                    Some(caption_fragment)
-                    &caption_fragment,
+                });
-                    original_positioning_context_length,
+                table_layout.fragments.extend(caption_fragments);
-                );
+            }
-
+        }
-                caption.context.base.set_fragment(caption_fragment.clone());
-                Some(caption_fragment)
-            }));
 
         table_layout.content_block_size = current_block_offset + offset_from_wrapper.block_end;
         table_layout

tests/wpt/meta/MANIFEST.json

@@ -257401,7 +257401,7 @@
       ]
      ],
      "caption-relative-positioning.html": [
-      "2be1e86bc077f1293ccc9ea851f23123ae679157",
+      "083a39fdf6800bfbec43a418068573f014c8adc5",
       [
        null,
        [

tests/wpt/tests/css/css-tables/caption-relative-positioning.html

@@ -11,7 +11,7 @@
             position: relative;
             background: green;
             width: 100px;
-            height: 100px;
+            height: 50px;
             margin-left: 200px;
             left: -200px;
         }
@@ -20,7 +20,10 @@
     <p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
     <div style="width: 100px; background: red;">
         <table>
-            <caption></caption>
+            <caption style="caption-side: top"></caption>
+        </table>
+        <table>
+            <caption style="caption-side: bottom"></caption>
         </table>
     </div>
 </html>