LibWeb: Implement the `font-size-adjust` property by tcl3 · Pull Request #6477 · LadybirdBrowser/ladybird

tcl3 · 2025-10-14T15:42:54Z

This PR implements the font-size-adjust property, which can be used to control how fonts are scaled based on various metrics. It can be used to ensure fallback fonts remain readable when their metrics differ significantly from the preferred font.

I believe the reason that some of the relevant reftests don't pass is that we are using font metrics that have been converted to CSSPixels, which results in noticable precision loss.

I'm also still trying to track down why I'm not getting the correct values when using from-font.

Results in a diff of +743/-7 WPT subtests in the css/css-fonts directory.

Calme1709 · 2025-10-16T01:42:28Z

+    auto first_value = parse_css_value_for_property(PropertyID::FontSizeAdjust, tokens);
+    if (!first_value)
+        return nullptr;
+    if (first_value->is_number() || first_value->is_calculated() || first_value->to_keyword() == Keyword::FromFont) {


If/when #6426 is merged this won't be enough since <number> may produce a TreeCountingFunctionStyleValue - perhaps it should instead be written more like:

if (auto maybe_number_value = parse_number_value(tokens)) { if (tokens.has_next_token()) return nullptr; transaction.commit(); return maybe_number_value ; } auto keyword_value = parse_keyword_value(tokens); if (!keyword_value) return nullptr; if (keyword_value->to_keyword() == Keyword::FromFont) { if (tokens.has_next_token()) return nullptr; transaction.commit(); return keyword_value; } auto font_metric_keyword = keyword_to_font_metric(keyword_value->to_keyword()); if (!font_metric_keyword.has_value()) return nullptr; ...

Calme1709 · 2025-10-16T01:44:44Z

+    if (first_value->is_keyword() && first_value->as_keyword().keyword() == Keyword::ExHeight)
+        return second_value;
+
+    if (second_value->is_keyword() && second_value->as_keyword().keyword() == Keyword::ExHeight)


I don't think this is ever true since we check above that second_value is NumberStyleValue, CalculatedStyleValue or from-font.

Calme1709 · 2025-10-16T01:58:30Z

    VERIFY_NOT_REACHED();
 }

+NonnullRefPtr<StyleValue const> StyleComputer::compute_font_size_adjust(NonnullRefPtr<StyleValue const> const& specified_value, ComputationContext const& computation_context)


We have already absolutized the StyleValue at the start of compute_value_of_property so we can:

Rename the parameter to absolutized_value in line with the other non-font computation functions.

Not pass a length resolution context to resolve_number in a couple places below

Update this function to just take a FontMetrics rather than a full ComputationContext

Edit: I see now that we also call this from compute_font where we don't absolutize the value.

Calme1709 · 2025-10-16T02:01:54Z


+NonnullRefPtr<StyleValue const> StyleComputer::compute_font_size_adjust(NonnullRefPtr<StyleValue const> const& specified_value, ComputationContext const& computation_context)
+{
+    if (specified_value->is_keyword() && specified_value->to_keyword() == Keyword::None)


Missing a spec link

Calme1709 · 2025-10-16T02:10:26Z

+    auto number = 0.0;
+    if (value_list[1]->is_number()) {
+        number = value_list[1]->as_number().number();
+    } else if (value.is_calculated() && value.as_calculated().resolves_to_number()) {


Have we not already converted CalculatedStyleValue to NumberStyleValue in compute_font_size_adjust?

Calme1709 · 2025-10-16T02:19:42Z

            number = *resolved_metric;
        } else {
-            font_metric_keyword = keyword;
+            VERIFY_NOT_REACHED();


Should these changes be ammended into the "LibWeb: Resolve font-size-adjust computed value" commit?

Calme1709 · 2025-10-16T02:21:58Z

        m_attempted_pseudo_class_matches = results;
    }

+    float adjusted_font_size() const { return m_adjusted_font_size; }


This doesn't respect animated values of font-size-adjust

Calme1709 · 2025-10-16T02:44:22Z

+    auto const& font_size_adjust_specified_value = style.property(PropertyID::FontSizeAdjust, ComputedProperties::WithAnimationsApplied::No);
+    style.set_property(
+        PropertyID::FontSizeAdjust,
+        compute_font_size_adjust(font_size_adjust_specified_value, font_computation_context),


It's not great that we are calling compute_font_size_adjust twice for non-animated values (once here and then again in compute_property_values), ideally we would only compute the values required for creating a length resolution context here and then do compute_font_for_style_values at NodeWithStyle::apply_style() time, avoiding this call - it would have the added benefit of respecting animated values.

font_computation_context's length resolution context is based on the parent element where I presume we should be using one based on this element.

github-actions · 2025-10-17T10:10:47Z