From c86bc2eb8b69b6a4d8c80aca6b36a7dc041eddd9 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 3 Apr 2015 17:18:22 -0700 Subject: [PATCH] layout: Don't take collapsible top margin into account when determining the float ceiling. As the float ceiling is relative to the border box, not the margin box, of the parent flow, top margin must not be included. This exposed a pre-existing bug whereby margins are discarded if a block contains only floats and no content, affecting the tests `float_intrinsic_height.html` and `margins_inside_floats_a.html`. As a workaround, some invisible content has been added to the bodies of both tests. --- components/layout/block.rs | 3 ++- components/layout/model.rs | 19 +++++++++++---- tests/ref/basic.list | 1 + tests/ref/float_intrinsic_height.html | 2 +- tests/ref/float_under_top_margin_a.html | 29 +++++++++++++++++++++++ tests/ref/float_under_top_margin_ref.html | 29 +++++++++++++++++++++++ tests/ref/margins_inside_floats_a.html | 2 +- 7 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 tests/ref/float_under_top_margin_a.html create mode 100644 tests/ref/float_under_top_margin_ref.html diff --git a/components/layout/block.rs b/components/layout/block.rs index 24eac4f48ea..236bfd321ea 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -1990,7 +1990,8 @@ pub struct ISizeConstraintSolution { } impl ISizeConstraintSolution { - pub fn new(inline_size: Au, margin_inline_start: Au, margin_inline_end: Au) -> ISizeConstraintSolution { + pub fn new(inline_size: Au, margin_inline_start: Au, margin_inline_end: Au) + -> ISizeConstraintSolution { ISizeConstraintSolution { inline_start: Au(0), inline_end: Au(0), diff --git a/components/layout/model.rs b/components/layout/model.rs index 64ba9a001d8..2479ab7cb47 100644 --- a/components/layout/model.rs +++ b/components/layout/model.rs @@ -174,7 +174,12 @@ impl MarginCollapseInfo { pub fn current_float_ceiling(&mut self) -> Au { match self.state { - MarginCollapseState::AccumulatingCollapsibleTopMargin => self.block_start_margin.collapse(), + MarginCollapseState::AccumulatingCollapsibleTopMargin => { + // We do not include the top margin in the float ceiling, because the float flow + // needs to be positioned relative to our *border box*, not our margin box. See + // `tests/ref/float_under_top_margin_a.html`. + Au(0) + } MarginCollapseState::AccumulatingMarginIn => self.margin_in.collapse(), } } @@ -182,18 +187,22 @@ impl MarginCollapseInfo { /// Adds the child's potentially collapsible block-start margin to the current margin state and /// advances the Y offset by the appropriate amount to handle that margin. Returns the amount /// that should be added to the Y offset during block layout. - pub fn advance_block_start_margin(&mut self, child_collapsible_margins: &CollapsibleMargins) -> Au { + pub fn advance_block_start_margin(&mut self, child_collapsible_margins: &CollapsibleMargins) + -> Au { match (self.state, *child_collapsible_margins) { - (MarginCollapseState::AccumulatingCollapsibleTopMargin, CollapsibleMargins::None(block_start, _)) => { + (MarginCollapseState::AccumulatingCollapsibleTopMargin, + CollapsibleMargins::None(block_start, _)) => { self.state = MarginCollapseState::AccumulatingMarginIn; block_start } - (MarginCollapseState::AccumulatingCollapsibleTopMargin, CollapsibleMargins::Collapse(block_start, _)) => { + (MarginCollapseState::AccumulatingCollapsibleTopMargin, + CollapsibleMargins::Collapse(block_start, _)) => { self.block_start_margin.union(block_start); self.state = MarginCollapseState::AccumulatingMarginIn; Au(0) } - (MarginCollapseState::AccumulatingMarginIn, CollapsibleMargins::None(block_start, _)) => { + (MarginCollapseState::AccumulatingMarginIn, + CollapsibleMargins::None(block_start, _)) => { let previous_margin_value = self.margin_in.collapse(); self.margin_in = AdjoiningMargins::new(); previous_margin_value + block_start diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 1d814a10e87..f7e535872ef 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -100,6 +100,7 @@ flaky_cpu == append_style_a.html append_style_b.html == float_intrinsic_width_a.html float_intrinsic_width_ref.html == float_right_intrinsic_width_a.html float_right_intrinsic_width_ref.html == float_table_a.html float_table_ref.html +== float_under_top_margin_a.html float_under_top_margin_ref.html == floated_generated_content_a.html floated_generated_content_b.html == floated_list_item_a.html floated_list_item_ref.html == floated_table_with_margin_a.html floated_table_with_margin_ref.html diff --git a/tests/ref/float_intrinsic_height.html b/tests/ref/float_intrinsic_height.html index b6d9e7e302a..f349085d23c 100644 --- a/tests/ref/float_intrinsic_height.html +++ b/tests/ref/float_intrinsic_height.html @@ -19,6 +19,6 @@
-
+   diff --git a/tests/ref/float_under_top_margin_a.html b/tests/ref/float_under_top_margin_a.html new file mode 100644 index 00000000000..5cf1fdbdbd4 --- /dev/null +++ b/tests/ref/float_under_top_margin_a.html @@ -0,0 +1,29 @@ + + + + + + +
+ + + diff --git a/tests/ref/float_under_top_margin_ref.html b/tests/ref/float_under_top_margin_ref.html new file mode 100644 index 00000000000..904cbd5c66d --- /dev/null +++ b/tests/ref/float_under_top_margin_ref.html @@ -0,0 +1,29 @@ + + + + + + +
+ + + diff --git a/tests/ref/margins_inside_floats_a.html b/tests/ref/margins_inside_floats_a.html index 139f12677df..9ad6aa576ed 100644 --- a/tests/ref/margins_inside_floats_a.html +++ b/tests/ref/margins_inside_floats_a.html @@ -4,7 +4,7 @@
Must be this tall
to write multi-threaded code.
-