layout: Merge BoxFragment::used_overflow into ComputedValuesExt::effective_overflow (#35670) (#35670)

* Update wpt-test



* Merge used_overflow to effective_overflow; remove duplicate call



* Remove more duplicate calls; update effective_overflow logic



* Update reference link&style



* Apply final review suggestions

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>

components/layout_2020/display_list/stacking_context.rs

@@ -39,7 +39,7 @@ use crate::fragment_tree::{
     PositioningFragment, SpecificLayoutInfo,
 };
 use crate::geom::{AuOrAuto, PhysicalRect, PhysicalSides};
-use crate::style_ext::{AxesOverflow, ComputedValuesExt};
+use crate::style_ext::ComputedValuesExt;
 
 #[derive(Clone)]
 pub(crate) struct ContainingBlock {
@@ -1372,23 +1372,6 @@ impl BoxFragment {
         Some(display_list.define_clip_chain(*parent_clip_chain_id, [clip_id]))
     }
 
-    // TODO: merge this function with style.effective_overflow()
-    fn used_overflow(&self) -> AxesOverflow {
-        let mut overflow = self.style.effective_overflow();
-        let is_replaced_element = self.base.flags.contains(FragmentFlags::IS_REPLACED);
-
-        if is_replaced_element {
-            if overflow.x != ComputedOverflow::Visible {
-                overflow.x = ComputedOverflow::Clip;
-            }
-            if overflow.y != ComputedOverflow::Visible {
-                overflow.y = ComputedOverflow::Clip;
-            }
-        }
-
-        overflow
-    }
-
     fn build_overflow_frame_if_necessary(
         &self,
         display_list: &mut DisplayList,
@@ -1396,7 +1379,7 @@ impl BoxFragment {
         parent_clip_chain_id: &wr::ClipChainId,
         containing_block_rect: &PhysicalRect<Au>,
     ) -> Option<OverflowFrameData> {
-        let overflow = self.used_overflow();
+        let overflow = self.style.effective_overflow(self.base.flags);
 
         if overflow.x == ComputedOverflow::Visible && overflow.y == ComputedOverflow::Visible {
             return None;
@@ -1483,8 +1466,6 @@ impl BoxFragment {
             display_list.wr.pipeline_id,
         );
 
-        let overflow = self.style.effective_overflow();
-
         let sensitivity = AxesScrollSensitivity {
             x: overflow.x.into(),
             y: overflow.y.into(),

components/layout_2020/flexbox/layout.rs

@@ -2510,7 +2510,7 @@ impl FlexItemBox {
     ) -> Au {
         // FIXME(stshine): Consider more situations when auto min size is not needed.
         let style = &self.independent_formatting_context.style();
-        if style.establishes_scroll_container() {
+        if style.establishes_scroll_container(self.base_fragment_info().flags) {
             return Au::zero();
         }
 

components/layout_2020/flow/construct.rs

@@ -25,6 +25,7 @@ use crate::dom_traversal::{
 use crate::flow::float::FloatBox;
 use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox};
 use crate::formatting_contexts::IndependentFormattingContext;
+use crate::fragment_tree::FragmentFlags;
 use crate::layout_box_base::LayoutBoxBase;
 use crate::positioned::AbsolutelyPositionedBox;
 use crate::style_ext::{ComputedValuesExt, DisplayGeneratingBox, DisplayInside, DisplayOutside};
@@ -493,7 +494,11 @@ where
         let kind = match contents {
             Contents::NonReplaced(contents) => match display_inside {
                 DisplayInside::Flow { is_list_item }
-                    if !info.style.establishes_block_formatting_context() =>
+                    // Fragment flags are just used to indicate that the element is not replaced, so empty
+                    // flags are okay here.
+                    if !info.style.establishes_block_formatting_context(
+                        FragmentFlags::empty()
+                    ) =>
                 {
                     BlockLevelCreator::SameFormattingContextBlock(
                         IntermediateBlockContainer::Deferred {

components/layout_2020/flow/root.rs

@@ -54,7 +54,7 @@ impl BoxTree {
         // Zero box for `:root { display: none }`, one for the root element otherwise.
         assert!(boxes.len() <= 1);
 
-        // From https://drafts.csswg.org/css-overflow/#overflow-propagation:
+        // From https://www.w3.org/TR/css-overflow-3/#overflow-propagation:
         // > UAs must apply the overflow-* values set on the root element to the viewport when the
         // > root element’s display value is not none. However, when the root element is an [HTML]
         // > html element (including XML syntax for HTML) whose overflow value is visible (in both
@@ -63,10 +63,11 @@ impl BoxTree {
         // > element to the viewport. The element from which the value is propagated must then have a
         // > used overflow value of visible.
         let root_style = root_element.style(context);
-        let root_overflow = root_style.effective_overflow();
+
-        let mut viewport_overflow = root_overflow;
+        let mut viewport_overflow_x = root_style.clone_overflow_x();
-        if root_overflow.x == Overflow::Visible &&
+        let mut viewport_overflow_y = root_style.clone_overflow_y();
-            root_overflow.y == Overflow::Visible &&
+        if viewport_overflow_x == Overflow::Visible &&
+            viewport_overflow_y == Overflow::Visible &&
             !root_style.get_box().display.is_none()
         {
             for child in iter_child_nodes(root_element) {
@@ -80,7 +81,9 @@ impl BoxTree {
 
                 let style = child.style(context);
                 if !style.get_box().display.is_none() {
-                    viewport_overflow = style.effective_overflow();
+                    viewport_overflow_x = style.clone_overflow_x();
+                    viewport_overflow_y = style.clone_overflow_y();
+
                     break;
                 }
             }
@@ -94,9 +97,12 @@ impl BoxTree {
                 contains_floats,
             },
             canvas_background: CanvasBackground::for_root_element(context, root_element),
+            // From https://www.w3.org/TR/css-overflow-3/#overflow-propagation:
+            // > If visible is applied to the viewport, it must be interpreted as auto.
+            // > If clip is applied to the viewport, it must be interpreted as hidden.
             viewport_scroll_sensitivity: AxesScrollSensitivity {
-                x: viewport_overflow.x.to_scrollable().into(),
+                x: viewport_overflow_x.to_scrollable().into(),
-                y: viewport_overflow.y.to_scrollable().into(),
+                y: viewport_overflow_y.to_scrollable().into(),
             },
         }
     }

components/layout_2020/fragment_tree/box_fragment.rs

@@ -154,7 +154,7 @@ impl BoxFragment {
         //
         // This applies even if there is no baseline set, so we unconditionally set the value here
         // and ignore anything that is set via [`Self::with_baselines`].
-        if self.style.establishes_scroll_container() {
+        if self.style.establishes_scroll_container(self.base.flags) {
             let content_rect_size = self.content_rect.size.to_logical(writing_mode);
             let padding = self.padding.to_logical(writing_mode);
             let border = self.border.to_logical(writing_mode);
@@ -228,7 +228,7 @@ impl BoxFragment {
             self.clearance,
             self.scrollable_overflow(),
             self.baselines,
-            self.style.effective_overflow(),
+            self.style.effective_overflow(self.base.flags),
         ));
 
         for child in &self.children {
@@ -239,7 +239,7 @@ impl BoxFragment {
 
     pub fn scrollable_overflow_for_parent(&self) -> PhysicalRect<Au> {
         let mut overflow = self.border_rect();
-        if self.style.establishes_scroll_container() {
+        if self.style.establishes_scroll_container(self.base.flags) {
             return overflow;
         }
 
@@ -251,7 +251,7 @@ impl BoxFragment {
             overflow.max_y().max(scrollable_overflow.max_y()),
         );
 
-        let overflow_style = self.style.effective_overflow();
+        let overflow_style = self.style.effective_overflow(self.base.flags);
         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;

components/layout_2020/style_ext.rs

@@ -285,10 +285,10 @@ pub(crate) trait ComputedValuesExt {
     ) -> LogicalSides<LengthPercentageOrAuto<'_>>;
     fn has_transform_or_perspective(&self, fragment_flags: FragmentFlags) -> bool;
     fn effective_z_index(&self, fragment_flags: FragmentFlags) -> i32;
-    fn effective_overflow(&self) -> AxesOverflow;
+    fn effective_overflow(&self, fragment_flags: FragmentFlags) -> AxesOverflow;
-    fn establishes_block_formatting_context(&self) -> bool;
+    fn establishes_block_formatting_context(&self, fragment_flags: FragmentFlags) -> bool;
     fn establishes_stacking_context(&self, fragment_flags: FragmentFlags) -> bool;
-    fn establishes_scroll_container(&self) -> bool;
+    fn establishes_scroll_container(&self, fragment_flags: FragmentFlags) -> bool;
     fn establishes_containing_block_for_absolute_descendants(
         &self,
         fragment_flags: FragmentFlags,
@@ -501,10 +501,26 @@ 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.
     /// <https://www.w3.org/TR/css-overflow-3/#overflow-control>
-    fn effective_overflow(&self) -> AxesOverflow {
+    fn effective_overflow(&self, fragment_flags: FragmentFlags) -> AxesOverflow {
         let style_box = self.get_box();
-        let overflow_x = style_box.overflow_x;
+        let mut overflow_x = style_box.overflow_x;
-        let overflow_y = style_box.overflow_y;
+        let mut overflow_y = style_box.overflow_y;
+
+        // From <https://www.w3.org/TR/css-overflow-4/#overflow-control>:
+        // "On replaced elements, the used values of all computed values other than visible is clip."
+        if fragment_flags.contains(FragmentFlags::IS_REPLACED) {
+            if overflow_x != Overflow::Visible {
+                overflow_x = Overflow::Clip;
+            }
+            if overflow_y != Overflow::Visible {
+                overflow_y = Overflow::Clip;
+            }
+            return AxesOverflow {
+                x: overflow_x,
+                y: overflow_y,
+            };
+        }
+
         let ignores_overflow = match style_box.display.inside() {
             stylo::DisplayInside::Table => {
                 // According to <https://drafts.csswg.org/css-tables/#global-style-overrides>,
@@ -530,6 +546,7 @@ impl ComputedValuesExt for ComputedValues {
             },
             _ => false,
         };
+
         if ignores_overflow {
             AxesOverflow {
                 x: Overflow::Visible,
@@ -545,8 +562,8 @@ impl ComputedValuesExt for ComputedValues {
 
     /// Return true if this style is a normal block and establishes
     /// a new block formatting context.
-    fn establishes_block_formatting_context(&self) -> bool {
+    fn establishes_block_formatting_context(&self, fragment_flags: FragmentFlags) -> bool {
-        if self.establishes_scroll_container() {
+        if self.establishes_scroll_container(fragment_flags) {
             return true;
         }
 
@@ -572,10 +589,10 @@ impl ComputedValuesExt for ComputedValues {
     }
 
     /// Whether or not the `overflow` value of this style establishes a scroll container.
-    fn establishes_scroll_container(&self) -> bool {
+    fn establishes_scroll_container(&self, fragment_flags: FragmentFlags) -> bool {
         // Checking one axis suffices, because the computed value ensures that
         // either both axes are scrollable, or none is scrollable.
-        self.effective_overflow().x.is_scrollable()
+        self.effective_overflow(fragment_flags).x.is_scrollable()
     }
 
     /// Returns true if this fragment establishes a new stacking context and false otherwise.

tests/wpt/meta/css/css-overflow/overflow-img-display-table.html.ini

@@ -1,2 +0,0 @@
-[overflow-img-display-table.html]
-  expected: FAIL