LibWeb: Simplify serialization of coordinating value list shorthands

We know that longhand values are always `StyleValueList`s

Introduces a new fail in the `view-timeline-shorthand.html` WPT test
but this is because the test is incorrect and we now correctly don't
contract when `view-timeline-inset` has dissimilar cardinality. See
web-platform-tests/wpt#56181
This commit is contained in:
Callum Law 2025-11-29 15:25:07 +13:00 committed by Sam Atkins
parent 11a9b5a67b
commit e8ba1b4fdf
Notes: github-actions[bot] 2025-12-01 10:18:32 +00:00
2 changed files with 14 additions and 48 deletions

View file

@ -70,24 +70,16 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
return ""_string;
}
// FIXME: This is required as parse_comma_separated_value_list() returns a single value directly instead of a list if there's only one.
auto const style_value_as_value_list = [&](RefPtr<StyleValue const> value) -> StyleValueVector {
if (value->is_value_list())
return value->as_value_list().values();
return { value.release_nonnull() };
};
auto const coordinating_value_list_shorthand_to_string = [&](StringView entry_when_all_longhands_initial, Vector<PropertyID> const& required_longhands = {}, Vector<PropertyID> const& reset_only_longhands = {}) {
for (auto reset_only_longhand : reset_only_longhands) {
if (!longhand(reset_only_longhand)->equals(property_initial_value(reset_only_longhand)))
return ""_string;
}
auto entry_count = style_value_as_value_list(longhand(m_properties.sub_properties[0])).size();
auto entry_count = longhand(m_properties.sub_properties[0])->as_value_list().size();
// If we don't have the same number of values for each non-reset-only longhand, we can't serialize this shorthand.
if (any_of(m_properties.sub_properties, [&](auto longhand_id) { return !reset_only_longhands.contains_slow(longhand_id) && style_value_as_value_list(longhand(longhand_id)).size() != entry_count; }))
if (any_of(m_properties.sub_properties, [&](auto longhand_id) { return !reset_only_longhands.contains_slow(longhand_id) && longhand(longhand_id)->as_value_list().size() != entry_count; }))
return ""_string;
// We should serialize a longhand if it is not a reset-only longhand and one of the following is true:
@ -103,9 +95,9 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
if (required_longhands.contains_slow(longhand_id))
return true;
auto longhand_value = style_value_as_value_list(longhand(longhand_id))[entry_index];
auto longhand_value = longhand(longhand_id)->as_value_list().values()[entry_index];
if (!longhand_value->equals(style_value_as_value_list(property_initial_value(longhand_id))[0]))
if (!longhand_value->equals(property_initial_value(longhand_id)->as_value_list().values()[0]))
return true;
for (size_t other_longhand_index = longhand_index + 1; other_longhand_index < m_properties.sub_properties.size(); other_longhand_index++) {
@ -114,10 +106,10 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
if (reset_only_longhands.contains_slow(other_longhand_id))
continue;
auto other_longhand_value = style_value_as_value_list(longhand(other_longhand_id))[entry_index];
auto other_longhand_value = longhand(other_longhand_id)->as_value_list().values()[entry_index];
// FIXME: This should really account for the other longhand being included in the serialization for any reason, not just because it is not the initial value.
if (other_longhand_value->equals(style_value_as_value_list(property_initial_value(other_longhand_id))[0]))
if (other_longhand_value->equals(property_initial_value(other_longhand_id)->as_value_list().values()[0]))
continue;
if (parse_css_value(Parser::ParsingParams {}, other_longhand_value->to_string(mode), longhand_id))
@ -140,7 +132,7 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
if (!builder.is_empty() && !first)
builder.append(' ');
auto longhand_value = style_value_as_value_list(longhand(longhand_id))[entry_index];
auto longhand_value = longhand(longhand_id)->as_value_list().values()[entry_index];
builder.append(longhand_value->to_string(mode));
first = false;
@ -862,36 +854,10 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const
}
case PropertyID::Transition:
return coordinating_value_list_shorthand_to_string("all"sv);
case PropertyID::ViewTimeline: {
// FIXME: We can use coordinating_value_list_shorthand_to_string function once parse_comma_separated_value_list
// always returns a list, currently it doesn't properly handle the fact that the entries for
// view-timeline-inset are themselves StyleValueLists
StringBuilder builder;
auto const& name_values = style_value_as_value_list(longhand(PropertyID::ViewTimelineName));
auto const& axis_values = style_value_as_value_list(longhand(PropertyID::ViewTimelineAxis));
auto const& inset_values = style_value_as_value_list(longhand(PropertyID::ViewTimelineInset));
if (name_values.size() != axis_values.size())
return ""_string;
for (size_t i = 0; i < name_values.size(); i++) {
if (!builder.is_empty())
builder.append(", "sv);
builder.append(name_values[i]->to_string(mode));
if (axis_values[i]->to_keyword() != Keyword::Block)
builder.appendff(" {}", axis_values[i]->to_string(mode));
auto stringified_inset = inset_values[i]->to_string(mode);
if (stringified_inset != "auto"sv)
builder.appendff(" {}", stringified_inset);
}
return builder.to_string_without_validation();
}
case PropertyID::ViewTimeline:
// NB: We don't need to specify a value to use when the entry is empty as all values are initial since
// view-timeline-name is always included
return coordinating_value_list_shorthand_to_string(""sv, { PropertyID::ViewTimelineName });
case PropertyID::WhiteSpace: {
auto white_space_collapse_property = longhand(PropertyID::WhiteSpaceCollapse);
auto text_wrap_mode_property = longhand(PropertyID::TextWrapMode);