Handle inline margins when avoiding floats (#30072)

components/layout_2020/flow/float.rs

@@ -13,7 +13,7 @@ use crate::formatting_contexts::IndependentFormattingContext;
 use crate::fragment_tree::{BoxFragment, CollapsedBlockMargins, CollapsedMargin, FloatFragment};
 use crate::geom::flow_relative::{Rect, Vec2};
 use crate::positioned::PositioningContext;
-use crate::style_ext::{ComputedValuesExt, DisplayInside};
+use crate::style_ext::{ComputedValuesExt, DisplayInside, PaddingBorderMargin};
 use crate::ContainingBlock;
 use euclid::num::Zero;
 use servo_arc::Arc;
@@ -85,6 +85,11 @@ pub(crate) struct PlacementAmongFloats<'a> {
     /// The minimum position in the block direction for the placement. Objects should not
     /// be placed before this point.
     ceiling: Length,
+    /// The inline position where the object would be if there were no floats. The object
+    /// can be placed after it due to floats, but not before it.
+    min_inline_start: Length,
+    /// The maximum inline position that the object can attain when avoiding floats.
+    max_inline_end: Length,
 }
 
 impl<'a> PlacementAmongFloats<'a> {
@@ -92,18 +97,26 @@ impl<'a> PlacementAmongFloats<'a> {
         float_context: &'a FloatContext,
         ceiling: Length,
         object_size: Vec2<Length>,
+        pbm: &PaddingBorderMargin,
     ) -> Self {
         assert!(!ceiling.px().is_infinite());
         let mut current_band = float_context.bands.find(ceiling).unwrap();
         current_band.top = ceiling;
         let current_bands = VecDeque::from([current_band]);
         let next_band = float_context.bands.find_next(ceiling).unwrap();
+        let min_inline_start = float_context.containing_block_info.inline_start +
+            pbm.margin.inline_start.auto_is(Length::zero);
+        let max_inline_end = (float_context.containing_block_info.inline_end -
+            pbm.margin.inline_end.auto_is(Length::zero))
+        .max(min_inline_start + object_size.inline);
         PlacementAmongFloats {
             float_context,
             current_bands,
             next_band,
             object_size,
             ceiling,
+            min_inline_start,
+            max_inline_end,
         }
     }
 
@@ -147,8 +160,8 @@ impl<'a> PlacementAmongFloats<'a> {
     /// Find the start and end of the inline space provided by the current set of bands
     /// under consideration.
     fn calculate_inline_start_and_end(&self) -> (Length, Length) {
-        let mut max_inline_start = self.float_context.containing_block_info.inline_start;
+        let mut max_inline_start = self.min_inline_start;
-        let mut min_inline_end = self.float_context.containing_block_info.inline_end;
+        let mut min_inline_end = self.max_inline_end;
         for band in self.current_bands.iter() {
             if let Some(left) = band.left {
                 max_inline_start = max_inline_start.max(left);
@@ -200,15 +213,14 @@ impl<'a> PlacementAmongFloats<'a> {
         // cleared all floats.
         Rect {
             start_corner: Vec2 {
-                inline: self.float_context.containing_block_info.inline_start,
+                inline: self.min_inline_start,
                 block: self
                     .ceiling
                     .max(self.float_context.clear_left_position)
                     .max(self.float_context.clear_right_position),
             },
             size: Vec2 {
-                inline: self.float_context.containing_block_info.inline_end -
+                inline: self.max_inline_end - self.min_inline_start,
-                    self.float_context.containing_block_info.inline_start,
                 block: Length::new(f32::INFINITY),
             },
         }
@@ -1067,34 +1079,35 @@ impl SequentialLayoutState {
 
     /// 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',
+    /// Given such a block with the provided 'clear', 'block_start_margin', 'pbm' 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.
+    /// - The Rect in which the block can be placed without overlapping floats.
     pub(crate) fn calculate_clearance_and_inline_adjustment(
         &self,
         clear: Clear,
         block_start_margin: &CollapsedMargin,
+        pbm: &PaddingBorderMargin,
         object_size: Vec2<Length>,
-    ) -> (Option<Length>, Length) {
+    ) -> (Option<Length>, Rect<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 mut placement = PlacementAmongFloats::new(&self.floats, ceiling, object_size, pbm);
-        let position = placement.place().start_corner;
+        let placement_rect = placement.place();
+        let position = &placement_rect.start_corner;
         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, placement_rect)
-        (clearance, inline_adjustment)
     }
 
     /// Adds a new adjoining margin.

components/layout_2020/flow/mod.rs

@@ -885,6 +885,8 @@ impl NonReplacedFormattingContext {
             block_size.clamp_between_extremums(min_box_size.block, max_box_size.block)
         });
 
+        let margin_inline_start;
+        let margin_inline_end;
         let (margin_block_start, margin_block_end) =
             solve_block_margins_for_in_flow_block_level(&pbm);
         let collapsed_margin_block_start = CollapsedMargin::new(margin_block_start);
@@ -900,7 +902,6 @@ impl NonReplacedFormattingContext {
         //  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 clearance;
-        let inline_adjustment_from_floats;
         let mut content_size;
         let mut layout;
         if let LengthOrAuto::LengthPercentage(ref inline_size) = box_size.inline {
@@ -924,11 +925,14 @@ impl NonReplacedFormattingContext {
                 }),
             };
 
-            (clearance, inline_adjustment_from_floats) = sequential_layout_state
+            (clearance, (margin_inline_start, margin_inline_end)) =
-                .calculate_clearance_and_inline_adjustment(
+                solve_clearance_and_inline_margins_avoiding_floats(
-                    self.style.get_box().clear,
+                    &sequential_layout_state,
+                    &containing_block,
                     &collapsed_margin_block_start,
+                    &pbm,
                     &content_size + &pbm.padding_border_sums,
+                    &self.style,
                 );
         } else {
             // First compute the clear position required by the 'clear' property.
@@ -951,6 +955,7 @@ impl NonReplacedFormattingContext {
                 &sequential_layout_state.floats,
                 ceiling,
                 minimum_size_of_block,
+                &pbm,
             );
             let mut placement_rect;
 
@@ -1013,19 +1018,15 @@ impl NonReplacedFormattingContext {
                 None
             };
 
-            inline_adjustment_from_floats = placement_rect.start_corner.inline -
+            (margin_inline_start, margin_inline_end) = solve_inline_margins_avoiding_floats(
-                sequential_layout_state
+                &sequential_layout_state,
-                    .floats
+                &containing_block,
-                    .containing_block_info
+                &pbm,
-                    .inline_start;
+                content_size.inline + pbm.padding_border_sums.inline,
+                placement_rect,
+            );
         }
 
-        // TODO: solve_inline_margins_for_in_flow_block_level() doesn't take floats into account.
-        let (margin_inline_start, margin_inline_end) = solve_inline_margins_for_in_flow_block_level(
-            &containing_block,
-            &pbm,
-            content_size.inline,
-        );
         let margin = Sides {
             inline_start: margin_inline_start,
             inline_end: margin_inline_end,
@@ -1054,10 +1055,7 @@ impl NonReplacedFormattingContext {
                 block: pbm.padding.block_start +
                     pbm.border.block_start +
                     clearance.unwrap_or_else(Length::zero),
-                inline: pbm.padding.inline_start +
+                inline: pbm.padding.inline_start + pbm.border.inline_start + margin.inline_start,
-                    pbm.border.inline_start +
-                    margin.inline_start +
-                    inline_adjustment_from_floats,
             },
             size: content_size,
         };
@@ -1087,24 +1085,15 @@ fn layout_in_flow_replaced_block_level<'a>(
     mut sequential_layout_state: Option<&mut SequentialLayoutState>,
 ) -> BoxFragment {
     let pbm = style.padding_border_margin(containing_block);
-    let size = replaced.used_size_as_if_inline_element(containing_block, style, None, &pbm);
+    let content_size = replaced.used_size_as_if_inline_element(containing_block, style, None, &pbm);
 
-    // TODO: solve_inline_margins_for_in_flow_block_level() doesn't take floats into account.
+    let margin_inline_start;
-    let (margin_inline_start, margin_inline_end) =
+    let margin_inline_end;
-        solve_inline_margins_for_in_flow_block_level(containing_block, &pbm, size.inline);
+    let (margin_block_start, margin_block_end) = solve_block_margins_for_in_flow_block_level(&pbm);
-    let margin = Sides {
+    let fragments = replaced.make_fragments(style, content_size.clone());
-        inline_start: margin_inline_start,
-        inline_end: margin_inline_end,
-        block_start: pbm.margin.block_start.auto_is(Length::zero),
-        block_end: pbm.margin.block_end.auto_is(Length::zero),
-    };
-    let fragments = replaced.make_fragments(style, size.clone());
 
     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);
-
         // From https://drafts.csswg.org/css2/#floats:
         // "The border box of a table, a block-level replaced element, or an element in
         //  the normal flow that establishes a new block formatting context (such as an
@@ -1115,11 +1104,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."
-        (clearance, inline_adjustment_from_floats) = sequential_layout_state
+        let collapsed_margin_block_start = CollapsedMargin::new(margin_block_start);
-            .calculate_clearance_and_inline_adjustment(
+        let size = &content_size + &pbm.padding_border_sums;
-                style.get_box().clear,
+        (clearance, (margin_inline_start, margin_inline_end)) =
-                &block_start_margin,
+            solve_clearance_and_inline_margins_avoiding_floats(
-                &size + &pbm.padding_border_sums,
+                &sequential_layout_state,
+                &containing_block,
+                &collapsed_margin_block_start,
+                &pbm,
+                size.clone(),
+                &style,
             );
 
         // Clearance prevents margin collapse between this block and previous ones,
@@ -1127,30 +1121,40 @@ fn layout_in_flow_replaced_block_level<'a>(
         if clearance.is_some() {
             sequential_layout_state.collapse_margins();
         }
-        sequential_layout_state.adjoin_assign(&block_start_margin);
+        sequential_layout_state.adjoin_assign(&collapsed_margin_block_start);
 
         // Margins can never collapse into replaced elements.
         sequential_layout_state.collapse_margins();
-        sequential_layout_state.advance_block_position(
+        sequential_layout_state
-            pbm.padding_border_sums.block + size.block + clearance.unwrap_or_else(Length::zero),
+            .advance_block_position(size.block + clearance.unwrap_or_else(Length::zero));
-        );
+        sequential_layout_state.adjoin_assign(&CollapsedMargin::new(margin_block_end));
-        sequential_layout_state.adjoin_assign(&CollapsedMargin::new(margin.block_end));
     } else {
         clearance = None;
-        inline_adjustment_from_floats = Length::zero();
+        (margin_inline_start, margin_inline_end) = solve_inline_margins_for_in_flow_block_level(
+            containing_block,
+            &pbm,
+            content_size.inline,
+        );
+    };
+
+    let margin = Sides {
+        inline_start: margin_inline_start,
+        inline_end: margin_inline_end,
+        block_start: margin_block_start,
+        block_end: margin_block_end,
     };
 
     let start_corner = Vec2 {
         block: pbm.padding.block_start +
             pbm.border.block_start +
             clearance.unwrap_or_else(Length::zero),
-        inline: pbm.padding.inline_start +
+        inline: pbm.padding.inline_start + pbm.border.inline_start + margin.inline_start,
-            pbm.border.inline_start +
-            margin.inline_start +
-            inline_adjustment_from_floats,
     };
 
-    let content_rect = Rect { start_corner, size };
+    let content_rect = Rect {
+        start_corner,
+        size: content_size,
+    };
     let block_margins_collapsed_with_children = CollapsedBlockMargins::from_margin(&margin);
     BoxFragment::new(
         base_fragment_info,
@@ -1239,6 +1243,62 @@ fn solve_containing_block_padding_border_and_margin_for_in_flow_box<'a>(
     }
 }
 
+/// A block-level element that establishes an independent formatting context (or is replaced)
+/// must not overlap floats.
+/// This can be achieved by adding clearance (to adjust the position in the block axis)
+/// and/or modifying the margins in the inline axis.
+/// This function takes care of calculating them.
+fn solve_clearance_and_inline_margins_avoiding_floats(
+    sequential_layout_state: &SequentialLayoutState,
+    containing_block: &ContainingBlock,
+    block_start_margin: &CollapsedMargin,
+    pbm: &PaddingBorderMargin,
+    size: Vec2<Length>,
+    style: &Arc<ComputedValues>,
+) -> (Option<Length>, (Length, Length)) {
+    let (clearance, placement_rect) = sequential_layout_state
+        .calculate_clearance_and_inline_adjustment(
+            style.get_box().clear,
+            &block_start_margin,
+            &pbm,
+            size.clone(),
+        );
+    let inline_margins = solve_inline_margins_avoiding_floats(
+        &sequential_layout_state,
+        &containing_block,
+        &pbm,
+        size.inline,
+        placement_rect,
+    );
+    (clearance, inline_margins)
+}
+
+/// Resolves the margins of an in-flow block-level box in the inline axis
+/// so that it's placed within the given rect, avoiding floats.
+/// Auto margins resolve using the free space in the rect.
+fn solve_inline_margins_avoiding_floats(
+    sequential_layout_state: &SequentialLayoutState,
+    containing_block: &ContainingBlock,
+    pbm: &PaddingBorderMargin,
+    inline_size: Length,
+    placement_rect: Rect<Length>,
+) -> (Length, Length) {
+    let inline_adjustment = placement_rect.start_corner.inline -
+        sequential_layout_state
+            .floats
+            .containing_block_info
+            .inline_start;
+    assert!(placement_rect.size.inline >= inline_size);
+    let free_space = placement_rect.size.inline - inline_size;
+    let margin_inline_start = match (pbm.margin.inline_start, pbm.margin.inline_end) {
+        (LengthOrAuto::Auto, LengthOrAuto::Auto) => inline_adjustment + free_space / 2.,
+        (LengthOrAuto::Auto, _) => inline_adjustment + free_space,
+        _ => inline_adjustment,
+    };
+    let margin_inline_end = containing_block.inline_size - inline_size - margin_inline_start;
+    (margin_inline_start, margin_inline_end)
+}
+
 /// Resolves the margins of an in-flow block-level box in the inline axis,
 /// distributing free space into 'auto' values and solving over-constrained cases.
 /// <https://drafts.csswg.org/css2/#blockwidth>

tests/wpt/meta-legacy-layout/css/CSS2/floats/floats-wrap-bfc-with-margin-008.tentative.html.ini

@@ -0,0 +1,2 @@
+[floats-wrap-bfc-with-margin-008.tentative.html]
+  expected: FAIL

tests/wpt/meta-legacy-layout/css/CSS2/floats/floats-wrap-bfc-with-margin-009.tentative.html.ini

@@ -0,0 +1,2 @@
+[floats-wrap-bfc-with-margin-009.tentative.html]
+  expected: FAIL

tests/wpt/meta/MANIFEST.json

@@ -61729,6 +61729,58 @@
         {}
        ]
       ],
+      "floats-wrap-bfc-with-margin-006.tentative.html": [
+       "fa337713259eab84315012ee3a1f598014fa0cb8",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/reference/ref-filled-green-100px-square.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
+      "floats-wrap-bfc-with-margin-007.tentative.html": [
+       "552b3bc3dacd6b0ae9d8f1caae1ecde902efbdf1",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/reference/ref-filled-green-100px-square.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
+      "floats-wrap-bfc-with-margin-008.tentative.html": [
+       "34c0ee86113e9b818f2014fb49412b580a2b47ce",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/reference/ref-filled-green-100px-square.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
+      "floats-wrap-bfc-with-margin-009.tentative.html": [
+       "61db4b1fa3eaa9ebe5fa210d00c24d4cdaf24dec",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/reference/ref-filled-green-100px-square.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
       "floats-wrap-top-below-bfc-001l.xht": [
        "17bec33eb143ce14f13c439e83f433c3fef74fdd",
        [

tests/wpt/meta/css/CSS2/floats/floats-wrap-bfc-with-margin-004.html.ini

@@ -1,2 +0,0 @@
-[floats-wrap-bfc-with-margin-004.html]
-  expected: FAIL

tests/wpt/meta/css/CSS2/floats/floats-wrap-bfc-with-margin-005.html.ini

@@ -1,2 +0,0 @@
-[floats-wrap-bfc-with-margin-005.html]
-  expected: FAIL

tests/wpt/meta/css/CSS2/floats/new-fc-beside-float-with-margin-rtl.html.ini

@@ -1,2 +0,0 @@
-[new-fc-beside-float-with-margin-rtl.html]
-  expected: FAIL

tests/wpt/meta/css/CSS2/floats/new-fc-beside-float-with-margin.html.ini

@@ -0,0 +1,2 @@
+[new-fc-beside-float-with-margin.html]
+  expected: FAIL

tests/wpt/meta/css/CSS2/floats/zero-width-floats-positioning.tentative.html.ini

@@ -1,2 +0,0 @@
-[zero-width-floats-positioning.tentative.html]
-  expected: FAIL

tests/wpt/meta/css/css-flexbox/flex-direction-modify.html.ini

@@ -1,2 +0,0 @@
-[flex-direction-modify.html]
-  expected: FAIL

tests/wpt/mozilla/meta/css/block_formatting_context_margin_inout_a.html.ini

@@ -1,2 +0,0 @@
-[block_formatting_context_margin_inout_a.html]
-  expected: FAIL

tests/wpt/mozilla/meta/css/block_formatting_context_negative_margins_a.html.ini

@@ -1,2 +0,0 @@
-[block_formatting_context_negative_margins_a.html]
-  expected: FAIL

tests/wpt/tests/css/CSS2/floats/floats-wrap-bfc-with-margin-006.tentative.html

@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css2/#floats">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The BFC roots fit next to the floats, so they shouldn't be moved below.">
+<style>
+.wrapper {
+  width: 50px;
+  margin-left: 50px;
+  background: red;
+}
+.float {
+  float: right;
+  clear: right;
+  width: 25px;
+  height: 50px;
+  background: green;
+}
+.bfc {
+  overflow: hidden;
+  height: 50px;
+  margin-left: -50px;
+  background: green;
+}
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div class="wrapper">
+  <div class="float"></div>
+  <div class="bfc"></div>
+  <div class="float"></div>
+  <div class="bfc" style="width: 75px"></div>
+</div>

tests/wpt/tests/css/CSS2/floats/floats-wrap-bfc-with-margin-007.tentative.html

@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css2/#floats">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The BFC roots fit next to the floats, so they shouldn't be moved below.">
+<style>
+.wrapper {
+  width: 50px;
+  background: red;
+}
+.float {
+  float: left;
+  clear: left;
+  width: 25px;
+  height: 50px;
+  background: green;
+}
+.bfc {
+  overflow: hidden;
+  height: 50px;
+  margin-right: -50px;
+  background: green;
+}
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div class="wrapper">
+  <div class="float"></div>
+  <div class="bfc"></div>
+  <div class="float"></div>
+  <div class="bfc" style="width: 75px"></div>
+</div>

tests/wpt/tests/css/CSS2/floats/floats-wrap-bfc-with-margin-008.tentative.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css2/#floats">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The BFC root doesn't fit next to the float, so it should be moved below.">
+<style>
+.wrapper {
+  width: 100px;
+  background: green;
+}
+.float {
+  float: right;
+  width: 50px;
+  height: 50px;
+  background: green;
+}
+.bfc {
+  overflow: hidden;
+  width: 100px;
+  height: 50px;
+  background: green;
+}
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div class="wrapper">
+  <div class="float"></div>
+  <div style="margin-right: 50px">
+    <div class="bfc"></div>
+  </div>
+</div>

tests/wpt/tests/css/CSS2/floats/floats-wrap-bfc-with-margin-009.tentative.html

@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css2/#floats">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The BFC root doesn't fit next to the float, so it should be moved below.">
+<style>
+.wrapper {
+  width: 100px;
+  background: green;
+}
+.float {
+  float: left;
+  width: 50px;
+  height: 50px;
+  background: green;
+}
+.bfc {
+  overflow: hidden;
+  width: 100px;
+  height: 50px;
+  margin-left: -50px;
+  background: green;
+}
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div class="wrapper">
+  <div class="float"></div>
+  <div style="margin-left: 50px">
+    <div class="bfc"></div>
+  </div>
+</div>