From 50d72f0d8c84bc3238c239b73103d57c7e8aaf47 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 3 Sep 2023 14:10:46 +0200 Subject: [PATCH] LibWeb: Handle case where abspos flex child position depends on height There's a particularly awkward case where the static position of an abspos child of a flex container is dependent on its height. This can happen when `align-items: center` is in effect, as we have to adjust the abspos child's Y position by half of its height. This patch solves the issue by reordering operations in the abspos height resolution algorithm, to make sure that height is resolved before the static position is calculated. --- .../abspos-flex-child-with-auto-height.txt | 12 ++++++ .../abspos-flex-child-with-auto-height.html | 18 +++++++++ .../LibWeb/Layout/FormattingContext.cpp | 40 +++++++++++++------ 3 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/flex/abspos-flex-child-with-auto-height.txt create mode 100644 Tests/LibWeb/Layout/input/flex/abspos-flex-child-with-auto-height.html diff --git a/Tests/LibWeb/Layout/expected/flex/abspos-flex-child-with-auto-height.txt b/Tests/LibWeb/Layout/expected/flex/abspos-flex-child-with-auto-height.txt new file mode 100644 index 0000000000..d29154a5c1 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/flex/abspos-flex-child-with-auto-height.txt @@ -0,0 +1,12 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x16 [BFC] children: not-inline + Box at (8,8) content-size 784x0 positioned flex-container(row) [FFC] children: not-inline + BlockContainer at (8,-42) content-size 100x100 positioned [BFC] children: not-inline + BlockContainer at (8,-42) content-size 100x200 children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] overflow: [0,-42 800x642] + PaintableWithLines (BlockContainer) [0,0 800x16] overflow: [0,-42 800x200] + PaintableBox (Box) [8,8 784x0] overflow: [8,-42 100x200] + PaintableWithLines (BlockContainer
.abspos) [8,-42 100x100] overflow: [8,-42 100x200] + PaintableWithLines (BlockContainer
.green) [8,-42 100x200] diff --git a/Tests/LibWeb/Layout/input/flex/abspos-flex-child-with-auto-height.html b/Tests/LibWeb/Layout/input/flex/abspos-flex-child-with-auto-height.html new file mode 100644 index 0000000000..e07a2d8228 --- /dev/null +++ b/Tests/LibWeb/Layout/input/flex/abspos-flex-child-with-auto-height.html @@ -0,0 +1,18 @@ +
diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp index cfd3f8c777..6e294513e7 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp @@ -823,6 +823,21 @@ void FormattingContext::compute_height_for_absolutely_positioned_non_replaced_el // In the before pass, if it turns out we need the automatic height of the box, we abort these steps. // This allows the box to retain an indefinite height from the perspective of inside layout. + auto apply_min_max_height_constraints = [this, &box, &available_space](CSSPixels unconstrained_height) -> CSSPixels { + auto const& computed_min_height = box.computed_values().min_height(); + auto const& computed_max_height = box.computed_values().max_height(); + auto constrained_height = unconstrained_height; + if (!computed_max_height.is_none()) { + auto inner_max_height = calculate_inner_height(box, available_space.height, computed_max_height); + constrained_height = min(constrained_height, inner_max_height.to_px(box)); + } + if (!computed_min_height.is_auto()) { + auto inner_min_height = calculate_inner_height(box, available_space.height, computed_min_height); + constrained_height = max(constrained_height, inner_min_height.to_px(box)); + } + return constrained_height; + }; + auto margin_top = box.computed_values().margin().top(); auto margin_bottom = box.computed_values().margin().bottom(); auto top = box.computed_values().inset().top(); @@ -888,14 +903,22 @@ void FormattingContext::compute_height_for_absolutely_positioned_non_replaced_el margin_bottom = CSS::Length::make_px(0); // then set top to the static position, - auto static_position = calculate_static_position(box); - top = CSS::Length::make_px(static_position.y()); - // and finally apply rule number three below. + + // NOTE: We actually perform these two steps in the opposite order, + // because the static position may depend on the height of the box (due to alignment properties). + auto maybe_height = compute_auto_height_for_absolutely_positioned_element(box, available_space, before_or_after_inside_layout); if (!maybe_height.has_value()) return; height = CSS::Size::make_px(maybe_height.value()); + + auto constrained_height = apply_min_max_height_constraints(maybe_height.value()); + m_state.get_mutable(box).set_content_height(constrained_height); + + auto static_position = calculate_static_position(box); + top = CSS::Length::make_px(static_position.y()); + solve_for_bottom(); } @@ -993,17 +1016,8 @@ void FormattingContext::compute_height_for_absolutely_positioned_non_replaced_el } else { used_height = calculate_inner_height(box, available_space.height, height).to_px(box); } - auto const& computed_min_height = box.computed_values().min_height(); - auto const& computed_max_height = box.computed_values().max_height(); - if (!computed_max_height.is_none()) { - auto inner_max_height = calculate_inner_height(box, available_space.height, computed_max_height); - used_height = min(used_height, inner_max_height.to_px(box)); - } - if (!computed_min_height.is_auto()) { - auto inner_min_height = calculate_inner_height(box, available_space.height, computed_min_height); - used_height = max(used_height, inner_min_height.to_px(box)); - } + used_height = apply_min_max_height_constraints(used_height); // NOTE: The following is not directly part of any spec, but this is where we resolve // the final used values for vertical margin/border/padding.