From 62da65099281aac8e69f2027959c041de04f7aee Mon Sep 17 00:00:00 2001 From: Callum Law Date: Thu, 12 Jun 2025 18:53:08 +1200 Subject: [PATCH] LibWeb: Don't serialize longhands if we directly serialized shorthand The spec assumes that we only store values against expanded longhands, there are however limited circumstances where we store against shorthands directly in addition to the expanded longhands. For example if the value of the shorthand is unresolved we store an UnresolvedStyleValue against the shorthand directly and a PendingSubstitutionStyleValue against each of the longhands. This commit updates the logic so that in the case we serialize a shorthand directly we should also mark it's longhands as serialized to avoid serializing them separately. This also avoids the scenario where we tried to create and serialize a ShorthandStyleValue with PendingSubstitutionStyleValue longhands, so we can remove the check and related FIXME for that. --- Libraries/LibWeb/CSS/CSSStyleProperties.cpp | 23 +++++++++++++++---- .../cssom/serialize-variable-reference.txt | 7 +++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp index b1ff78a5eb7..6ed81ab349a 100644 --- a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp @@ -1187,6 +1187,21 @@ String CSSStyleProperties::serialized() const // 2. Let already serialized be an empty array. HashTable already_serialized; + Function append_property_to_already_serialized = [&](auto property) { + already_serialized.set(property); + + // AD-HOC: The spec assumes that we only store values against expanded longhands, there are however limited + // circumstances where we store against shorthands directly in addition to the expanded longhands. For + // example if the value of the shorthand is unresolved we store an UnresolvedStyleValue against the + // shorthand directly and a PendingSubstitutionStyleValue against each of the longhands. In the case we + // serialize a shorthand directly we should also mark it's longhands as serialized to avoid serializing + // them separately. + if (property_is_shorthand(property)) { + for (auto longhand : longhands_for_shorthand(property)) + append_property_to_already_serialized(longhand); + } + }; + // NB: The spec treats custom properties the same as any other property, and expects the above loop to handle them. // However, our implementation separates them from regular properties, so we need to handle them separately here. // FIXME: Is the relative order of custom properties and regular properties supposed to be preserved? @@ -1239,9 +1254,7 @@ String CSSStyleProperties::serialized() const Vector longhands; for (auto const& longhand_declaration : m_properties) { - // FIXME: Some of the ad-hoc ShorthandStyleValue::to_string cases don't account for the possibility - // of subproperty values pending substitution, to avoid crashing we don't include those here - if (!already_serialized.contains(longhand_declaration.property_id) && shorthands_for_longhand(longhand_declaration.property_id).contains_slow(shorthand) && !longhand_declaration.value->is_pending_substitution()) + if (!already_serialized.contains(longhand_declaration.property_id) && shorthands_for_longhand(longhand_declaration.property_id).contains_slow(shorthand)) longhands.append(longhand_declaration); } @@ -1294,7 +1307,7 @@ String CSSStyleProperties::serialized() const // 11. Append the property names of all items of current longhands to already serialized. for (auto const& longhand : current_longhands) - already_serialized.set(longhand.property_id); + append_property_to_already_serialized(longhand.property_id); // 12. Continue with the steps labeled declaration loop. } @@ -1313,7 +1326,7 @@ String CSSStyleProperties::serialized() const list.append(move(serialized_declaration)); // 8. Append property to already serialized. - already_serialized.set(property); + append_property_to_already_serialized(declaration.property_id); } } diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/serialize-variable-reference.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/serialize-variable-reference.txt index bdfb3f43d45..53bbc026a54 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/serialize-variable-reference.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/serialize-variable-reference.txt @@ -2,9 +2,8 @@ Harness status: OK Found 4 tests -2 Pass -2 Fail +4 Pass Pass Longhand with variable preserves original serialization: with whitespace -Fail Shorthand with variable preserves original serialization: with whitespace +Pass Shorthand with variable preserves original serialization: with whitespace Pass Longhand with variable preserves original serialization but trims whitespace: without whitespace -Fail Shorthand with variable preserves original serialization but trims whitespace: without whitespace \ No newline at end of file +Pass Shorthand with variable preserves original serialization but trims whitespace: without whitespace \ No newline at end of file