LibWeb/CSS: Use PendingSubstitutionValue for unresolved shorthands

Previously, we would just assign the UnresolvedStyleValue to each
longhand, which was completely wrong but happened to work if it was a
ShorthandStyleValue (because that's basically a list of "set property X
to Y", and doesn't care which property it's the value of).

For example, the included `var-in-margin-shorthand.html` test would:
1. Set `margin-top` to `var(--a) 10px`
2. Resolve it to `margin-top: 5px 10px`
3. Reject that as invalid

What now happens is:
1. Set `margin-top` to a PendingSubstitutionValue
2. Resolve `margin` to `5px 10px`
3. Expand that out into its longhands
4. `margin-top` is `5px` 🎉

In order to support this, `for_each_property_expanding_shorthands()` now
runs the callback for the shorthand too if it's an unresolved or
pending-substitution value. This is so that we can store those in the
CascadedProperties until they can be resolved - otherwise, by the time
we want to resolve them, we don't have them any more.

`cascade_declarations()` has an unfortunate hack: it tracks, for each
declaration, which properties have already been given values, so that
it can avoid overwriting an actual value with a pending one. This is
necessary because of the unfortunate way that CSSStyleProperties holds
expanded longhands, and not just the original declarations. The spec
disagrees with itself about this, but we do need to do that expansion
for `element.style` to work correctly. This HashTable is unfortunate
but it does solve the problem until a better solution can be found.
This commit is contained in:
Sam Atkins 2025-05-07 13:01:51 +01:00
parent 398d2f9981
commit 4edafb35cd
Notes: github-actions[bot] 2025-05-14 10:47:57 +00:00
6 changed files with 164 additions and 41 deletions

View file

@ -8,6 +8,7 @@
*/
#include <AK/BinarySearch.h>
#include <AK/Bitmap.h>
#include <AK/Debug.h>
#include <AK/Error.h>
#include <AK/Find.h>
@ -16,11 +17,9 @@
#include <AK/Math.h>
#include <AK/NonnullRawPtr.h>
#include <AK/QuickSort.h>
#include <AK/TemporaryChange.h>
#include <LibGfx/Font/Font.h>
#include <LibGfx/Font/FontDatabase.h>
#include <LibGfx/Font/FontStyleMapping.h>
#include <LibGfx/Font/FontWeight.h>
#include <LibGfx/Font/Typeface.h>
#include <LibGfx/Font/WOFF/Loader.h>
#include <LibGfx/Font/WOFF2/Loader.h>
@ -60,6 +59,7 @@
#include <LibWeb/CSS/StyleValues/LengthStyleValue.h>
#include <LibWeb/CSS/StyleValues/MathDepthStyleValue.h>
#include <LibWeb/CSS/StyleValues/NumberStyleValue.h>
#include <LibWeb/CSS/StyleValues/PendingSubstitutionStyleValue.h>
#include <LibWeb/CSS/StyleValues/PercentageStyleValue.h>
#include <LibWeb/CSS/StyleValues/PositionStyleValue.h>
#include <LibWeb/CSS/StyleValues/RatioStyleValue.h>
@ -81,14 +81,12 @@
#include <LibWeb/HTML/HTMLHtmlElement.h>
#include <LibWeb/HTML/Parser/HTMLParser.h>
#include <LibWeb/HTML/Scripting/TemporaryExecutionContext.h>
#include <LibWeb/HighResolutionTime/TimeOrigin.h>
#include <LibWeb/Layout/Node.h>
#include <LibWeb/MimeSniff/MimeType.h>
#include <LibWeb/MimeSniff/Resource.h>
#include <LibWeb/Namespace.h>
#include <LibWeb/Painting/PaintableBox.h>
#include <LibWeb/Platform/FontPlugin.h>
#include <LibWeb/ReferrerPolicy/AbstractOperations.h>
#include <math.h>
#include <stdio.h>
@ -614,6 +612,21 @@ static void sort_matching_rules(Vector<MatchingRule const*>& matching_rules)
void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_id, CSSStyleValue const& value, AllowUnresolved allow_unresolved, Function<void(PropertyID, CSSStyleValue const&)> const& set_longhand_property)
{
if (property_is_shorthand(property_id) && (value.is_unresolved() || value.is_pending_substitution())) {
// If a shorthand property contains an arbitrary substitution function in its value, the longhand properties
// its associated with must instead be filled in with a special, unobservable-to-authors pending-substitution
// value that indicates the shorthand contains an arbitrary substitution function, and thus the longhands
// value cant be determined until after substituted.
// https://drafts.csswg.org/css-values-5/#pending-substitution-value
// Ensure we keep the longhand around until it can be resolved.
set_longhand_property(property_id, value);
auto pending_substitution_value = PendingSubstitutionStyleValue::create();
for (auto longhand_id : longhands_for_shorthand(property_id)) {
for_each_property_expanding_shorthands(longhand_id, pending_substitution_value, allow_unresolved, set_longhand_property);
}
return;
}
auto map_logical_property_to_real_property = [](PropertyID property_id) -> Optional<PropertyID> {
// FIXME: Honor writing-mode, direction and text-orientation.
switch (property_id) {
@ -969,12 +982,12 @@ void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_i
}
if (property_is_shorthand(property_id)) {
// ShorthandStyleValue was handled already.
// That means if we got here, that `value` must be a CSS-wide keyword, which we should apply to our longhand properties.
// ShorthandStyleValue was handled already, as were unresolved shorthands.
// That means the only values we should see are the CSS-wide keywords, or the guaranteed-invalid value.
// Both should be applied to our longhand properties.
// We don't directly call `set_longhand_property()` because the longhands might have longhands of their own.
// (eg `grid` -> `grid-template` -> `grid-template-areas` & `grid-template-rows` & `grid-template-columns`)
// Forget this requirement if we're ignoring unresolved values and the value is unresolved.
VERIFY(value.is_css_wide_keyword() || (allow_unresolved == AllowUnresolved::Yes && value.is_unresolved()));
VERIFY(value.is_css_wide_keyword() || value.is_guaranteed_invalid());
for (auto longhand : longhands_for_shorthand(property_id))
for_each_property_expanding_shorthands(longhand, value, allow_unresolved, set_longhand_property);
return;
@ -1043,7 +1056,9 @@ void StyleComputer::cascade_declarations(
Important important,
Optional<FlyString> layer_name) const
{
auto seen_properties = MUST(Bitmap::create(to_underlying(last_property_id) + 1, false));
auto cascade_style_declaration = [&](CSSStyleProperties const& declaration) {
seen_properties.fill(false);
for (auto const& property : declaration.properties()) {
if (important != property.important)
continue;
@ -1051,16 +1066,52 @@ void StyleComputer::cascade_declarations(
if (pseudo_element.has_value() && !pseudo_element_supports_property(*pseudo_element, property.property_id))
continue;
if (property.property_id == CSS::PropertyID::All) {
set_all_properties(cascaded_properties, element, pseudo_element, property.value, m_document, &declaration, cascade_origin, important, layer_name);
auto property_value = property.value;
if (property_value->is_unresolved())
property_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingParams { element.document() }, element, pseudo_element, property.property_id, property_value->as_unresolved());
if (property_value->is_guaranteed_invalid()) {
// https://drafts.csswg.org/css-values-5/#invalid-at-computed-value-time
// When substitution results in a propertys value containing the guaranteed-invalid value, this makes the
// declaration invalid at computed-value time. When this happens, the computed value is one of the
// following depending on the propertys type:
// -> The property is a non-registered custom property
// -> The property is a registered custom property with universal syntax
// FIXME: Process custom properties here?
if (false) {
// The computed value is the guaranteed-invalid value.
}
// -> Otherwise
else {
// Either the propertys inherited value or its initial value depending on whether the property is
// inherited or not, respectively, as if the propertys value had been specified as the unset keyword.
property_value = CSSKeywordValue::create(Keyword::Unset);
}
}
if (property.property_id == PropertyID::All) {
set_all_properties(cascaded_properties, element, pseudo_element, property_value, m_document, &declaration, cascade_origin, important, layer_name);
continue;
}
auto property_value = property.value;
if (property.value->is_unresolved())
property_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingParams { document() }, element, pseudo_element, property.property_id, property.value->as_unresolved());
if (!property_value->is_unresolved())
set_property_expanding_shorthands(cascaded_properties, property.property_id, property_value, &declaration, cascade_origin, important, layer_name);
// NOTE: This is a duplicate of set_property_expanding_shorthands() with some extra checks.
for_each_property_expanding_shorthands(property.property_id, property_value, AllowUnresolved::No, [&](PropertyID longhand_id, CSSStyleValue const& longhand_value) {
// If we're a PSV that's already been seen, that should mean that our shorthand already got
// resolved and gave us a value, so we don't want to overwrite it with a PSV.
if (seen_properties.get(to_underlying(longhand_id)) && property_value->is_pending_substitution())
return;
seen_properties.set(to_underlying(longhand_id), true);
if (longhand_value.is_revert()) {
cascaded_properties.revert_property(longhand_id, important, cascade_origin);
} else if (longhand_value.is_revert_layer()) {
cascaded_properties.revert_layer_property(longhand_id, important, layer_name);
} else {
cascaded_properties.set_property(longhand_id, longhand_value, important, cascade_origin, layer_name, declaration);
}
});
}
};
@ -1155,27 +1206,46 @@ void StyleComputer::collect_animation_into(DOM::Element& element, Optional<CSS::
dbgln("Animation {} contains {} properties to interpolate, progress = {}%", animation->id(), valid_properties, progress_in_keyframe * 100);
}
for (auto const& it : keyframe_values.properties) {
auto resolve_property = [&](auto& property) {
return property.visit(
// FIXME: Follow https://drafts.csswg.org/web-animations-1/#ref-for-computed-keyframes in whatever the right place is.
auto compute_keyframe_values = [refresh, &computed_properties, &element, &pseudo_element](auto const& keyframe_values) {
HashMap<PropertyID, RefPtr<CSSStyleValue const>> result;
for (auto const& [property_id, value] : keyframe_values.properties) {
auto style_value = value.visit(
[&](Animations::KeyframeEffect::KeyFrameSet::UseInitial) -> RefPtr<CSSStyleValue const> {
if (refresh == AnimationRefresh::Yes)
return {};
return computed_properties.property(it.key);
if (property_is_shorthand(property_id))
return {};
return computed_properties.property(property_id);
},
[&](RefPtr<CSSStyleValue const> value) -> RefPtr<CSSStyleValue const> {
if (value->is_revert() || value->is_revert_layer())
return computed_properties.property(it.key);
if (value->is_unresolved())
return Parser::Parser::resolve_unresolved_style_value(Parser::ParsingParams { element.document() }, element, pseudo_element, it.key, value->as_unresolved());
return value;
});
};
auto resolved_start_property = resolve_property(it.value);
if (!style_value) {
result.set(property_id, nullptr);
continue;
}
auto const& end_property = keyframe_end_values.properties.get(it.key);
if (!end_property.has_value()) {
if (style_value->is_revert() || style_value->is_revert_layer())
style_value = computed_properties.property(property_id);
if (style_value->is_unresolved())
style_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingParams { element.document() }, element, pseudo_element, property_id, style_value->as_unresolved());
for_each_property_expanding_shorthands(property_id, *style_value, AllowUnresolved::No, [&result](PropertyID id, CSSStyleValue const& longhand_value) {
result.set(id, { longhand_value });
});
}
return result;
};
HashMap<PropertyID, RefPtr<CSSStyleValue const>> computed_start_values = compute_keyframe_values(keyframe_values);
HashMap<PropertyID, RefPtr<CSSStyleValue const>> computed_end_values = compute_keyframe_values(keyframe_end_values);
for (auto const& it : computed_start_values) {
auto resolved_start_property = it.value;
RefPtr resolved_end_property = computed_end_values.get(it.key).value_or(nullptr);
if (!resolved_end_property) {
if (resolved_start_property) {
computed_properties.set_animated_property(it.key, *resolved_start_property);
dbgln_if(LIBWEB_CSS_ANIMATION_DEBUG, "No end property for property {}, using {}", string_from_property_id(it.key), resolved_start_property->to_string(CSSStyleValue::SerializationMode::Normal));
@ -1183,8 +1253,6 @@ void StyleComputer::collect_animation_into(DOM::Element& element, Optional<CSS::
continue;
}
auto resolved_end_property = resolve_property(end_property.value());
if (resolved_end_property && !resolved_start_property)
resolved_start_property = property_initial_value(it.key);