From 7301af8468c380a70d839cfbadf933180b7ab24f Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Mon, 3 Feb 2025 15:46:40 +0100 Subject: [PATCH] layout: Fix painting order of collapsed table borders (#35219) In #35075 I painted them in front of the decorations (backgrounds and borders) of any block-level box in the same stacking context. I did that because other browsers paint them in front of the decorations of the descendants of the table, but my approach also painted them in front of decorations of following siblings of the table, which was wrong. This patch makes it so that collapsed table orders are painted in the same step as decorations. However, tables defer painting their collapsed borders after the decorations of their descendants. This matches Blink and WebKit, and brings us closer to Gecko (which is slightly different in some corner cases). Signed-off-by: Oriol Brufau --- components/layout_2020/display_list/mod.rs | 23 +-- .../display_list/stacking_context.rs | 68 ++++--- tests/wpt/meta/MANIFEST.json | 169 ++++++++++++++++++ .../collapsed-borders-painting-order-001.html | 17 ++ .../collapsed-borders-painting-order-002.html | 18 ++ .../collapsed-borders-painting-order-003.html | 18 ++ .../collapsed-borders-painting-order-004.html | 16 ++ .../collapsed-borders-painting-order-005.html | 16 ++ .../collapsed-borders-painting-order-006.html | 16 ++ .../collapsed-borders-painting-order-007.html | 19 ++ .../collapsed-borders-painting-order-008.html | 20 +++ .../collapsed-borders-painting-order-009.html | 19 ++ .../collapsed-borders-painting-order-010.html | 19 ++ .../collapsed-borders-painting-order-011.html | 20 +++ .../collapsed-borders-painting-order-012.html | 19 ++ .../collapsed-borders-painting-order-013.html | 19 ++ 16 files changed, 461 insertions(+), 35 deletions(-) create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-001.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-002.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-003.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-004.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-005.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-006.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-007.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-008.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-009.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-010.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-011.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-012.html create mode 100644 tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-013.html diff --git a/components/layout_2020/display_list/mod.rs b/components/layout_2020/display_list/mod.rs index 070e2b0918c..7dc3823d437 100644 --- a/components/layout_2020/display_list/mod.rs +++ b/components/layout_2020/display_list/mod.rs @@ -245,6 +245,7 @@ impl Fragment { containing_block: &PhysicalRect, section: StackingContextSection, is_hit_test_for_scrollable_overflow: bool, + is_collapsed_table_borders: bool, ) { match self { Fragment::Box(box_fragment) | Fragment::Float(box_fragment) => { @@ -254,6 +255,7 @@ impl Fragment { box_fragment, containing_block, is_hit_test_for_scrollable_overflow, + is_collapsed_table_borders, ) .build(builder, section), Visibility::Hidden => (), @@ -515,6 +517,7 @@ struct BuilderForBoxFragment<'a> { padding_edge_clip_chain_id: RefCell>, content_edge_clip_chain_id: RefCell>, is_hit_test_for_scrollable_overflow: bool, + is_collapsed_table_borders: bool, } impl<'a> BuilderForBoxFragment<'a> { @@ -522,6 +525,7 @@ impl<'a> BuilderForBoxFragment<'a> { fragment: &'a BoxFragment, containing_block: &'a PhysicalRect, is_hit_test_for_scrollable_overflow: bool, + is_collapsed_table_borders: bool, ) -> Self { let border_rect = fragment .border_rect() @@ -562,6 +566,7 @@ impl<'a> BuilderForBoxFragment<'a> { padding_edge_clip_chain_id: RefCell::new(None), content_edge_clip_chain_id: RefCell::new(None), is_hit_test_for_scrollable_overflow, + is_collapsed_table_borders, } } @@ -652,16 +657,14 @@ impl<'a> BuilderForBoxFragment<'a> { return; } - match section { - StackingContextSection::CollapsedTableBorders => { - self.build_collapsed_table_borders(builder); - return; - }, - StackingContextSection::Outline => { - self.build_outline(builder); - return; - }, - _ => {}, + if self.is_collapsed_table_borders { + self.build_collapsed_table_borders(builder); + return; + } + + if section == StackingContextSection::Outline { + self.build_outline(builder); + return; } self.build_hit_test(builder, self.border_rect); diff --git a/components/layout_2020/display_list/stacking_context.rs b/components/layout_2020/display_list/stacking_context.rs index 6ae87d331f8..69a45a48916 100644 --- a/components/layout_2020/display_list/stacking_context.rs +++ b/components/layout_2020/display_list/stacking_context.rs @@ -88,7 +88,6 @@ pub(crate) type ContainingBlockInfo<'a> = ContainingBlockManager<'a, ContainingB pub(crate) enum StackingContextSection { OwnBackgroundsAndBorders, DescendantBackgroundsAndBorders, - CollapsedTableBorders, Foreground, Outline, } @@ -262,6 +261,7 @@ pub(crate) enum StackingContextContent { containing_block: PhysicalRect, fragment: Fragment, is_hit_test_for_scrollable_overflow: bool, + is_collapsed_table_borders: bool, }, /// An index into [StackingContext::atomic_inline_stacking_containers]. @@ -292,6 +292,7 @@ impl StackingContextContent { containing_block, fragment, is_hit_test_for_scrollable_overflow, + is_collapsed_table_borders, } => { builder.current_scroll_node_id = *scroll_node_id; builder.current_reference_frame_scroll_node_id = *reference_frame_scroll_node_id; @@ -301,6 +302,7 @@ impl StackingContextContent { containing_block, *section, *is_hit_test_for_scrollable_overflow, + *is_collapsed_table_borders, ); }, Self::AtomicInlineStackingContainer { index } => { @@ -669,8 +671,12 @@ impl StackingContext { } } - let mut fragment_builder = - BuilderForBoxFragment::new(box_fragment, containing_block, false); + let mut fragment_builder = BuilderForBoxFragment::new( + box_fragment, + containing_block, + false, /* is_hit_test_for_scrollable_overflow */ + false, /* is_collapsed_table_borders */ + ); let painter = super::background::BackgroundPainter { style, painting_area_override: Some(painting_area), @@ -728,16 +734,6 @@ impl StackingContext { child.build_display_list(builder, &self.atomic_inline_stacking_containers); } - // Additional step 4.5: Collapsed table borders - // This step isn't in the spec, but other browsers seem to paint them at this point. - while contents.peek().is_some_and(|(_, child)| { - child.section() == StackingContextSection::CollapsedTableBorders - }) { - let (i, child) = contents.next().unwrap(); - self.debug_push_print_item(DebugPrintField::Contents, i); - child.build_display_list(builder, &self.atomic_inline_stacking_containers); - } - // Step 5: Float stacking containers for (i, child) in self.float_stacking_containers.iter().enumerate() { self.debug_push_print_item(DebugPrintField::FloatStackingContainers, i); @@ -928,6 +924,7 @@ impl Fragment { containing_block: containing_block.rect, fragment: fragment_clone, is_hit_test_for_scrollable_overflow: false, + is_collapsed_table_borders: false, }); }, } @@ -1101,7 +1098,12 @@ impl BoxFragment { display_list, &containing_block.scroll_node_id, &containing_block.clip_chain_id, - BuilderForBoxFragment::new(self, &containing_block.rect, false), + BuilderForBoxFragment::new( + self, + &containing_block.rect, + false, /* is_hit_test_for_scrollable_overflow */ + false, /* is_collapsed_table_borders */ + ), ) .unwrap_or(containing_block.clip_chain_id); @@ -1173,7 +1175,12 @@ impl BoxFragment { display_list, &new_scroll_node_id, &new_clip_chain_id, - BuilderForBoxFragment::new(self, &containing_block.rect, false), + BuilderForBoxFragment::new( + self, + &containing_block.rect, + false, /* is_hit_test_for_scrollable_overflow*/ + false, /* is_collapsed_table_borders */ + ), ) { new_clip_chain_id = clip_chain_id; } @@ -1205,20 +1212,11 @@ impl BoxFragment { containing_block: containing_block.rect, fragment: fragment.clone(), is_hit_test_for_scrollable_overflow: false, + is_collapsed_table_borders: false, }); }; add_fragment(self.get_stacking_context_section()); - - if let Fragment::Box(box_fragment) = &fragment { - if matches!( - box_fragment.borrow().specific_layout_info, - Some(SpecificLayoutInfo::TableGridWithCollapsedBorders(_)) - ) { - add_fragment(StackingContextSection::CollapsedTableBorders); - } - } - if !self.style.get_outline().outline_width.is_zero() { add_fragment(StackingContextSection::Outline); } @@ -1245,6 +1243,7 @@ impl BoxFragment { containing_block: containing_block.rect, fragment: fragment.clone(), is_hit_test_for_scrollable_overflow: true, + is_collapsed_table_borders: false, }); } @@ -1293,6 +1292,25 @@ impl BoxFragment { StackingContextBuildMode::SkipHoisted, ); } + + if matches!(&fragment, Fragment::Box(box_fragment) if matches!( + box_fragment.borrow().specific_layout_info, + Some(SpecificLayoutInfo::TableGridWithCollapsedBorders(_)) + )) { + stacking_context + .contents + .push(StackingContextContent::Fragment { + scroll_node_id: new_scroll_node_id, + reference_frame_scroll_node_id: reference_frame_scroll_node_id_for_fragments, + clip_chain_id: new_clip_chain_id, + // TODO: should this use `self.get_stacking_context_section()`? + section: StackingContextSection::DescendantBackgroundsAndBorders, + containing_block: containing_block.rect, + fragment: fragment.clone(), + is_hit_test_for_scrollable_overflow: false, + is_collapsed_table_borders: true, + }); + } } fn build_clip_frame_if_necessary( diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 6438fdaff1a..87a07138782 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -248194,6 +248194,175 @@ {} ] ], + "collapsed-borders-painting-order-001.html": [ + "df9e254e5e8acbcdab45966d5a1e4439a4fa8216", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-002.html": [ + "ef78b68a7b86ce888982f52886ecc96ee948fb2b", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-003.html": [ + "5ec44ac207042eb8d918106f7333c0ecb83bc28d", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-004.html": [ + "b727fef9155bcbc7e2d387707e6d9617bb5e7033", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-005.html": [ + "944db4e9eec534bbaec766fac7d5a790746a4843", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-006.html": [ + "fa27a19e52b749134c1c2d8d048051ac95b11625", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-007.html": [ + "e7129d68a3ff0675f2a1f23fa65dfe06833e5101", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-008.html": [ + "b0d76adda9d16c9d7e96b6c70a948445115d7107", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-009.html": [ + "cc643dcb5564edd3a9c1a32605febf123578f1e6", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-010.html": [ + "e89e1d202ab17c7d008b4798bbe1b7f0878ed06e", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-011.html": [ + "174e16da3ef13be6f5a7caf55f3a6ad48c58c8d1", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-012.html": [ + "bf907dfa6c74c26b375f254f2a50a1c39e811dbd", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], + "collapsed-borders-painting-order-013.html": [ + "9a0a860d35639b7dfb2a71f601746bfe41a18384", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], "padding-percentage.html": [ "67f8009de365a32275232b5c5b008072a0c8fc17", [ diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-001.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-001.html new file mode 100644 index 00000000000..df9e254e5e8 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-001.html @@ -0,0 +1,17 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-002.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-002.html new file mode 100644 index 00000000000..ef78b68a7b8 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-002.html @@ -0,0 +1,18 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-003.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-003.html new file mode 100644 index 00000000000..5ec44ac2070 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-003.html @@ -0,0 +1,18 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-004.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-004.html new file mode 100644 index 00000000000..b727fef9155 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-004.html @@ -0,0 +1,16 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-005.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-005.html new file mode 100644 index 00000000000..944db4e9eec --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-005.html @@ -0,0 +1,16 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-006.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-006.html new file mode 100644 index 00000000000..fa27a19e52b --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-006.html @@ -0,0 +1,16 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-007.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-007.html new file mode 100644 index 00000000000..e7129d68a3f --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-007.html @@ -0,0 +1,19 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-008.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-008.html new file mode 100644 index 00000000000..b0d76adda9d --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-008.html @@ -0,0 +1,20 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-009.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-009.html new file mode 100644 index 00000000000..cc643dcb556 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-009.html @@ -0,0 +1,19 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-010.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-010.html new file mode 100644 index 00000000000..e89e1d202ab --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-010.html @@ -0,0 +1,19 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-011.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-011.html new file mode 100644 index 00000000000..174e16da3ef --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-011.html @@ -0,0 +1,20 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-012.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-012.html new file mode 100644 index 00000000000..bf907dfa6c7 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-012.html @@ -0,0 +1,19 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+
diff --git a/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-013.html b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-013.html new file mode 100644 index 00000000000..9a0a860d356 --- /dev/null +++ b/tests/wpt/tests/css/css-tables/tentative/collapsed-borders-painting-order-013.html @@ -0,0 +1,19 @@ + +Painting order of collapsed table borders + + + + + + +

Test passes if there is a filled green square and no red.

+ + +
+ + +
+