Skip to content

Commit 7d8122b

Browse files
committed
Check computed (min-)block-size when determining margin collapse
To collapse margins through, CSS2 requires "zero or auto computed height" and "zero computed min-height" (the latter should also also include auto, which was introduced in CSS3 as the new initial value). Similarly, "auto computed height" is required to collapse the bottom margin with the bottom margin of the contents. Therefore this patch stops collapsing when the used height is clamped to zero by max-height, but the computed value is not zero. Same for non-zero percentages that are indefinite or that resolve to 0px. Note that 0% and calc(0% + 0px) are still considered to be zero (so they allow margin collapse), this may a bit inconsistent but matches Firefox (for the collapsing through case). This also matches the heuristics in find_block_margin_collapsing_with_parent(). We could change the heuristics instead, but then they would have to track block sizes in order to be able to resolve percentages. The change makes margin-collapse-through-percentage-height-block.html fail, that test is only passing on Blink, which did a different interpretation of the spec. To be fair, various places of CSS2 loosely consider that indefinite percentages compute to auto, and even Firefox does so when determining whether to collapse the top margin with the contents. Since the spec isn't particularly clear and interoperability is lacking, I filed w3c/csswg-drafts#8919.
1 parent fe5b494 commit 7d8122b

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

components/layout_2020/flow/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -644,13 +644,14 @@ fn layout_in_flow_non_replaced_block_level(
644644
NonReplacedContents::EstablishesAnIndependentFormattingContext(_) => false,
645645
};
646646

647+
let computed_block_size = style.content_block_size();
647648
let start_margin_can_collapse_with_children = block_is_same_formatting_context &&
648649
pbm.padding.block_start == Length::zero() &&
649650
pbm.border.block_start == Length::zero();
650651
let end_margin_can_collapse_with_children = block_is_same_formatting_context &&
651652
pbm.padding.block_end == Length::zero() &&
652653
pbm.border.block_end == Length::zero() &&
653-
block_size == LengthOrAuto::Auto;
654+
computed_block_size.is_auto();
654655

655656
let mut clearance = None;
656657
let parent_containing_block_position_info;
@@ -751,12 +752,13 @@ fn layout_in_flow_non_replaced_block_level(
751752
} else {
752753
content_block_size += collapsible_margins_in_children.end.solve();
753754
}
755+
let computed_min_block_size = style.min_block_size();
754756
block_margins_collapsed_with_children.collapsed_through =
755757
collapsible_margins_in_children.collapsed_through &&
756-
block_is_same_formatting_context &&
757758
pbm.padding_border_sums.block == Length::zero() &&
758-
block_size.auto_is(|| Length::zero()) == Length::zero() &&
759-
min_box_size.block == Length::zero();
759+
(computed_block_size.is_definitely_zero() || computed_block_size.is_auto()) &&
760+
(computed_min_block_size.is_definitely_zero() ||
761+
computed_min_block_size.is_auto());
760762
},
761763
NonReplacedContents::EstablishesAnIndependentFormattingContext(non_replaced) => {
762764
let independent_layout = non_replaced.layout(
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[margin-collapse-through-percentage-height-block.html]
2+
expected: FAIL

0 commit comments

Comments
 (0)