LibWeb: Do not clear float sides for floating boxes with clear: ..
Some checks are pending
CI / Lagom (arm64, Sanitizer_CI, false, macos-15, macOS, Clang) (push) Waiting to run
CI / Lagom (x86_64, Fuzzers_CI, false, ubuntu-24.04, Linux, Clang) (push) Waiting to run
CI / Lagom (x86_64, Sanitizer_CI, false, ubuntu-24.04, Linux, GNU) (push) Waiting to run
CI / Lagom (x86_64, Sanitizer_CI, true, ubuntu-24.04, Linux, Clang) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (macos-14, macOS, macOS-universal2) (push) Waiting to run
Package the js repl as a binary artifact / build-and-package (ubuntu-24.04, Linux, Linux-x86_64) (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

We used to always clear the side data after encountering a box with
`clear: ..`, but this is not the right thing to do if that same box also
has `float: ..` set. For example, with `clear: right` and `float: left`
it might be that the next box still wants to clear the right side, and
since the previous box is floating it did not push the next box down far
enough to justify clearing the side data at that point.

This changes the logic to only clear the float side if the clearing box
itself is not floating. We also no longer clear the opposite side after
placing a floating box; that doesn't seem to be necessary anymore.

Fixes #4102.
This commit is contained in:
Jelle Raaijmakers 2025-03-26 19:26:20 +00:00
parent a6b664d4d0
commit f340f8682b
Notes: github-actions[bot] 2025-03-27 00:57:50 +00:00
4 changed files with 57 additions and 14 deletions

View file

@ -933,7 +933,8 @@ BlockFormattingContext::DidIntroduceClearance BlockFormattingContext::clear_floa
m_y_offset_of_current_block_container = clearance_y_in_containing_block;
}
float_side.clear();
if (!child_box.is_floating())
float_side.clear();
}
};
@ -1053,7 +1054,7 @@ void BlockFormattingContext::layout_floating_box(Box const& box, BlockContainer
}
// Then we float it to the left or right.
auto float_box = [&](FloatSide side, FloatSideData& side_data, FloatSideData& other_side_data) {
auto float_box = [&](FloatSide side, FloatSideData& side_data) {
CSSPixels offset_from_edge = 0;
auto float_to_edge = [&] {
if (side == FloatSide::Left)
@ -1165,18 +1166,13 @@ void BlockFormattingContext::layout_floating_box(Box const& box, BlockContainer
// NOTE: We don't set the X position here, that happens later, once we know the root block width.
// See parent_context_did_dimension_child_root_box() for that logic.
box_state.set_content_y(y);
// If the new box was inserted below the bottom of the opposite side,
// we reset the other side back to its edge.
if (top_margin_edge > other_side_data.y_offset)
other_side_data.clear();
};
// Next, float to the left and/or right
if (box.computed_values().float_() == CSS::Float::Left) {
float_box(FloatSide::Left, m_left_floats, m_right_floats);
float_box(FloatSide::Left, m_left_floats);
} else if (box.computed_values().float_() == CSS::Float::Right) {
float_box(FloatSide::Right, m_right_floats, m_left_floats);
float_box(FloatSide::Right, m_right_floats);
}
m_state.get_mutable(root()).add_floating_descendant(box);

View file

@ -314,11 +314,9 @@ void InlineFormattingContext::generate_line_boxes()
case InlineLevelIterator::Item::Type::FloatingElement:
if (is<Box>(*item.node)) {
[[maybe_unused]] auto introduce_clearance = parent().clear_floating_boxes(*item.node, *this);
// Even if this introduces clearance, we do NOT reset
// the margin state, because that is clearance between
// floats and does not contribute to the height of the
// Inline Formatting Context.
(void)parent().clear_floating_boxes(*item.node, *this);
// Even if this introduces clearance, we do NOT reset the margin state, because that is clearance
// between floats and does not contribute to the height of the Inline Formatting Context.
parent().layout_floating_box(static_cast<Layout::Box const&>(*item.node), containing_block(), *m_available_space, 0, &line_builder);
}
break;

View file

@ -0,0 +1,23 @@
Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <html> at (0,0) content-size 800x166 [BFC] children: not-inline
BlockContainer <body> at (8,8) content-size 784x150 children: not-inline
BlockContainer <div.a> at (8,8) content-size 784x50 children: not-inline
BlockContainer <(anonymous)> at (8,58) content-size 784x0 children: inline
TextNode <#text>
BlockContainer <div.b> at (742,58) content-size 50x50 floating [BFC] children: not-inline
TextNode <#text>
BlockContainer <div.c> at (8,108) content-size 0x0 floating [BFC] children: not-inline
TextNode <#text>
BlockContainer <div.d> at (8,108) content-size 784x50 children: not-inline
BlockContainer <(anonymous)> at (8,158) content-size 784x0 children: inline
TextNode <#text>
ViewportPaintable (Viewport<#document>) [0,0 800x600]
PaintableWithLines (BlockContainer<HTML>) [0,0 800x166]
PaintableWithLines (BlockContainer<BODY>) [8,8 784x150]
PaintableWithLines (BlockContainer<DIV>.a) [8,8 784x50]
PaintableWithLines (BlockContainer(anonymous)) [8,58 784x0]
PaintableWithLines (BlockContainer<DIV>.b) [742,58 50x50]
PaintableWithLines (BlockContainer<DIV>.c) [8,108 0x0]
PaintableWithLines (BlockContainer<DIV>.d) [8,108 784x50]
PaintableWithLines (BlockContainer(anonymous)) [8,158 784x0]

View file

@ -0,0 +1,26 @@
<!DOCTYPE html>
<style>
.a {
background-color: red;
height: 50px;
}
.b {
background-color: green;
height: 50px;
width: 50px;
float: right;
}
.c {
clear: both;
float: left;
}
.d {
background-color: blue;
clear: right;
height: 50px;
}
</style>
<div class="a"></div>
<div class="b"></div>
<div class="c"></div>
<div class="d"></div>