From 381d3bf4e0012a2dd89da70d4b169e50f89ddff7 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 27 Aug 2025 15:12:17 +0100 Subject: [PATCH] LibWeb: Stop pretending text-decoration-thickness is a LengthPercentage It has two keywords: auto and from-font. from-font isn't handled properly yet, but at least we have a FIXME for it now. :^) --- Libraries/LibWeb/CSS/ComputedProperties.cpp | 22 +++++++++++++++++++++ Libraries/LibWeb/CSS/ComputedProperties.h | 1 + Libraries/LibWeb/CSS/ComputedValues.h | 13 ++++++++---- Libraries/LibWeb/Layout/Node.cpp | 3 +-- Libraries/LibWeb/Painting/PaintableBox.cpp | 21 ++++++++++++++++---- 5 files changed, 50 insertions(+), 10 deletions(-) diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index aa083be4921..30bfe37104a 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -1192,6 +1192,28 @@ TextDecorationStyle ComputedProperties::text_decoration_style() const return keyword_to_text_decoration_style(value.to_keyword()).release_value(); } +TextDecorationThickness ComputedProperties::text_decoration_thickness() const +{ + auto const& value = property(PropertyID::TextDecorationThickness); + if (value.is_keyword()) { + switch (value.to_keyword()) { + case Keyword::Auto: + return { TextDecorationThickness::Auto {} }; + case Keyword::FromFont: + return { TextDecorationThickness::FromFont {} }; + default: + VERIFY_NOT_REACHED(); + } + } + if (value.is_length()) + return TextDecorationThickness { LengthPercentage { value.as_length().length() } }; + if (value.is_percentage()) + return TextDecorationThickness { LengthPercentage { value.as_percentage().percentage() } }; + if (value.is_calculated()) + return TextDecorationThickness { LengthPercentage { value.as_calculated() } }; + VERIFY_NOT_REACHED(); +} + TextTransform ComputedProperties::text_transform() const { auto const& value = property(PropertyID::TextTransform); diff --git a/Libraries/LibWeb/CSS/ComputedProperties.h b/Libraries/LibWeb/CSS/ComputedProperties.h index 30743239860..cad4171509f 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.h +++ b/Libraries/LibWeb/CSS/ComputedProperties.h @@ -119,6 +119,7 @@ public: OutlineStyle outline_style() const; Vector text_decoration_line() const; TextDecorationStyle text_decoration_style() const; + TextDecorationThickness text_decoration_thickness() const; TextTransform text_transform() const; Vector text_shadow(Layout::Node const&) const; TextWrapMode text_wrap_mode() const; diff --git a/Libraries/LibWeb/CSS/ComputedValues.h b/Libraries/LibWeb/CSS/ComputedValues.h index 7fb8e206d8c..363b05490af 100644 --- a/Libraries/LibWeb/CSS/ComputedValues.h +++ b/Libraries/LibWeb/CSS/ComputedValues.h @@ -146,7 +146,6 @@ public: static CSS::TextJustify text_justify() { return CSS::TextJustify::Auto; } static CSS::Positioning position() { return CSS::Positioning::Static; } static CSS::TextDecorationLine text_decoration_line() { return CSS::TextDecorationLine::None; } - static CSS::Length text_decoration_thickness() { return Length::make_auto(); } static CSS::TextDecorationStyle text_decoration_style() { return CSS::TextDecorationStyle::Solid; } static CSS::TextTransform text_transform() { return CSS::TextTransform::None; } static CSS::TextOverflow text_overflow() { return CSS::TextOverflow::Clip; } @@ -427,6 +426,12 @@ struct BorderRadiusData { CSS::LengthPercentage vertical_radius { InitialValues::border_radius() }; }; +struct TextDecorationThickness { + struct Auto { }; + struct FromFont { }; + Variant value; +}; + // FIXME: Find a better place for this helper. inline Gfx::ScalingMode to_gfx_scaling_mode(CSS::ImageRendering css_value, Gfx::IntRect source, Gfx::IntRect target) { @@ -476,7 +481,7 @@ public: CSS::TextWrapMode text_wrap_mode() const { return m_inherited.text_wrap_mode; } CSS::TextRendering text_rendering() const { return m_inherited.text_rendering; } Vector const& text_decoration_line() const { return m_noninherited.text_decoration_line; } - CSS::LengthPercentage const& text_decoration_thickness() const { return m_noninherited.text_decoration_thickness; } + TextDecorationThickness const& text_decoration_thickness() const { return m_noninherited.text_decoration_thickness; } CSS::TextDecorationStyle text_decoration_style() const { return m_noninherited.text_decoration_style; } Color text_decoration_color() const { return m_noninherited.text_decoration_color; } CSS::TextTransform text_transform() const { return m_inherited.text_transform; } @@ -735,7 +740,7 @@ protected: Optional z_index; // FIXME: Store this as flags in a u8. Vector text_decoration_line { InitialValues::text_decoration_line() }; - CSS::LengthPercentage text_decoration_thickness { InitialValues::text_decoration_thickness() }; + TextDecorationThickness text_decoration_thickness { TextDecorationThickness::Auto {} }; CSS::TextDecorationStyle text_decoration_style { InitialValues::text_decoration_style() }; Color text_decoration_color { InitialValues::color() }; CSS::TextOverflow text_overflow { InitialValues::text_overflow() }; @@ -898,7 +903,7 @@ public: void set_text_align(CSS::TextAlign text_align) { m_inherited.text_align = text_align; } void set_text_justify(CSS::TextJustify text_justify) { m_inherited.text_justify = text_justify; } void set_text_decoration_line(Vector value) { m_noninherited.text_decoration_line = move(value); } - void set_text_decoration_thickness(CSS::LengthPercentage value) { m_noninherited.text_decoration_thickness = move(value); } + void set_text_decoration_thickness(TextDecorationThickness value) { m_noninherited.text_decoration_thickness = move(value); } void set_text_decoration_style(CSS::TextDecorationStyle value) { m_noninherited.text_decoration_style = value; } void set_text_decoration_color(Color value) { m_noninherited.text_decoration_color = value; } void set_text_transform(CSS::TextTransform value) { m_inherited.text_transform = value; } diff --git a/Libraries/LibWeb/Layout/Node.cpp b/Libraries/LibWeb/Layout/Node.cpp index f3432750ddd..3fb45deb80a 100644 --- a/Libraries/LibWeb/Layout/Node.cpp +++ b/Libraries/LibWeb/Layout/Node.cpp @@ -653,8 +653,7 @@ void NodeWithStyle::apply_style(CSS::ComputedProperties const& computed_style) // we just manually grab the value from `color`. This makes it dependent on `color` being // specified first, so it's far from ideal. computed_values.set_text_decoration_color(computed_style.color_or_fallback(CSS::PropertyID::TextDecorationColor, CSS::ColorResolutionContext::for_layout_node_with_style(*this), computed_values.color())); - if (auto maybe_text_decoration_thickness = computed_style.length_percentage(CSS::PropertyID::TextDecorationThickness, *this, CSS::ComputedProperties::ClampNegativeLengths::No); maybe_text_decoration_thickness.has_value()) - computed_values.set_text_decoration_thickness(maybe_text_decoration_thickness.release_value()); + computed_values.set_text_decoration_thickness(computed_style.text_decoration_thickness()); computed_values.set_webkit_text_fill_color(computed_style.color_or_fallback(CSS::PropertyID::WebkitTextFillColor, CSS::ColorResolutionContext::for_layout_node_with_style(*this), computed_values.color())); diff --git a/Libraries/LibWeb/Painting/PaintableBox.cpp b/Libraries/LibWeb/Painting/PaintableBox.cpp index 4a29be34ee4..e17d473e9ec 100644 --- a/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -1570,10 +1570,23 @@ void PaintableWithLines::resolve_paint_properties() auto const& font = fragment.m_layout_node->first_available_font(); auto const glyph_height = CSSPixels::nearest_value_for(font.pixel_size()); auto const css_line_thickness = [&] { - auto computed_thickness = text_node.computed_values().text_decoration_thickness().resolved(text_node, CSS::Length(1, CSS::Length::Type::Em).to_px(text_node)); - if (computed_thickness.is_auto()) - return max(glyph_height.scaled(0.1), 1); - return computed_thickness.to_px(*fragment.m_layout_node); + auto const& thickness = text_node.computed_values().text_decoration_thickness(); + return thickness.value.visit( + [glyph_height](CSS::TextDecorationThickness::Auto) { + // The UA chooses an appropriate thickness for text decoration lines; see below. + // https://drafts.csswg.org/css-text-decor-4/#valdef-text-decoration-thickness-auto + return max(glyph_height.scaled(0.1), 1); + }, + [glyph_height](CSS::TextDecorationThickness::FromFont) { + // If the first available font has metrics indicating a preferred underline width, use that width, + // otherwise behaves as auto. + // https://drafts.csswg.org/css-text-decor-4/#valdef-text-decoration-thickness-from-font + // FIXME: Implement this properly. + return max(glyph_height.scaled(0.1), 1); + }, + [&](CSS::LengthPercentage const& length_percentage) { + return length_percentage.resolved(text_node, CSS::Length(1, CSS::Length::Type::Em).to_px(text_node)).to_px(*fragment.m_layout_node); + }); }(); fragment.set_text_decoration_thickness(css_line_thickness);