From f724651e1a4bd52c486b2c7c9fa7e4e92371323a Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Tue, 30 Sep 2025 18:12:31 +0200 Subject: [PATCH] layout: Let min/max constraints avoid collapsing bottom margins (#36322) When `block-size` was intrinsic and there was no block-end padding or margin, we were allowing the block-end margin of the box to collapse with the block-end margin of its contents. However, due to `min-block-size` or `max-block-size`, the final size can be different than the intrinsic size of the contents. In that case it didn't make any sense to collapse end margins, only WebKit does it too. This patch takes the final size into account, like Gecko and Blink. This is under discussion in https://github.com/w3c/csswg-drafts/issues/12218, but for now we will match Blink. Testing: 2 tests are now passing. There is also 1 new failure, but that test only passes in WebKit and I don't agree with it. Fixes: #36321 Signed-off-by: Oriol Brufau --- components/layout/flow/mod.rs | 35 ++++++++++++++----- .../margin-collapse-038.xht.ini | 2 ++ .../max-height-separates-margin.html.ini | 2 -- .../min-height-separates-margin.html.ini | 2 -- 4 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 tests/wpt/meta/css/CSS2/margin-padding-clear/margin-collapse-038.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/normal-flow/max-height-separates-margin.html.ini delete mode 100644 tests/wpt/meta/css/CSS2/normal-flow/min-height-separates-margin.html.ini diff --git a/components/layout/flow/mod.rs b/components/layout/flow/mod.rs index ce28e25c144..8bc775f79ec 100644 --- a/components/layout/flow/mod.rs +++ b/components/layout/flow/mod.rs @@ -1091,15 +1091,9 @@ fn layout_in_flow_non_replaced_block_level_same_formatting_context( block_size_is_zero_or_intrinsic(style.min_block_size(), containing_block); block_margins_collapsed_with_children.collapsed_through = collapsed_through; - let end_margin_can_collapse_with_children = collapsed_through || - (pbm.padding.block_end.is_zero() && - pbm.border.block_end.is_zero() && - !containing_block_for_children.size.block.is_definite()); - if end_margin_can_collapse_with_children { - block_margins_collapsed_with_children - .end - .adjoin_assign(&collapsible_margins_in_children.end); - } else { + let end_margin_can_collapse_with_children = + pbm.padding.block_end.is_zero() && pbm.border.block_end.is_zero(); + if !end_margin_can_collapse_with_children { content_block_size += collapsible_margins_in_children.end.solve(); } @@ -1112,6 +1106,29 @@ fn layout_in_flow_non_replaced_block_level_same_formatting_context( false, /* is_table */ ); + // If the final block size is different than the intrinsic size of the contents, + // then we can't actually collapse the end margins. This can happen due to min + // or max block sizes, or due to `calc-size()` once we implement it. + // + // We also require `block-size` to have an intrinsic value, by checking whether + // the containing block established for the contents has an indefinite block size. + // However, even if `block-size: 0px` is extrinsic (so it would normally prevent + // collapsing the end margin with children), it doesn't prevent the top and end + // margins from collapsing through. If that happens, allow collapsing end margins. + // + // This is being discussed in https://github.com/w3c/csswg-drafts/issues/12218. + // It would probably make more sense to check the definiteness of the containing + // block in the logic above (when we check if there is some block-end padding or + // border), or maybe drop the condition altogether. But for now, we match Blink. + let end_margin_can_collapse_with_children = end_margin_can_collapse_with_children && + block_size == content_block_size && + (collapsed_through || !containing_block_for_children.size.block.is_definite()); + if end_margin_can_collapse_with_children { + block_margins_collapsed_with_children + .end + .adjoin_assign(&collapsible_margins_in_children.end); + } + if let Some(ref mut sequential_layout_state) = sequential_layout_state { // Now that we're done laying out our children, we can restore the // parent's containing block position information. diff --git a/tests/wpt/meta/css/CSS2/margin-padding-clear/margin-collapse-038.xht.ini b/tests/wpt/meta/css/CSS2/margin-padding-clear/margin-collapse-038.xht.ini new file mode 100644 index 00000000000..4add9bf88f0 --- /dev/null +++ b/tests/wpt/meta/css/CSS2/margin-padding-clear/margin-collapse-038.xht.ini @@ -0,0 +1,2 @@ +[margin-collapse-038.xht] + expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/normal-flow/max-height-separates-margin.html.ini b/tests/wpt/meta/css/CSS2/normal-flow/max-height-separates-margin.html.ini deleted file mode 100644 index a6cbf633e79..00000000000 --- a/tests/wpt/meta/css/CSS2/normal-flow/max-height-separates-margin.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[max-height-separates-margin.html] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/normal-flow/min-height-separates-margin.html.ini b/tests/wpt/meta/css/CSS2/normal-flow/min-height-separates-margin.html.ini deleted file mode 100644 index 4c4407f7f22..00000000000 --- a/tests/wpt/meta/css/CSS2/normal-flow/min-height-separates-margin.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[min-height-separates-margin.html] - expected: FAIL