From 75e87c32f29c554c9814c5a11a9c3450280feb0b Mon Sep 17 00:00:00 2001 From: Andi Gallo Date: Sun, 11 Jun 2023 12:04:01 +0000 Subject: [PATCH] LibWeb: Fix table width algorithm when available space is a constraint Handle available space more carefully when computing a table width, in order to avoid creating a definite infinite width when available space width is max-content, as it's the case in calculate_max_content_width. The constraint is thus correctly propagated by the time we cache the computed value, which was previously rejected by the hash function due to being definite but infinite instead of max-content. --- .../Libraries/LibWeb/Layout/AvailableSpace.cpp | 1 + .../LibWeb/Layout/BlockFormattingContext.cpp | 9 ++++++++- Userland/Libraries/LibWeb/Layout/LayoutState.cpp | 16 ++++++++++++++++ Userland/Libraries/LibWeb/Layout/LayoutState.h | 2 ++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibWeb/Layout/AvailableSpace.cpp b/Userland/Libraries/LibWeb/Layout/AvailableSpace.cpp index 3cd4004f84..1622d5db04 100644 --- a/Userland/Libraries/LibWeb/Layout/AvailableSpace.cpp +++ b/Userland/Libraries/LibWeb/Layout/AvailableSpace.cpp @@ -11,6 +11,7 @@ namespace Web::Layout { AvailableSize AvailableSize::make_definite(CSSPixels value) { + VERIFY(isfinite(value.value())); return AvailableSize { Type::Definite, value }; } diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index 321457917f..c8273a01d0 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -415,7 +415,14 @@ CSSPixels BlockFormattingContext::compute_table_box_width_inside_table_wrapper(B auto available_width = width_of_containing_block - margin_left.to_px(box) - margin_right.to_px(box); LayoutState throwaway_state(&m_state); - throwaway_state.get_mutable(box).set_content_width(available_width); + if (available_space.width.is_definite()) + throwaway_state.get_mutable(box).set_content_width(available_width); + else if (available_space.width.is_min_content()) + throwaway_state.get_mutable(box).set_min_content_width(); + else { + VERIFY(available_space.width.is_max_content()); + throwaway_state.get_mutable(box).set_max_content_width(); + } auto context = create_independent_formatting_context_if_needed(throwaway_state, box); VERIFY(context); context->run(box, LayoutMode::IntrinsicSizing, m_state.get(box).available_inner_space_or_constraints_from(available_space)); diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp index 0149c863eb..d517d7437b 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp @@ -297,6 +297,7 @@ void LayoutState::UsedValues::set_node(NodeWithStyleAndBoxModelMetrics& node, Us void LayoutState::UsedValues::set_content_width(CSSPixels width) { + VERIFY(isfinite(width.value())); if (width < 0) { // Negative widths are not allowed in CSS. We have a bug somewhere! Clamp to 0 to avoid doing too much damage. dbgln_if(LIBWEB_CSS_DEBUG, "FIXME: Layout calculated a negative width for {}: {}", m_node->debug_description(), width); @@ -308,6 +309,7 @@ void LayoutState::UsedValues::set_content_width(CSSPixels width) void LayoutState::UsedValues::set_content_height(CSSPixels height) { + VERIFY(isfinite(height.value())); if (height < 0) { // Negative heights are not allowed in CSS. We have a bug somewhere! Clamp to 0 to avoid doing too much damage. dbgln_if(LIBWEB_CSS_DEBUG, "FIXME: Layout calculated a negative height for {}: {}", m_node->debug_description(), height); @@ -387,4 +389,18 @@ void LayoutState::UsedValues::set_indefinite_content_height() m_has_definite_height = false; } +void LayoutState::UsedValues::set_min_content_width() +{ + width_constraint = SizeConstraint::MinContent; + m_content_width = 0; + m_has_definite_height = false; +} + +void LayoutState::UsedValues::set_max_content_width() +{ + width_constraint = SizeConstraint::MaxContent; + m_content_width = INFINITY; + m_has_definite_height = false; +} + } diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.h b/Userland/Libraries/LibWeb/Layout/LayoutState.h index 47814383f9..2b56e495b4 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.h +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.h @@ -51,6 +51,8 @@ struct LayoutState { void set_indefinite_content_width(); void set_indefinite_content_height(); + void set_min_content_width(); + void set_max_content_width(); // NOTE: These are used by FlexFormattingContext to assign a temporary main size to items // early on, so that descendants have something to resolve percentages against.