From c2118ff4b1b450eea36ee1dfeef60fd0ed619842 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 15 Apr 2016 05:31:28 +0501 Subject: [PATCH] =?UTF-8?q?servo:=20Merge=20#10458=20-=20layout:=20Disallo?= =?UTF-8?q?w=20margins=20from=20collapsing=20through=20block=20formatting?= =?UTF-8?q?=20contexts=20per=20CSS=202.1=20=C2=A7=208.3.1=20(from=20pcwalt?= =?UTF-8?q?on:block-formatting-context-margin-collapse);=20r=3Dmbrubeck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #10449. r? @mbrubeck Source-Repo: https://github.com/servo/servo Source-Revision: 7d7404333d75cfd1aab3df07597304d5e78617b5 --- servo/components/layout/block.rs | 21 ++++++++++---- servo/components/layout/floats.rs | 4 +++ servo/components/layout/model.rs | 47 ++++++++++++++++++++----------- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/servo/components/layout/block.rs b/servo/components/layout/block.rs index 763698d85a43..fa91b847b202 100644 --- a/servo/components/layout/block.rs +++ b/servo/components/layout/block.rs @@ -809,6 +809,7 @@ impl BlockFlow { // At this point, `cur_b` is at the content edge of our box. Now iterate over children. let mut floats = self.base.floats.clone(); let thread_id = self.base.thread_id; + let (mut had_floated_children, mut had_children_with_clearance) = (false, false); for (child_index, kid) in self.base.child_iter_mut().enumerate() { if flow::base(kid).flags.contains(IS_ABSOLUTELY_POSITIONED) { // Assume that the *hypothetical box* for an absolute flow starts immediately @@ -844,11 +845,12 @@ impl BlockFlow { // before. flow::mut_base(kid).floats = floats.clone(); if flow::base(kid).flags.is_float() { + had_floated_children = true; flow::mut_base(kid).position.start.b = cur_b; { let kid_block = kid.as_mut_block(); - kid_block.float.as_mut().unwrap().float_ceiling = - margin_collapse_info.current_float_ceiling(); + let float_ceiling = margin_collapse_info.current_float_ceiling(); + kid_block.float.as_mut().unwrap().float_ceiling = float_ceiling } kid.place_float_if_applicable(layout_context); @@ -873,9 +875,17 @@ impl BlockFlow { kid.assign_block_size_for_inorder_child_if_necessary(layout_context, thread_id); + if !had_children_with_clearance && + floats.is_present() && + (flow::base(kid).flags.contains(CLEARS_LEFT) || + flow::base(kid).flags.contains(CLEARS_RIGHT)) { + had_children_with_clearance = true + } + // Handle any (possibly collapsed) top margin. let delta = margin_collapse_info.advance_block_start_margin( - &flow::base(kid).collapsible_margins); + &flow::base(kid).collapsible_margins, + !had_children_with_clearance); translate_including_floats(&mut cur_b, delta, &mut floats); // Clear past the floats that came in, if necessary. @@ -932,7 +942,8 @@ impl BlockFlow { margin_collapse_info.finish_and_compute_collapsible_margins( &self.fragment, self.base.block_container_explicit_block_size, - can_collapse_block_end_margin_with_kids); + can_collapse_block_end_margin_with_kids, + !had_floated_children); self.base.collapsible_margins = collapsible_margins; translate_including_floats(&mut cur_b, delta, &mut floats); @@ -1740,7 +1751,7 @@ impl Flow for BlockFlow { self.base.position.size.block = self.fragment.border_box.size.block; } None - } else if self.is_root() || self.base.flags.is_float() || self.is_inline_block() { + } else if self.is_root() || self.formatting_context_type() != FormattingContextType::None { // Root element margins should never be collapsed according to CSS § 8.3.1. debug!("assign_block_size: assigning block_size for root flow {:?}", flow::base(self).debug_id()); diff --git a/servo/components/layout/floats.rs b/servo/components/layout/floats.rs index a31d7461db01..63fdc6da7969 100644 --- a/servo/components/layout/floats.rs +++ b/servo/components/layout/floats.rs @@ -413,6 +413,10 @@ impl Floats { } clearance } + + pub fn is_present(&self) -> bool { + self.list.is_present() + } } /// The speculated inline sizes of floats flowing through or around a flow (depending on whether diff --git a/servo/components/layout/model.rs b/servo/components/layout/model.rs index 8b18a1f02ada..ac592bb4139a 100644 --- a/servo/components/layout/model.rs +++ b/servo/components/layout/model.rs @@ -18,7 +18,7 @@ use style::values::computed::{BorderRadiusSize, LengthOrPercentageOrAuto}; use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrNone}; /// A collapsible margin. See CSS 2.1 § 8.3.1. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub struct AdjoiningMargins { /// The value of the greatest positive margin. pub most_positive: Au, @@ -61,7 +61,7 @@ impl AdjoiningMargins { } /// Represents the block-start and block-end margins of a flow with collapsible margins. See CSS 2.1 § 8.3.1. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub enum CollapsibleMargins { /// Margins may not collapse with this flow. None(Au, Au), @@ -126,17 +126,20 @@ impl MarginCollapseInfo { pub fn finish_and_compute_collapsible_margins(mut self, fragment: &Fragment, containing_block_size: Option, - can_collapse_block_end_margin_with_kids: bool) + can_collapse_block_end_margin_with_kids: bool, + mut may_collapse_through: bool) -> (CollapsibleMargins, Au) { let state = match self.state { MarginCollapseState::AccumulatingCollapsibleTopMargin => { - let may_collapse_through = match fragment.style().content_block_size() { - LengthOrPercentageOrAuto::Auto => true, - LengthOrPercentageOrAuto::Length(Au(0)) => true, - LengthOrPercentageOrAuto::Percentage(0.) => true, - LengthOrPercentageOrAuto::Percentage(_) if containing_block_size.is_none() => true, - _ => false, - }; + may_collapse_through = may_collapse_through && + match fragment.style().content_block_size() { + LengthOrPercentageOrAuto::Auto => true, + LengthOrPercentageOrAuto::Length(Au(0)) => true, + LengthOrPercentageOrAuto::Percentage(0.) => true, + LengthOrPercentageOrAuto::Percentage(_) if + containing_block_size.is_none() => true, + _ => false, + }; if may_collapse_through { match fragment.style().min_block_size() { @@ -166,12 +169,14 @@ impl MarginCollapseInfo { FinalMarginState::MarginsCollapseThrough => { let advance = self.block_start_margin.collapse(); self.margin_in.union(AdjoiningMargins::from_margin(block_end_margin)); - (CollapsibleMargins::Collapse(self.block_start_margin, self.margin_in), advance) + (CollapsibleMargins::Collapse(self.block_start_margin, self.margin_in), + advance) } FinalMarginState::BottomMarginCollapses => { let advance = self.margin_in.collapse(); self.margin_in.union(AdjoiningMargins::from_margin(block_end_margin)); - (CollapsibleMargins::Collapse(self.block_start_margin, self.margin_in), advance) + (CollapsibleMargins::Collapse(self.block_start_margin, self.margin_in), + advance) } } } else { @@ -203,8 +208,14 @@ 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) + pub fn advance_block_start_margin(&mut self, + child_collapsible_margins: &CollapsibleMargins, + can_collapse_block_start_margin: bool) -> Au { + if !can_collapse_block_start_margin { + self.state = MarginCollapseState::AccumulatingMarginIn + } + match (self.state, *child_collapsible_margins) { (MarginCollapseState::AccumulatingCollapsibleTopMargin, CollapsibleMargins::None(block_start, _)) => { @@ -223,14 +234,16 @@ impl MarginCollapseInfo { self.margin_in = AdjoiningMargins::new(); previous_margin_value + block_start } - (MarginCollapseState::AccumulatingMarginIn, CollapsibleMargins::Collapse(block_start, _)) => { + (MarginCollapseState::AccumulatingMarginIn, + CollapsibleMargins::Collapse(block_start, _)) => { self.margin_in.union(block_start); let margin_value = self.margin_in.collapse(); self.margin_in = AdjoiningMargins::new(); margin_value } (_, CollapsibleMargins::CollapseThrough(_)) => { - // For now, we ignore this; this will be handled by `advance_block-end_margin` below. + // For now, we ignore this; this will be handled by `advance_block_end_margin` + // below. Au(0) } } @@ -271,9 +284,11 @@ impl MarginCollapseInfo { } } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub enum MarginCollapseState { + /// We are accumulating margin on the logical top of this flow. AccumulatingCollapsibleTopMargin, + /// We are accumulating margin between two blocks. AccumulatingMarginIn, }