From 9175e598adcf33097cd6bd29a1e2b428d564c295 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Wed, 11 Sep 2024 17:30:17 +0200 Subject: [PATCH] Let table-related boxes adjust their `overflow` values (#33400) The `overflow` property doesn't apply to table track and track groups, and table elements only accept a few `overflow` values. Therefore, this patch adds an `effective_overflow()` method to get the actual value that needs to be used. Signed-off-by: Oriol Brufau Co-authored-by: Martin Robinson --- .../display_list/stacking_context.rs | 5 +- components/layout_2020/flow/root.rs | 4 +- .../layout_2020/fragment_tree/box_fragment.rs | 10 ++-- components/layout_2020/style_ext.rs | 58 ++++++++++++++++++- .../paint/overflow-hidden-table.html.ini | 2 - .../css/css-ui/text-overflow-025.html.ini | 2 - 6 files changed, 64 insertions(+), 17 deletions(-) delete mode 100644 tests/wpt/meta/css/css-tables/tentative/paint/overflow-hidden-table.html.ini delete mode 100644 tests/wpt/meta/css/css-ui/text-overflow-025.html.ini diff --git a/components/layout_2020/display_list/stacking_context.rs b/components/layout_2020/display_list/stacking_context.rs index d69e9330371..6809c9df577 100644 --- a/components/layout_2020/display_list/stacking_context.rs +++ b/components/layout_2020/display_list/stacking_context.rs @@ -1287,8 +1287,7 @@ impl BoxFragment { parent_clip_id: &wr::ClipChainId, containing_block_rect: &PhysicalRect, ) -> Option<(ScrollTreeNodeId, wr::ClipChainId, LayoutSize)> { - let overflow_x = self.style.get_box().overflow_x; - let overflow_y = self.style.get_box().overflow_y; + let overflow = self.style.effective_overflow(); if !self.style.establishes_scroll_container() { return None; } @@ -1318,7 +1317,7 @@ impl BoxFragment { ); let sensitivity = - if ComputedOverflow::Hidden == overflow_x && ComputedOverflow::Hidden == overflow_y { + if ComputedOverflow::Hidden == overflow.x && ComputedOverflow::Hidden == overflow.y { ScrollSensitivity::Script } else { ScrollSensitivity::ScriptAndInputEvents diff --git a/components/layout_2020/flow/root.rs b/components/layout_2020/flow/root.rs index 26a1f94753d..da4584e631a 100644 --- a/components/layout_2020/flow/root.rs +++ b/components/layout_2020/flow/root.rs @@ -67,7 +67,7 @@ impl BoxTree { // TODO: This should handle when different overflow is set multiple axes, which requires the // compositor scroll tree to allow setting a value per axis. let root_style = root_element.style(context); - let mut root_overflow = root_style.get_box().overflow_y; + let mut root_overflow = root_style.effective_overflow().y; if root_overflow == Overflow::Visible && !root_style.get_box().display.is_none() { for child in iter_child_nodes(root_element) { if !child.to_threadsafe().as_element().map_or(false, |element| { @@ -78,7 +78,7 @@ impl BoxTree { let style = child.style(context); if !style.get_box().display.is_none() { - root_overflow = style.get_box().overflow_y; + root_overflow = style.effective_overflow().y; break; } } diff --git a/components/layout_2020/fragment_tree/box_fragment.rs b/components/layout_2020/fragment_tree/box_fragment.rs index eac31c6a478..c16e1f17231 100644 --- a/components/layout_2020/fragment_tree/box_fragment.rs +++ b/components/layout_2020/fragment_tree/box_fragment.rs @@ -241,7 +241,7 @@ impl BoxFragment { \nclearance={:?}\ \nscrollable_overflow={:?}\ \nbaselines={:?}\ - \noverflow={:?} / {:?}", + \noverflow={:?}", self.base, self.content_rect, self.padding_rect(), @@ -250,8 +250,7 @@ impl BoxFragment { self.clearance, self.scrollable_overflow(), self.baselines, - self.style.get_box().overflow_x, - self.style.get_box().overflow_y, + self.style.effective_overflow(), )); for child in &self.children { @@ -274,12 +273,13 @@ impl BoxFragment { overflow.max_y().max(scrollable_overflow.max_y()), ); - if self.style.get_box().overflow_y == ComputedOverflow::Visible { + let overflow_style = self.style.effective_overflow(); + if overflow_style.y == ComputedOverflow::Visible { overflow.origin.y = overflow.origin.y.min(scrollable_overflow.origin.y); overflow.size.height = bottom_right.y - overflow.origin.y; } - if self.style.get_box().overflow_x == ComputedOverflow::Visible { + if overflow_style.x == ComputedOverflow::Visible { overflow.origin.x = overflow.origin.x.min(scrollable_overflow.origin.x); overflow.size.width = bottom_right.x - overflow.origin.x; } diff --git a/components/layout_2020/style_ext.rs b/components/layout_2020/style_ext.rs index 83b6de39f60..c7eb46a15dd 100644 --- a/components/layout_2020/style_ext.rs +++ b/components/layout_2020/style_ext.rs @@ -13,6 +13,7 @@ use style::properties::longhands::backface_visibility::computed_value::T as Back use style::properties::longhands::box_sizing::computed_value::T as BoxSizing; use style::properties::longhands::column_span::computed_value::T as ColumnSpan; use style::properties::ComputedValues; +use style::servo::selector_parser::PseudoElement; use style::values::computed::basic_shape::ClipPath; use style::values::computed::image::Image as ComputedImageLayer; use style::values::computed::{AlignItems, LengthPercentage, NonNegativeLengthPercentage, Size}; @@ -29,6 +30,7 @@ use crate::dom_traversal::Contents; use crate::fragment_tree::FragmentFlags; use crate::geom::{ AuOrAuto, LengthPercentageOrAuto, LogicalSides, LogicalVec2, PhysicalSides, PhysicalSize, + PhysicalVec, }; use crate::{ContainingBlock, IndefiniteContainingBlock}; @@ -255,6 +257,7 @@ pub(crate) trait ComputedValuesExt { ) -> LogicalSides>; fn has_transform_or_perspective(&self, fragment_flags: FragmentFlags) -> bool; fn effective_z_index(&self, fragment_flags: FragmentFlags) -> i32; + fn effective_overflow(&self) -> PhysicalVec; fn establishes_block_formatting_context(&self) -> bool; fn establishes_stacking_context(&self, fragment_flags: FragmentFlags) -> bool; fn establishes_scroll_container(&self) -> bool; @@ -614,10 +617,60 @@ impl ComputedValuesExt for ComputedValues { } } + /// Get the effective overflow of this box. The property only applies to block containers, + /// flex containers, and grid containers. And some box types only accept a few values. + /// + fn effective_overflow(&self) -> PhysicalVec { + let style_box = self.get_box(); + let mut overflow_x = style_box.overflow_x; + let mut overflow_y = style_box.overflow_y; + // According to , + // overflow applies to table-wrapper boxes and not to table grid boxes. + // That's what Blink and WebKit do, however Firefox matches a CSSWG resolution that says + // the opposite: + // Due to the way that we implement table-wrapper boxes, it's easier to align with Firefox. + match style_box.display.inside() { + stylo::DisplayInside::Table + if matches!(self.pseudo(), Some(PseudoElement::ServoTableGrid)) => + { + // + // Tables ignore overflow values different than visible, clip and hidden. + // We also need to make sure that both axes have the same scrollability. + if matches!(overflow_x, Overflow::Auto | Overflow::Scroll) { + overflow_x = Overflow::Visible; + if overflow_y.is_scrollable() { + overflow_y = Overflow::Visible; + } + } + if matches!(overflow_y, Overflow::Auto | Overflow::Scroll) { + overflow_y = Overflow::Visible; + if overflow_x.is_scrollable() { + overflow_x = Overflow::Visible; + } + } + }, + stylo::DisplayInside::TableColumn | + stylo::DisplayInside::TableColumnGroup | + stylo::DisplayInside::TableRow | + stylo::DisplayInside::TableRowGroup | + stylo::DisplayInside::TableHeaderGroup | + stylo::DisplayInside::TableFooterGroup | + stylo::DisplayInside::Table => { + // + // Table-track and table-track-group boxes ignore overflow. + // We also ignore it on table-wrapper boxes (see above). + overflow_x = Overflow::Visible; + overflow_y = Overflow::Visible; + }, + _ => {}, + } + PhysicalVec::new(overflow_x, overflow_y) + } + /// Return true if this style is a normal block and establishes /// a new block formatting context. fn establishes_block_formatting_context(&self) -> bool { - if self.get_box().overflow_x.is_scrollable() { + if self.establishes_scroll_container() { return true; } @@ -635,8 +688,7 @@ impl ComputedValuesExt for ComputedValues { /// Whether or not the `overflow` value of this style establishes a scroll container. fn establishes_scroll_container(&self) -> bool { - self.get_box().overflow_x != Overflow::Visible || - self.get_box().overflow_y != Overflow::Visible + self.effective_overflow().x.is_scrollable() } /// Returns true if this fragment establishes a new stacking context and false otherwise. diff --git a/tests/wpt/meta/css/css-tables/tentative/paint/overflow-hidden-table.html.ini b/tests/wpt/meta/css/css-tables/tentative/paint/overflow-hidden-table.html.ini deleted file mode 100644 index 85975fc74ce..00000000000 --- a/tests/wpt/meta/css/css-tables/tentative/paint/overflow-hidden-table.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[overflow-hidden-table.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-ui/text-overflow-025.html.ini b/tests/wpt/meta/css/css-ui/text-overflow-025.html.ini deleted file mode 100644 index a7d356084b2..00000000000 --- a/tests/wpt/meta/css/css-ui/text-overflow-025.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[text-overflow-025.html] - expected: FAIL