diff --git a/components/layout_2020/flow/float.rs b/components/layout_2020/flow/float.rs index 75739dafd41..e00237a2838 100644 --- a/components/layout_2020/flow/float.rs +++ b/components/layout_2020/flow/float.rs @@ -905,15 +905,24 @@ impl SequentialLayoutState { self.current_margin = CollapsedMargin::zero(); } - /// Returns the amount of clearance (if any) that a block with the given `clear` value - /// needs to have at `current_block_position_including_margins()`. - /// `block_start_margin` is the top margin of the block, after collapsing (if possible) - /// with the margin of its contents. This must not be included in `current_margin`, - /// since adding clearance will prevent `current_margin` and `block_start_margin` - /// from collapsing together. - /// - /// https://www.w3.org/TR/2011/REC-CSS2-20110607/visuren.html#flow-control - pub(crate) fn calculate_clearance( + /// Computes the position of the block-start border edge of an element + /// with the provided `block_start_margin`, assuming no clearance. + fn position_without_clearance(&self, block_start_margin: &CollapsedMargin) -> Length { + // Adjoin `current_margin` and `block_start_margin` since there is no clearance. + self.bfc_relative_block_position + self.current_margin.adjoin(&block_start_margin).solve() + } + + /// Computes the position of the block-start border edge of an element + /// with the provided `block_start_margin`, assuming a clearance of 0px. + fn position_with_zero_clearance(&self, block_start_margin: &CollapsedMargin) -> Length { + // Clearance prevents `current_margin` and `block_start_margin` from being + // adjoining, so we need to solve them separately and then sum. + self.bfc_relative_block_position + self.current_margin.solve() + block_start_margin.solve() + } + + /// Returns the block-end outer edge of the lowest float that is to be cleared (if any) + /// by an element with the provided `clear` and `block_start_margin`. + fn calculate_clear_position( &self, clear: Clear, block_start_margin: &CollapsedMargin, @@ -924,8 +933,7 @@ impl SequentialLayoutState { // Calculate the hypothetical position where the element's top border edge // would have been if the element's `clear` property had been `none`. - let hypothetical_block_position = self.bfc_relative_block_position + - self.current_margin.adjoin(&block_start_margin).solve(); + let hypothetical_block_position = self.position_without_clearance(&block_start_margin); // Check if the hypothetical position is past the relevant floats, // in that case we don't need to add clearance. @@ -939,23 +947,60 @@ impl SequentialLayoutState { .max(self.floats.clear_right_position), }; if hypothetical_block_position >= clear_position { - return None; + None + } else { + Some(clear_position) } + } - // We need to add clearance between `current_margin` and `block_start_margin`, - // this prevents them from collapsing. - // Compute the position of the element's top border edge if we added - // a clearance of 0px. - let position_with_zero_clearance = self.bfc_relative_block_position + - self.current_margin.solve() + - block_start_margin.solve(); + /// Returns the amount of clearance (if any) that a block with the given `clear` value + /// needs to have at `current_block_position_including_margins()`. + /// `block_start_margin` is the top margin of the block, after collapsing (if possible) + /// with the margin of its contents. This must not be included in `current_margin`, + /// since adding clearance will prevent `current_margin` and `block_start_margin` + /// from collapsing together. + /// + /// + pub(crate) fn calculate_clearance( + &self, + clear: Clear, + block_start_margin: &CollapsedMargin, + ) -> Option { + return self + .calculate_clear_position(clear, &block_start_margin) + .map(|offset| offset - self.position_with_zero_clearance(&block_start_margin)); + } - // Set clearance to the amount necessary to place the border edge of the block - // even with the bottom outer edge of the lowest float that is to be cleared. - // Note that CSS2 also allows the alternative option of flooring this amount by - // `hypothetical_block_position - position_with_zero_clearance`, - // but other browser don't seem to do that. - Some(clear_position - position_with_zero_clearance) + /// A block that is replaced or establishes an independent formatting context can't overlap floats, + /// it has to be placed next to them, and may get some clearance if there isn't enough space. + /// Given such a block with the provided 'clear', 'block_start_margin' and 'object_size', + /// this method finds an area that is big enough and doesn't overlap floats. + /// It returns a tuple with: + /// - The clearance amount (if any), which includes both the effect of 'clear' + /// and the extra space to avoid floats. + /// - The inline offset from the containing block that we need to avoid floats. + pub(crate) fn calculate_clearance_and_inline_adjustment( + &self, + clear: Clear, + block_start_margin: &CollapsedMargin, + object_size: Vec2, + ) -> (Option, Length) { + // First compute the clear position required by the 'clear' property. + // The code below may then add extra clearance when the element can't fit + // next to floats not covered by 'clear'. + let clear_position = self.calculate_clear_position(clear, &block_start_margin); + let ceiling = + clear_position.unwrap_or_else(|| self.position_without_clearance(&block_start_margin)); + let mut placement = PlacementAmongFloats::new(&self.floats, ceiling, object_size); + let position = placement.place(); + let has_clearance = clear_position.is_some() || position.block > ceiling; + let clearance = if has_clearance { + Some(position.block - self.position_with_zero_clearance(&block_start_margin)) + } else { + None + }; + let inline_adjustment = position.inline - self.floats.containing_block_info.inline_start; + (clearance, inline_adjustment) } /// Adds a new adjoining margin. diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index 577070fbdab..ea9d9f437a2 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -4,7 +4,6 @@ //! Flow layout, also known as block-and-inline layout. -use self::float::PlacementAmongFloats; use crate::cell::ArcRefCell; use crate::context::LayoutContext; use crate::flow::float::{ContainingBlockPositionInfo, FloatBox, SequentialLayoutState}; @@ -94,7 +93,11 @@ impl BlockLevelBox { BlockLevelBox::SameFormattingContextBlock { ref style, .. } => &style, BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(_) | BlockLevelBox::OutOfFlowFloatBox(_) => return true, - BlockLevelBox::Independent(ref context) => context.style(), + BlockLevelBox::Independent(ref context) => { + // FIXME: If the element doesn't fit next to floats, it will get clearance. + // In that case this should be returning false. + context.style() + }, }; // FIXME: This should only return false when 'clear' causes clearance. @@ -879,10 +882,6 @@ impl NonReplacedFormattingContext { &self.style, ); - let block_start_margin = CollapsedMargin::new(margin.block_start); - let clearance = sequential_layout_state - .calculate_clearance(self.style.get_box().clear, &block_start_margin); - let layout = self.layout( layout_context, positioning_context, @@ -904,70 +903,35 @@ impl NonReplacedFormattingContext { // sufficient space. They may even make the border box of said element narrower // than defined by section 10.3.3. CSS 2 does not define when a UA may put said // element next to the float or by how much said element may become narrower." - let mut adjustment_from_floats = Vec2::zero(); - adjustment_from_floats.block = clearance.unwrap_or_else(Length::zero); - + let clearance; + let inline_adjustment_from_floats; let inline_size_is_auto = self .style .box_size(containing_block.style.writing_mode) .inline .is_auto(); - if !inline_size_is_auto { - // We calculate a hypothetical value for `bfc_relative_block_position`, - // assuming that there was no adjustment from floats. The real value will - // depend on whether or not there was adjustment. - let hypothetical_bfc_relative_block_position = if clearance.is_some() { - sequential_layout_state.bfc_relative_block_position + - sequential_layout_state.current_margin.solve() + - block_start_margin.solve() - } else { - sequential_layout_state.bfc_relative_block_position + - sequential_layout_state - .current_margin - .adjoin(&block_start_margin) - .solve() - }; - + let block_start_margin = CollapsedMargin::new(margin.block_start); + if inline_size_is_auto { + // TODO: Use PlacementAmongFloats to avoid overlapping floats. + clearance = sequential_layout_state + .calculate_clearance(self.style.get_box().clear, &block_start_margin); + inline_adjustment_from_floats = Length::zero(); + } else { let size = &Vec2 { inline: containing_block_for_children.inline_size, block: block_size, } + &pbm.padding_border_sums; - let placement = PlacementAmongFloats::new( - &sequential_layout_state.floats, - hypothetical_bfc_relative_block_position + clearance.unwrap_or_else(Length::zero), - size.clone(), - ) - .place(); - - // This placement is in the coordinates of the float-containing block formatting - // context, but we need to calculate an offset to use for placing this replaced - // element. - adjustment_from_floats = &placement - - &Vec2 { - inline: sequential_layout_state - .floats - .containing_block_info - .inline_start, - block: hypothetical_bfc_relative_block_position, - }; + (clearance, inline_adjustment_from_floats) = sequential_layout_state + .calculate_clearance_and_inline_adjustment( + self.style.get_box().clear, + &block_start_margin, + size.clone(), + ); } - // Clearance and any adjustment from float should prevent margin collapse, so it's - // important to make sure that it is non-None even when it is zero. Yet, when we - // didn't have clearance or any adjustment from placing next to floats, we want the - // value of clearance on the Fragment to be None, so margin collapse still works - // properly. - let effective_clearance = if clearance.is_some() || !adjustment_from_floats.block.is_zero() - { - Some(adjustment_from_floats.block) - } else { - None - }; - - // If there was effective clearance, it prevent margins collapse between this - // block and previous ones, so in that case collapse margins before adjoining - // them below. - if effective_clearance.is_some() { + // Clearance prevents margin collapse between this block and previous ones, + // so in that case collapse margins before adjoining them below. + if clearance.is_some() { sequential_layout_state.collapse_margins(); } sequential_layout_state.adjoin_assign(&block_start_margin); @@ -975,7 +939,7 @@ impl NonReplacedFormattingContext { // Margins can never collapse into independent formatting contexts. sequential_layout_state.collapse_margins(); sequential_layout_state.advance_block_position( - pbm.padding_border_sums.block + adjustment_from_floats.block + block_size, + pbm.padding_border_sums.block + block_size + clearance.unwrap_or_else(Length::zero), ); sequential_layout_state.adjoin_assign(&CollapsedMargin::new(margin.block_end)); @@ -988,11 +952,11 @@ impl NonReplacedFormattingContext { start_corner: Vec2 { block: pbm.padding.block_start + pbm.border.block_start + - adjustment_from_floats.block, + clearance.unwrap_or_else(Length::zero), inline: pbm.padding.inline_start + pbm.border.inline_start + margin.inline_start + - adjustment_from_floats.inline, + inline_adjustment_from_floats, }, size: Vec2 { block: block_size, @@ -1008,7 +972,7 @@ impl NonReplacedFormattingContext { pbm.padding, pbm.border, margin, - effective_clearance, + clearance, block_margins_collapsed_with_children, ) } @@ -1037,27 +1001,10 @@ fn layout_in_flow_replaced_block_level<'a>( }; let fragments = replaced.make_fragments(style, size.clone()); - let mut effective_clearance = None; - let mut adjustment_from_floats = Vec2::zero(); + let clearance; + let inline_adjustment_from_floats; if let Some(ref mut sequential_layout_state) = sequential_layout_state { let block_start_margin = CollapsedMargin::new(margin.block_start); - let clearance = - sequential_layout_state.calculate_clearance(style.get_box().clear, &block_start_margin); - - // We calculate a hypothetical value for `bfc_relative_block_position`, - // assuming that there was no adjustment from floats. The real value will - // depend on whether or not there was adjustment. - let hypothetical_bfc_relative_block_position = if clearance.is_some() { - sequential_layout_state.bfc_relative_block_position + - sequential_layout_state.current_margin.solve() + - block_start_margin.solve() - } else { - sequential_layout_state.bfc_relative_block_position + - sequential_layout_state - .current_margin - .adjoin(&block_start_margin) - .solve() - }; // From https://drafts.csswg.org/css2/#floats: // "The border box of a table, a block-level replaced element, or an element in @@ -1069,40 +1016,16 @@ fn layout_in_flow_replaced_block_level<'a>( // sufficient space. They may even make the border box of said element narrower // than defined by section 10.3.3. CSS 2 does not define when a UA may put said // element next to the float or by how much said element may become narrower." - let placement_among_floats = PlacementAmongFloats::new( - &sequential_layout_state.floats, - hypothetical_bfc_relative_block_position + clearance.unwrap_or_else(Length::zero), - &size + &pbm.padding_border_sums, - ) - .place(); + (clearance, inline_adjustment_from_floats) = sequential_layout_state + .calculate_clearance_and_inline_adjustment( + style.get_box().clear, + &block_start_margin, + &size + &pbm.padding_border_sums, + ); - // This placement is in the coordinates of the float-containing block formatting - // context, but we need to calculate an offset to use for placing this replaced - // element. - adjustment_from_floats = &placement_among_floats - - &Vec2 { - inline: sequential_layout_state - .floats - .containing_block_info - .inline_start, - block: hypothetical_bfc_relative_block_position, - }; - - // Clearance and any adjustment from float should prevent margin collapse, so it's - // important to make sure that it is non-None even when it is zero. Yet, when we - // didn't have clearance or any adjustment from placing next to floats, we want the - // value of clearance on the Fragment to be None, so margin collapse still works - // properly. - effective_clearance = if clearance.is_some() || !adjustment_from_floats.block.is_zero() { - Some(adjustment_from_floats.block) - } else { - None - }; - - // If there was effective clearance, it prevent margins collapse between this - // block and previous ones, so in that case collapse margins before adjoining - // them below. - if effective_clearance.is_some() { + // Clearance prevents margin collapse between this block and previous ones, + // so in that case collapse margins before adjoining them below. + if clearance.is_some() { sequential_layout_state.collapse_margins(); } sequential_layout_state.adjoin_assign(&block_start_margin); @@ -1110,17 +1033,22 @@ fn layout_in_flow_replaced_block_level<'a>( // Margins can never collapse into replaced elements. sequential_layout_state.collapse_margins(); sequential_layout_state.advance_block_position( - pbm.padding_border_sums.block + size.block + adjustment_from_floats.block, + pbm.padding_border_sums.block + size.block + clearance.unwrap_or_else(Length::zero), ); sequential_layout_state.adjoin_assign(&CollapsedMargin::new(margin.block_end)); + } else { + clearance = None; + inline_adjustment_from_floats = Length::zero(); }; let start_corner = Vec2 { - block: pbm.padding.block_start + pbm.border.block_start + adjustment_from_floats.block, + block: pbm.padding.block_start + + pbm.border.block_start + + clearance.unwrap_or_else(Length::zero), inline: pbm.padding.inline_start + pbm.border.inline_start + margin.inline_start + - adjustment_from_floats.inline, + inline_adjustment_from_floats, }; let content_rect = Rect { start_corner, size }; @@ -1133,7 +1061,7 @@ fn layout_in_flow_replaced_block_level<'a>( pbm.padding, pbm.border, margin, - effective_clearance, + clearance, block_margins_collapsed_with_children, ) } diff --git a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-2.html.ini b/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-2.html.ini deleted file mode 100644 index e48bc64ad5d..00000000000 --- a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-2.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[new-fc-separates-from-float-2.html] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-3.html.ini b/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-3.html.ini deleted file mode 100644 index 2ad67d928d1..00000000000 --- a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float-3.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[new-fc-separates-from-float-3.html] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float.html.ini b/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float.html.ini deleted file mode 100644 index 75f0028b81a..00000000000 --- a/tests/wpt/meta/css/CSS2/floats/new-fc-separates-from-float.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[new-fc-separates-from-float.html] - expected: FAIL