layout: Check if root before establishing containing block (#36360)

As per
[w3.org/TR/filter-effects-1#FilterProperty](https://www.w3.org/TR/filter-effects-1/#FilterProperty),
`filter` shouldn't make the root element establish a containing block
for absolute and fixed positioned descendants. `will-change: filter` has
matching behavior.

This PR adds a check for if we are the root element before establishing
such a block.

To know if we are the root element, we look at the `FragmentFlags`
passed in. Previously for our function, these were dummy flags, always
constructed as empty. Thus, this PR also makes sure the correct
FragmentFlags are passed down the chain to the function
`establishes_containing_block_for_all_descendants`.

Testing:
- `/css/filter-effects/filtered-html-is-not-container.html` now passes
- `/css/css-will-change/will-change-fixedpos-cb-003.html` now passes
- Manual tests are working

Fixes: #35391

---------

Signed-off-by: haval0 <56519858+haval0@users.noreply.github.com>
Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
haval0 2025-04-29 21:19:31 +02:00 committed by GitHub
parent bab788f5d5
commit ec88b5d752
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 44 additions and 35 deletions

View file

@ -1774,7 +1774,9 @@ impl FlexItem<'_> {
non_stretch_layout_result: Option<&mut FlexItemLayoutResult>, non_stretch_layout_result: Option<&mut FlexItemLayoutResult>,
) -> Option<FlexItemLayoutResult> { ) -> Option<FlexItemLayoutResult> {
let containing_block = flex_context.containing_block; let containing_block = flex_context.containing_block;
let mut positioning_context = PositioningContext::new_for_style(self.box_.style()) let independent_formatting_context = &self.box_.independent_formatting_context;
let mut positioning_context = independent_formatting_context
.new_positioning_context()
.unwrap_or_else(|| { .unwrap_or_else(|| {
PositioningContext::new_for_subtree( PositioningContext::new_for_subtree(
flex_context flex_context
@ -1783,7 +1785,6 @@ impl FlexItem<'_> {
) )
}); });
let independent_formatting_context = &self.box_.independent_formatting_context;
let item_writing_mode = independent_formatting_context.style().writing_mode; let item_writing_mode = independent_formatting_context.style().writing_mode;
let item_is_horizontal = item_writing_mode.is_horizontal(); let item_is_horizontal = item_writing_mode.is_horizontal();
let flex_axis = flex_context.config.flex_axis; let flex_axis = flex_context.config.flex_axis;
@ -2616,7 +2617,9 @@ impl FlexItemBox {
cross_size_stretches_to_container_size: bool, cross_size_stretches_to_container_size: bool,
intrinsic_sizing_mode: IntrinsicSizingMode, intrinsic_sizing_mode: IntrinsicSizingMode,
) -> Au { ) -> Au {
let mut positioning_context = PositioningContext::new_for_style(self.style()) let mut positioning_context = self
.independent_formatting_context
.new_positioning_context()
.unwrap_or_else(|| { .unwrap_or_else(|| {
PositioningContext::new_for_subtree( PositioningContext::new_for_subtree(
flex_context flex_context

View file

@ -913,11 +913,10 @@ impl FloatBox {
positioning_context: &mut PositioningContext, positioning_context: &mut PositioningContext,
containing_block: &ContainingBlock, containing_block: &ContainingBlock,
) -> BoxFragment { ) -> BoxFragment {
let style = self.contents.style().clone();
positioning_context.layout_maybe_position_relative_fragment( positioning_context.layout_maybe_position_relative_fragment(
layout_context, layout_context,
containing_block, containing_block,
&style, &self.contents.base,
|positioning_context| { |positioning_context| {
self.contents self.contents
.layout_float_or_atomic_inline( .layout_float_or_atomic_inline(

View file

@ -326,13 +326,12 @@ impl LineItemLayout<'_, '_> {
let inline_box = self.layout.ifc.inline_boxes.get(identifier); let inline_box = self.layout.ifc.inline_boxes.get(identifier);
let inline_box = &*(inline_box.borrow()); let inline_box = &*(inline_box.borrow());
let style = &inline_box.base.style;
let space_above_baseline = inline_box_state.calculate_space_above_baseline(); let space_above_baseline = inline_box_state.calculate_space_above_baseline();
let block_start_offset = let block_start_offset =
self.calculate_inline_box_block_start(inline_box_state, space_above_baseline); self.calculate_inline_box_block_start(inline_box_state, space_above_baseline);
let positioning_context_or_start_offset_in_parent = let positioning_context_or_start_offset_in_parent =
match PositioningContext::new_for_style(style) { match inline_box.base.new_positioning_context() {
Some(positioning_context) => Either::Left(positioning_context), Some(positioning_context) => Either::Left(positioning_context),
None => Either::Right(self.current_positioning_context_mut().len()), None => Either::Right(self.current_positioning_context_mut().len()),
}; };

View file

@ -2004,7 +2004,8 @@ impl IndependentFormattingContext {
bidi_level: Level, bidi_level: Level,
) { ) {
// We need to know the inline size of the atomic before deciding whether to do the line break. // We need to know the inline size of the atomic before deciding whether to do the line break.
let mut child_positioning_context = PositioningContext::new_for_style(self.style()) let mut child_positioning_context = self
.new_positioning_context()
.unwrap_or_else(|| PositioningContext::new_for_subtree(true)); .unwrap_or_else(|| PositioningContext::new_for_subtree(true));
let IndependentFloatOrAtomicLayoutResult { let IndependentFloatOrAtomicLayoutResult {
mut fragment, mut fragment,

View file

@ -779,7 +779,7 @@ impl BlockLevelBox {
ArcRefCell::new(positioning_context.layout_maybe_position_relative_fragment( ArcRefCell::new(positioning_context.layout_maybe_position_relative_fragment(
layout_context, layout_context,
containing_block, containing_block,
&base.style, base,
|positioning_context| { |positioning_context| {
layout_in_flow_non_replaced_block_level_same_formatting_context( layout_in_flow_non_replaced_block_level_same_formatting_context(
layout_context, layout_context,
@ -798,7 +798,7 @@ impl BlockLevelBox {
positioning_context.layout_maybe_position_relative_fragment( positioning_context.layout_maybe_position_relative_fragment(
layout_context, layout_context,
containing_block, containing_block,
independent.style(), &independent.base,
|positioning_context| { |positioning_context| {
independent.layout_in_flow_block_level( independent.layout_in_flow_block_level(
layout_context, layout_context,

View file

@ -29,6 +29,7 @@ use crate::geom::{
PhysicalPoint, PhysicalRect, PhysicalSides, PhysicalSize, PhysicalVec, Size, Sizes, ToLogical, PhysicalPoint, PhysicalRect, PhysicalSides, PhysicalSize, PhysicalVec, Size, Sizes, ToLogical,
ToLogicalWithContainingBlock, ToLogicalWithContainingBlock,
}; };
use crate::layout_box_base::LayoutBoxBase;
use crate::sizing::ContentSizes; use crate::sizing::ContentSizes;
use crate::style_ext::{Clamp, ComputedValuesExt, ContentBoxSizesAndPBM, DisplayInside}; use crate::style_ext::{Clamp, ComputedValuesExt, ContentBoxSizesAndPBM, DisplayInside};
use crate::{ use crate::{
@ -103,6 +104,20 @@ impl AbsolutelyPositionedBox {
} }
} }
impl IndependentFormattingContext {
#[inline]
pub(crate) fn new_positioning_context(&self) -> Option<PositioningContext> {
self.base.new_positioning_context()
}
}
impl LayoutBoxBase {
#[inline]
pub(crate) fn new_positioning_context(&self) -> Option<PositioningContext> {
PositioningContext::new_for_style(&self.style, &self.base_fragment_info.flags)
}
}
impl PositioningContext { impl PositioningContext {
pub(crate) fn new_for_containing_block_for_all_descendants() -> Self { pub(crate) fn new_for_containing_block_for_all_descendants() -> Self {
Self { Self {
@ -130,14 +145,10 @@ impl PositioningContext {
self.for_nearest_positioned_ancestor.is_some() self.for_nearest_positioned_ancestor.is_some()
} }
pub(crate) fn new_for_style(style: &ComputedValues) -> Option<Self> { fn new_for_style(style: &ComputedValues, flags: &FragmentFlags) -> Option<Self> {
// NB: We never make PositioningContexts for replaced elements, which is why we always if style.establishes_containing_block_for_all_descendants(*flags) {
// pass false here.
if style.establishes_containing_block_for_all_descendants(FragmentFlags::empty()) {
Some(Self::new_for_containing_block_for_all_descendants()) Some(Self::new_for_containing_block_for_all_descendants())
} else if style } else if style.establishes_containing_block_for_absolute_descendants(*flags) {
.establishes_containing_block_for_absolute_descendants(FragmentFlags::empty())
{
Some(Self { Some(Self {
for_nearest_positioned_ancestor: Some(Vec::new()), for_nearest_positioned_ancestor: Some(Vec::new()),
for_nearest_containing_block_for_all_descendants: Vec::new(), for_nearest_containing_block_for_all_descendants: Vec::new(),
@ -213,12 +224,12 @@ impl PositioningContext {
&mut self, &mut self,
layout_context: &LayoutContext, layout_context: &LayoutContext,
containing_block: &ContainingBlock, containing_block: &ContainingBlock,
style: &ComputedValues, base: &LayoutBoxBase,
fragment_layout_fn: impl FnOnce(&mut Self) -> BoxFragment, fragment_layout_fn: impl FnOnce(&mut Self) -> BoxFragment,
) -> BoxFragment { ) -> BoxFragment {
// Try to create a context, but if one isn't necessary, simply create the fragment // Try to create a context, but if one isn't necessary, simply create the fragment
// using the given closure and the current `PositioningContext`. // using the given closure and the current `PositioningContext`.
let mut new_context = match Self::new_for_style(style) { let mut new_context = match base.new_positioning_context() {
Some(new_context) => new_context, Some(new_context) => new_context,
None => return fragment_layout_fn(self), None => return fragment_layout_fn(self),
}; };
@ -229,9 +240,8 @@ impl PositioningContext {
// If the new context has any hoisted boxes for the nearest containing block for // If the new context has any hoisted boxes for the nearest containing block for
// pass them up the tree. // pass them up the tree.
self.append(new_context); self.append(new_context);
if base.style.clone_position() == Position::Relative {
if style.clone_position() == Position::Relative { new_fragment.content_rect.origin += relative_adjustement(&base.style, containing_block)
new_fragment.content_rect.origin += relative_adjustement(style, containing_block)
.to_physical_vector(containing_block.style.writing_mode) .to_physical_vector(containing_block.style.writing_mode)
} }
@ -586,7 +596,7 @@ impl HoistedAbsolutelyPositionedBox {
.sizes .sizes
})); }));
let mut positioning_context = PositioningContext::new_for_style(&style).unwrap(); let mut positioning_context = context.new_positioning_context().unwrap();
let mut new_fragment = { let mut new_fragment = {
let content_size: LogicalVec2<Au>; let content_size: LogicalVec2<Au>;
let fragments; let fragments;

View file

@ -845,9 +845,9 @@ impl ComputedValuesExt for ComputedValues {
// > A value other than none for the filter property results in the creation of a containing // > A value other than none for the filter property results in the creation of a containing
// > block for absolute and fixed positioned descendants unless the element it applies to is // > block for absolute and fixed positioned descendants unless the element it applies to is
// > a document root element in the current browsing context. // > a document root element in the current browsing context.
// FIXME(#35391): Need to check if this is the root element. if !fragment_flags.contains(FragmentFlags::IS_ROOT_ELEMENT) &&
if !self.get_effects().filter.0.is_empty() || (!self.get_effects().filter.0.is_empty() ||
will_change_bits.intersects(WillChangeBits::FIXPOS_CB_NON_SVG) will_change_bits.intersects(WillChangeBits::FIXPOS_CB_NON_SVG))
{ {
return true; return true;
} }

View file

@ -1503,7 +1503,7 @@ impl<'a> TableLayout<'a> {
layout_context: &LayoutContext, layout_context: &LayoutContext,
parent_positioning_context: &mut PositioningContext, parent_positioning_context: &mut PositioningContext,
) -> BoxFragment { ) -> BoxFragment {
let mut positioning_context = PositioningContext::new_for_style(caption.context.style()); let mut positioning_context = caption.context.new_positioning_context();
let containing_block = &ContainingBlock { let containing_block = &ContainingBlock {
size: ContainingBlockSize { size: ContainingBlockSize {
inline: self.table_width + self.pbm.padding_border_sums.inline, inline: self.table_width + self.pbm.padding_border_sums.inline,
@ -2325,7 +2325,7 @@ impl<'a> RowFragmentLayout<'a> {
Self { Self {
row: table_row, row: table_row,
rect, rect,
positioning_context: PositioningContext::new_for_style(&table_row.base.style), positioning_context: table_row.base.new_positioning_context(),
containing_block, containing_block,
fragments: Vec::new(), fragments: Vec::new(),
} }
@ -2410,7 +2410,7 @@ impl RowGroupFragmentLayout {
let row_group = row_group.borrow(); let row_group = row_group.borrow();
( (
dimensions.get_row_group_rect(&row_group), dimensions.get_row_group_rect(&row_group),
PositioningContext::new_for_style(&row_group.base.style), row_group.base.new_positioning_context(),
) )
}; };
Self { Self {

View file

@ -251,8 +251,9 @@ impl taffy::LayoutPartialTree for TaffyContainerContext<'_> {
style, style,
}; };
let layout = { let layout = {
let mut child_positioning_context = let mut child_positioning_context = independent_context
PositioningContext::new_for_style(style).unwrap_or_else(|| { .new_positioning_context()
.unwrap_or_else(|| {
PositioningContext::new_for_subtree( PositioningContext::new_for_subtree(
self.positioning_context self.positioning_context
.collects_for_nearest_positioned_ancestor(), .collects_for_nearest_positioned_ancestor(),

View file

@ -1,2 +0,0 @@
[will-change-fixedpos-cb-003.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[filtered-html-is-not-container.html]
expected: FAIL