diff options
author | Martin Robinson <mrobinson@igalia.com> | 2023-06-21 08:31:55 +0200 |
---|---|---|
committer | Oriol Brufau <obrufau@igalia.com> | 2023-06-22 01:55:21 +0200 |
commit | 30ab348116a90e47f2fbb97a0f393c181cbdb175 (patch) | |
tree | 392de665307a300d225e11fee6ce5ef93e808735 | |
parent | 9edc2c664fc4b5e04d8805107369ca0f20081771 (diff) | |
download | servo-30ab348116a90e47f2fbb97a0f393c181cbdb175.tar.gz servo-30ab348116a90e47f2fbb97a0f393c181cbdb175.zip |
Properly position absolutes with static insets that are children of floats
Previously, final float positions were calculated when their parents
were positioned. This prevented proper positioning of absolute children
of floats with static insets, because they accumulate offsets as they
are hoisted up the tree.
This change moves the final float positioning to
`PlacementState::place_fragment` for the float itself so that it happens
before any insets are updated for hoisted descendants. In addition to
simplifying the code, this makes it a bit more efficient. Finally,
floats are taken into account when updating static insets of hoisted
boxes.
Fixes #29826.
17 files changed, 114 insertions, 135 deletions
diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index 82d4fa2efc9..7ccb092374b 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -4,8 +4,6 @@ //! Flow layout, also known as block-and-inline layout. -use std::ops::DerefMut; - use crate::cell::ArcRefCell; use crate::context::LayoutContext; use crate::flow::float::{ClearSide, ContainingBlockPositionInfo, FloatBox, SequentialLayoutState}; @@ -299,7 +297,7 @@ fn layout_block_level_children_in_parallel( let fragments = layout_results .into_iter() .map(|(mut fragment, mut child_positioning_context)| { - placement_state.place_fragment(&mut fragment); + placement_state.place_fragment(&mut fragment, None); child_positioning_context.adjust_static_position_of_hoisted_fragments(&fragment); positioning_context.append(child_positioning_context); fragment @@ -341,9 +339,7 @@ fn layout_block_level_children_sequentially( Some(&mut *sequential_layout_state), ); - placement_state.place_fragment(&mut fragment); - placement_state - .adjust_positions_of_float_children(&mut fragment, sequential_layout_state); + placement_state.place_fragment(&mut fragment, Some(sequential_layout_state)); child_positioning_context.adjust_static_position_of_hoisted_fragments(&fragment); positioning_context.append(child_positioning_context); fragment @@ -796,7 +792,11 @@ impl PlacementState { } } - fn place_fragment(&mut self, fragment: &mut Fragment) { + fn place_fragment( + &mut self, + fragment: &mut Fragment, + sequential_layout_state: Option<&mut SequentialLayoutState>, + ) { match fragment { Fragment::Box(fragment) => { let fragment_block_margins = &fragment.block_margins_collapsed_with_children; @@ -855,7 +855,25 @@ impl PlacementState { }; fragment.borrow_mut().adjust_offsets(offset); }, - Fragment::Anonymous(_) | Fragment::Float(_) => {}, + Fragment::Float(box_fragment) => { + // When Float fragments are created in block flows, they are positioned + // relative to the float containing independent block formatting context. + // Once we place a float's containing block, this function can be used to + // fix up the float position to be relative to the containing block. + let sequential_layout_state = sequential_layout_state + .expect("Found float fragment without SequentialLayoutState"); + let containing_block_info = &sequential_layout_state.floats.containing_block_info; + let margin = containing_block_info + .block_start_margins_not_collapsed + .adjoin(&self.start_margin); + let parent_fragment_offset_in_formatting_context = Vec2 { + inline: containing_block_info.inline_start, + block: containing_block_info.block_start + margin.solve(), + }; + box_fragment.content_rect.start_corner = &box_fragment.content_rect.start_corner - + &parent_fragment_offset_in_formatting_context; + }, + Fragment::Anonymous(_) => {}, _ => unreachable!(), } } @@ -874,53 +892,4 @@ impl PlacementState { }, ) } - - /// When Float fragments are created in block flows, they are positioned - /// relative to the float containing independent block formatting context. - /// Once we place a float's containing block, this function can be used to - /// fix up the float position to be relative to the containing block. - fn adjust_positions_of_float_children( - &self, - fragment: &mut Fragment, - sequential_layout_state: &mut SequentialLayoutState, - ) { - let fragment = match fragment { - Fragment::Box(ref mut fragment) => fragment, - _ => return, - }; - - // TODO(mrobinson): Will these margins be accurate if this fragment - // collapses through. Can a fragment collapse through when it has a - // non-zero sized float inside? The float won't be positioned correctly - // anyway (see the comment in `floats.rs` about margin collapse), but - // this might make the result even worse. - let collapsed_margins = self.start_margin.adjoin( - &sequential_layout_state - .floats - .containing_block_info - .block_start_margins_not_collapsed, - ); - - let parent_fragment_offset_in_cb = &fragment.content_rect.start_corner; - let parent_fragment_offset_in_formatting_context = Vec2 { - inline: sequential_layout_state - .floats - .containing_block_info - .inline_start + - parent_fragment_offset_in_cb.inline, - block: sequential_layout_state - .floats - .containing_block_info - .block_start + - collapsed_margins.solve() + - parent_fragment_offset_in_cb.block, - }; - - for child_fragment in fragment.children.iter_mut() { - if let Fragment::Float(box_fragment) = child_fragment.borrow_mut().deref_mut() { - box_fragment.content_rect.start_corner = &box_fragment.content_rect.start_corner - - &parent_fragment_offset_in_formatting_context; - } - } - } } diff --git a/components/layout_2020/positioned.rs b/components/layout_2020/positioned.rs index 446bef4811d..b1ab687a4e9 100644 --- a/components/layout_2020/positioned.rs +++ b/components/layout_2020/positioned.rs @@ -167,8 +167,8 @@ impl PositioningContext { parent_fragment: &Fragment, ) { let fragment_rect = match &parent_fragment { - Fragment::Box(b) => &b.content_rect, - Fragment::AbsoluteOrFixedPositioned(_) | Fragment::Float(_) => return, + Fragment::Box(b) | Fragment::Float(b) => &b.content_rect, + Fragment::AbsoluteOrFixedPositioned(_) => return, Fragment::Anonymous(a) => &a.rect, _ => unreachable!(), }; diff --git a/tests/wpt/metadata-layout-2020/css/CSS2/borders/border-right-width-095.xht.ini b/tests/wpt/metadata-layout-2020/css/CSS2/borders/border-right-width-095.xht.ini deleted file mode 100644 index 2e4df1bb52f..00000000000 --- a/tests/wpt/metadata-layout-2020/css/CSS2/borders/border-right-width-095.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[border-right-width-095.xht] - expected: FAIL diff --git a/tests/wpt/metadata-layout-2020/css/CSS2/floats-clear/clear-with-top-margin-after-cleared-empty-block.html.ini b/tests/wpt/metadata-layout-2020/css/CSS2/floats-clear/clear-with-top-margin-after-cleared-empty-block.html.ini deleted file mode 100644 index c1602cdda56..00000000000 --- a/tests/wpt/metadata-layout-2020/css/CSS2/floats-clear/clear-with-top-margin-after-cleared-empty-block.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[clear-with-top-margin-after-cleared-empty-block.html] - expected: FAIL diff --git a/tests/wpt/metadata-layout-2020/css/CSS2/margin-padding-clear/margin-collapse-min-height-001.xht.ini b/tests/wpt/metadata-layout-2020/css/CSS2/margin-padding-clear/margin-collapse-min-height-001.xht.ini deleted file mode 100644 index b2b41d98c21..00000000000 --- a/tests/wpt/metadata-layout-2020/css/CSS2/margin-padding-clear/margin-collapse-min-height-001.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[margin-collapse-min-height-001.xht] - expected: FAIL diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 6bd57108d76..f0b065a74d8 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -59963,6 +59963,19 @@ {} ] ], + "float-with-absolutely-positioned-child-with-static-inset.html": [ + "7d022306a861e8be7635bb2d0a8d7db60ba147c9", + [ + null, + [ + [ + "/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset-ref.html", + "==" + ] + ], + {} + ] + ], "floated-table-wider-than-specified.html": [ "f93d50e43dd3eb49d5c8964200b7fe4ebb5bd6c8", [ @@ -360016,6 +360029,10 @@ "6b46fb8eb9b890f51742f7eb6a2935677ceddbc7", [] ], + "float-with-absolutely-positioned-child-with-static-inset-ref.html": [ + "7cab50fc04b4d5788a482964e3616b906637e8e9", + [] + ], "floats-in-table-caption-001-ref.html": [ "91ddca772b7100d355c9bf766c530b78dd87a959", [] diff --git a/tests/wpt/mozilla/meta-layout-2020/css/absolute_hypothetical_float.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/absolute_hypothetical_float.html.ini deleted file mode 100644 index 290204b3777..00000000000 --- a/tests/wpt/mozilla/meta-layout-2020/css/absolute_hypothetical_float.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[absolute_hypothetical_float.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_a.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_a.html.ini deleted file mode 100644 index 6460fdb8036..00000000000 --- a/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_a.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[floats_margin_collapse_a.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_with_clearance_a.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_with_clearance_a.html.ini new file mode 100644 index 00000000000..ce0ab0b1ec1 --- /dev/null +++ b/tests/wpt/mozilla/meta-layout-2020/css/floats_margin_collapse_with_clearance_a.html.ini @@ -0,0 +1,2 @@ +[floats_margin_collapse_with_clearance_a.html] + expected: FAIL diff --git a/tests/wpt/mozilla/meta-layout-2020/css/overflow_clipping.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/overflow_clipping.html.ini deleted file mode 100644 index 00f0091e93a..00000000000 --- a/tests/wpt/mozilla/meta-layout-2020/css/overflow_clipping.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[overflow_clipping.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta-layout-2020/css/position_relative_stacking_context_contents_a.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/position_relative_stacking_context_contents_a.html.ini deleted file mode 100644 index 63c286cb005..00000000000 --- a/tests/wpt/mozilla/meta-layout-2020/css/position_relative_stacking_context_contents_a.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[position_relative_stacking_context_contents_a.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta-layout-2020/css/stacked_layers.html.ini b/tests/wpt/mozilla/meta-layout-2020/css/stacked_layers.html.ini deleted file mode 100644 index e13feb290c6..00000000000 --- a/tests/wpt/mozilla/meta-layout-2020/css/stacked_layers.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[stacked_layers.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index f8593976dd6..f8838691376 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -106,19 +106,6 @@ {} ] ], - "absolute_hypothetical_float.html": [ - "33506180d15b5ff5e18b4f8adce4e2346e4ff811", - [ - null, - [ - [ - "/_mozilla/css/absolute_hypothetical_float_ref.html", - "==" - ] - ], - {} - ] - ], "absolute_hypothetical_with_intervening_inline_block_a.html": [ "54d92051775d6fa2e5e3ac3ceb1848f58ae2c653", [ @@ -8104,10 +8091,6 @@ "62d0965f205e1d40ac752ee4324469a82ef3fae4", [] ], - "absolute_hypothetical_float_ref.html": [ - "008b2a65d29bddd21f7754af0422c3366d0c9d25", - [] - ], "absolute_hypothetical_with_intervening_inline_block_ref.html": [ "985e941cdd9fafd6412cf2e76955ee6b614affdc", [] diff --git a/tests/wpt/mozilla/tests/css/absolute_hypothetical_float.html b/tests/wpt/mozilla/tests/css/absolute_hypothetical_float.html deleted file mode 100644 index 33506180d15..00000000000 --- a/tests/wpt/mozilla/tests/css/absolute_hypothetical_float.html +++ /dev/null @@ -1,22 +0,0 @@ -<!DOCTYPE html> -<html> -<head> -<link rel=match href=absolute_hypothetical_float_ref.html> -<style> -#a { - float: right; - width: 250px; -} -#b { - background: blue; - position: absolute; - width: 100px; - height: 100px; -} -</style> -</head> -<body> -<div id=a><div id=b>asdf</div></div> -</body> -</html> - diff --git a/tests/wpt/mozilla/tests/css/absolute_hypothetical_float_ref.html b/tests/wpt/mozilla/tests/css/absolute_hypothetical_float_ref.html deleted file mode 100644 index 008b2a65d29..00000000000 --- a/tests/wpt/mozilla/tests/css/absolute_hypothetical_float_ref.html +++ /dev/null @@ -1,21 +0,0 @@ -<!DOCTYPE html> -<html> -<head> -<style> -#a { - float: right; - width: 250px; -} -#b { - background: blue; - width: 100px; - height: 100px; -} -</style> -</head> -<body> -<div id=a><div id=b>asdf</div></div> -</body> -</html> - - diff --git a/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset-ref.html b/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset-ref.html new file mode 100644 index 00000000000..7cab50fc04b --- /dev/null +++ b/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset-ref.html @@ -0,0 +1,32 @@ +<!DOCTYPE html> +<link rel="author" title="Martin Robinson" href="mrobinson@igalia.com"> +<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position"> +<meta name="assert" content="An absolutely positioned child of a float with a static inset should be positioned the same as a non-positioned child of that float." /> + +<style> +#float { + float: right; + width: 250px; +} + +#abs-child { + background: green; + width: 100px; + height: 100px; +} + +#abs-grandchild { + background: darkgreen; + width: 50px; + height: 50px; +} +</style> + +<body> + <div id="float"> + <div id="abs-child"> + <div id="abs-grandchild"></div> + </div> + </div> +</body> +</html> diff --git a/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset.html b/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset.html new file mode 100644 index 00000000000..7d022306a86 --- /dev/null +++ b/tests/wpt/web-platform-tests/css/CSS2/floats/float-with-absolutely-positioned-child-with-static-inset.html @@ -0,0 +1,35 @@ +<!DOCTYPE html> +<link rel="author" title="Martin Robinson" href="mrobinson@igalia.com"> +<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position"> +<link rel="match" href="float-with-absolutely-positioned-child-with-static-inset-ref.html"> +<meta name="assert" content="An absolutely positioned child of a float with a static inset should be positioned the same as a non-positioned child of that float." /> + +<style> +#float { + float: right; + width: 250px; +} + +#abs-child { + background: green; + position: absolute; + width: 100px; + height: 100px; +} + +#abs-grandchild { + background: darkgreen; + position: absolute; + width: 50px; + height: 50px; +} +</style> + +<body> + <div id="float"> + <div id="abs-child"> + <div id="abs-grandchild"></div> + </div> + </div> +</body> +</html> |