LibWeb: Parse transition property as a coordinating list shorthand

We don't need all this specific logic for parsing the `transition`
property - we also now maintain `none` as such until use time which
gains us a couple extra tests

Libraries/LibWeb/CMakeLists.txt

@@ -274,7 +274,6 @@ set(SOURCES
     CSS/StyleValues/SuperellipseStyleValue.cpp
     CSS/StyleValues/TextUnderlinePositionStyleValue.cpp
     CSS/StyleValues/TransformationStyleValue.cpp
-    CSS/StyleValues/TransitionStyleValue.cpp
     CSS/StyleValues/TreeCountingFunctionStyleValue.cpp
     CSS/StyleValues/UnicodeRangeStyleValue.cpp
     CSS/StyleValues/UnresolvedStyleValue.cpp

Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp

@@ -59,7 +59,6 @@
 #include <LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TimeStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TransformationStyleValue.h>
-#include <LibWeb/CSS/StyleValues/TransitionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/URLStyleValue.h>
 #include <LibWeb/CSS/StyleValues/UnresolvedStyleValue.h>
 #include <LibWeb/Dump.h>
@@ -5082,122 +5081,38 @@ RefPtr<StyleValue const> Parser::parse_transform_origin_value(TokenStream<Compon
     return make_list(x_value.release_nonnull(), y_value.release_nonnull(), third_value.release_nonnull());
 }
 
+// https://drafts.csswg.org/css-transitions-2/#transition-shorthand-property
 RefPtr<StyleValue const> Parser::parse_transition_value(TokenStream<ComponentValue>& tokens)
 {
-    if (auto none = parse_all_as_single_keyword_value(tokens, Keyword::None))
+    // [ [ none | <single-transition-property> ] || <time> || <easing-function> || <time> || <transition-behavior-value> ]#
-        return none;
+    Vector<PropertyID> longhand_ids {
+        PropertyID::TransitionProperty,
+        PropertyID::TransitionDuration,
+        PropertyID::TransitionTimingFunction,
+        PropertyID::TransitionDelay,
+        PropertyID::TransitionBehavior
+    };
 
-    Vector<TransitionStyleValue::Transition> transitions;
+    auto parsed_value = parse_coordinating_value_list_shorthand(tokens, PropertyID::Transition, longhand_ids);
-    auto transaction = tokens.begin_transaction();
 
-    while (tokens.has_next_token()) {
+    if (!parsed_value)
-        TransitionStyleValue::Transition transition;
+        return nullptr;
-        auto time_value_count = 0;
-        bool transition_behavior_found = false;
-        while (tokens.has_next_token() && !tokens.next_token().is(Token::Type::Comma)) {
-            if (auto maybe_time = parse_time(tokens); maybe_time.has_value()) {
-                auto time = maybe_time.release_value();
-                switch (time_value_count) {
-                case 0:
-                    if (!time.is_calculated() && !property_accepts_time(PropertyID::TransitionDuration, time.value()))
-                        return nullptr;
-                    transition.duration = move(time);
-                    break;
-                case 1:
-                    if (!time.is_calculated() && !property_accepts_time(PropertyID::TransitionDelay, time.value()))
-                        return nullptr;
-                    transition.delay = move(time);
-                    break;
-                default:
-                    ErrorReporter::the().report(InvalidPropertyError {
-                        .property_name = "transition"_fly_string,
-                        .value_string = tokens.dump_string(),
-                        .description = "Contains more than two time values"_string,
-                    });
-                    return {};
-                }
-                time_value_count++;
-                continue;
-            }
 
-            if (auto easing = parse_css_value_for_property(PropertyID::TransitionTimingFunction, tokens)) {
+    // https://drafts.csswg.org/css-transitions-1/#transition-shorthand-property
-                if (transition.easing) {
+    // If there is more than one <single-transition> in the shorthand, and any of the transitions has none as the
-                    ErrorReporter::the().report(InvalidPropertyError {
+    // <single-transition-property>, then the declaration is invalid.
-                        .property_name = "transition"_fly_string,
+    auto const& transition_properties_style_value = parsed_value->as_shorthand().longhand(PropertyID::TransitionProperty);
-                        .value_string = tokens.dump_string(),
-                        .description = "Contains multiple easing values"_string,
-                    });
-                    return {};
-                }
 
-                transition.easing = easing;
+    // FIXME: This can be removed once parse_coordinating_value_list_shorthand returns a list for single values too.
-                continue;
+    if (!transition_properties_style_value->is_value_list())
-            }
+        return parsed_value;
 
-            if (!transition_behavior_found && (tokens.peek_token().is_ident("normal"sv) || tokens.peek_token().is_ident("allow-discrete"sv))) {
+    auto const& transition_properties = transition_properties_style_value->as_value_list().values();
-                transition_behavior_found = true;
-                auto ident = tokens.consume_a_token().token().ident();
-                if (ident == "allow-discrete"sv)
-                    transition.transition_behavior = TransitionBehavior::AllowDiscrete;
-                continue;
-            }
 
-            if (auto token = tokens.peek_token(); token.is_ident("all"sv)) {
+    if (transition_properties.size() > 1 && transition_properties.find_first_index_if([](auto const& transition_property) { return transition_property->to_keyword() == Keyword::None; }).has_value())
-                auto transition_keyword = parse_keyword_value(tokens);
+        return nullptr;
-                VERIFY(transition_keyword->to_keyword() == Keyword::All);
-                if (transition.property_name) {
-                    ErrorReporter::the().report(InvalidPropertyError {
-                        .property_name = "transition"_fly_string,
-                        .value_string = tokens.dump_string(),
-                        .description = "Contains multiple property identifiers"_string,
-                    });
-                    return {};
-                }
-                transition.property_name = transition_keyword.release_nonnull();
-                continue;
-            }
 
-            if (auto transition_property = parse_custom_ident_value(tokens, { { "all"sv, "none"sv } })) {
+    return parsed_value;
-                if (transition.property_name) {
-                    ErrorReporter::the().report(InvalidPropertyError {
-                        .property_name = "transition"_fly_string,
-                        .value_string = tokens.dump_string(),
-                        .description = "Contains multiple property identifiers"_string,
-                    });
-                    return {};
-                }
-
-                auto custom_ident = transition_property->custom_ident();
-                if (auto property = property_id_from_string(custom_ident); property.has_value()) {
-                    transition.property_name = CustomIdentStyleValue::create(custom_ident);
-                    continue;
-                }
-            }
-
-            ErrorReporter::the().report(InvalidPropertyError {
-                .property_name = "transition"_fly_string,
-                .value_string = tokens.dump_string(),
-                .description = MUST(String::formatted("Unexpected token \"{}\"", tokens.next_token().to_string())),
-            });
-            return {};
-        }
-
-        if (!transition.property_name)
-            transition.property_name = KeywordStyleValue::create(Keyword::All);
-
-        if (!transition.easing)
-            transition.easing = KeywordStyleValue::create(Keyword::Ease);
-
-        transitions.append(move(transition));
-
-        if (!tokens.next_token().is(Token::Type::Comma))
-            break;
-
-        tokens.discard_a_token();
-    }
-
-    transaction.commit();
-    return TransitionStyleValue::create(move(transitions));
 }
 
 RefPtr<StyleValue const> Parser::parse_list_of_time_values(PropertyID property_id, TokenStream<ComponentValue>& tokens)

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -76,7 +76,6 @@
 #include <LibWeb/CSS/StyleValues/SuperellipseStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TimeStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TransformationStyleValue.h>
-#include <LibWeb/CSS/StyleValues/TransitionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/UnresolvedStyleValue.h>
 #include <LibWeb/DOM/Attr.h>
 #include <LibWeb/DOM/Document.h>
@@ -712,42 +711,6 @@ void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_i
         return;
     }
 
-    if (property_id == CSS::PropertyID::Transition) {
-        if (value.to_keyword() == Keyword::None) {
-            // Handle `none` as a shorthand for `all 0s ease 0s`.
-            set_longhand_property(CSS::PropertyID::TransitionProperty, KeywordStyleValue::create(Keyword::All));
-            set_longhand_property(CSS::PropertyID::TransitionDuration, TimeStyleValue::create(CSS::Time::make_seconds(0)));
-            set_longhand_property(CSS::PropertyID::TransitionDelay, TimeStyleValue::create(CSS::Time::make_seconds(0)));
-            set_longhand_property(CSS::PropertyID::TransitionTimingFunction, KeywordStyleValue::create(Keyword::Ease));
-            set_longhand_property(CSS::PropertyID::TransitionBehavior, KeywordStyleValue::create(Keyword::Normal));
-        } else if (value.is_transition()) {
-            auto const& transitions = value.as_transition().transitions();
-            Array<Vector<ValueComparingNonnullRefPtr<StyleValue const>>, 5> transition_values;
-            for (auto const& transition : transitions) {
-                transition_values[0].append(*transition.property_name);
-                transition_values[1].append(transition.duration.as_style_value());
-                transition_values[2].append(transition.delay.as_style_value());
-                if (transition.easing)
-                    transition_values[3].append(*transition.easing);
-                transition_values[4].append(KeywordStyleValue::create(to_keyword(transition.transition_behavior)));
-            }
-
-            set_longhand_property(CSS::PropertyID::TransitionProperty, StyleValueList::create(move(transition_values[0]), StyleValueList::Separator::Comma));
-            set_longhand_property(CSS::PropertyID::TransitionDuration, StyleValueList::create(move(transition_values[1]), StyleValueList::Separator::Comma));
-            set_longhand_property(CSS::PropertyID::TransitionDelay, StyleValueList::create(move(transition_values[2]), StyleValueList::Separator::Comma));
-            set_longhand_property(CSS::PropertyID::TransitionTimingFunction, StyleValueList::create(move(transition_values[3]), StyleValueList::Separator::Comma));
-            set_longhand_property(CSS::PropertyID::TransitionBehavior, StyleValueList::create(move(transition_values[4]), StyleValueList::Separator::Comma));
-        } else {
-            set_longhand_property(CSS::PropertyID::TransitionProperty, value);
-            set_longhand_property(CSS::PropertyID::TransitionDuration, value);
-            set_longhand_property(CSS::PropertyID::TransitionDelay, value);
-            set_longhand_property(CSS::PropertyID::TransitionTimingFunction, value);
-            set_longhand_property(CSS::PropertyID::TransitionBehavior, value);
-        }
-
-        return;
-    }
-
     if (property_is_shorthand(property_id)) {
         // 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.

Libraries/LibWeb/CSS/StyleValues/StyleValue.cpp

@@ -70,7 +70,6 @@
 #include <LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TimeStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TransformationStyleValue.h>
-#include <LibWeb/CSS/StyleValues/TransitionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/TreeCountingFunctionStyleValue.h>
 #include <LibWeb/CSS/StyleValues/URLStyleValue.h>
 #include <LibWeb/CSS/StyleValues/UnicodeRangeStyleValue.h>

Libraries/LibWeb/CSS/StyleValues/StyleValue.h

@@ -86,7 +86,6 @@ namespace Web::CSS {
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(TextUnderlinePosition, text_underline_position, TextUnderlinePositionStyleValue) \
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(Time, time, TimeStyleValue)                                                      \
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(Transformation, transformation, TransformationStyleValue)                        \
-    __ENUMERATE_CSS_STYLE_VALUE_TYPE(Transition, transition, TransitionStyleValue)                                    \
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(TreeCountingFunction, tree_counting_function, TreeCountingFunctionStyleValue)    \
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(UnicodeRange, unicode_range, UnicodeRangeStyleValue)                             \
     __ENUMERATE_CSS_STYLE_VALUE_TYPE(Unresolved, unresolved, UnresolvedStyleValue)                                    \

Libraries/LibWeb/CSS/StyleValues/TransitionStyleValue.cpp

@@ -1,41 +0,0 @@
-/*
- * Copyright (c) 2024, Matthew Olsson <mattco@serenityos.org>.
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <LibWeb/CSS/PropertyID.h>
-#include <LibWeb/CSS/StyleValues/TransitionStyleValue.h>
-
-namespace Web::CSS {
-
-String TransitionStyleValue::to_string(SerializationMode mode) const
-{
-    StringBuilder builder;
-    bool first = true;
-    for (auto const& transition : m_transitions) {
-        if (!first)
-            builder.append(", "sv);
-        first = false;
-        builder.appendff("{} {} {} {}", transition.property_name->to_string(mode), transition.duration, transition.easing->to_string(mode), transition.delay);
-        if (transition.transition_behavior != TransitionBehavior::Normal)
-            builder.appendff(" {}", CSS::to_string(transition.transition_behavior));
-    }
-
-    return MUST(builder.to_string());
-}
-
-bool TransitionStyleValue::properties_equal(TransitionStyleValue const& other) const
-{
-    if (m_transitions.size() != other.m_transitions.size())
-        return false;
-
-    for (size_t i = 0; i < m_transitions.size(); i++) {
-        if (m_transitions[i] != other.m_transitions[i])
-            return false;
-    }
-
-    return true;
-}
-
-}

Libraries/LibWeb/CSS/StyleValues/TransitionStyleValue.h

@@ -1,51 +0,0 @@
-/*
- * Copyright (c) 2024, Matthew Olsson <mattco@serenityos.org>.
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <LibWeb/CSS/CalculatedOr.h>
-#include <LibWeb/CSS/StyleValues/CustomIdentStyleValue.h>
-#include <LibWeb/CSS/StyleValues/StyleValue.h>
-#include <LibWeb/CSS/Time.h>
-
-namespace Web::CSS {
-
-class TransitionStyleValue final : public StyleValueWithDefaultOperators<TransitionStyleValue> {
-public:
-    struct Transition {
-        ValueComparingRefPtr<StyleValue const> property_name;
-        TimeOrCalculated duration { CSS::Time::make_seconds(0.0) };
-        TimeOrCalculated delay { CSS::Time::make_seconds(0.0) };
-        ValueComparingRefPtr<StyleValue const> easing;
-        TransitionBehavior transition_behavior { TransitionBehavior::Normal };
-
-        bool operator==(Transition const&) const = default;
-    };
-
-    static ValueComparingNonnullRefPtr<TransitionStyleValue const> create(Vector<Transition> transitions)
-    {
-        return adopt_ref(*new (nothrow) TransitionStyleValue(move(transitions)));
-    }
-
-    virtual ~TransitionStyleValue() override = default;
-
-    auto const& transitions() const { return m_transitions; }
-
-    virtual String to_string(SerializationMode) const override;
-
-    bool properties_equal(TransitionStyleValue const& other) const;
-
-private:
-    explicit TransitionStyleValue(Vector<Transition> transitions)
-        : StyleValueWithDefaultOperators(Type::Transition)
-        , m_transitions(move(transitions))
-    {
-    }
-
-    Vector<Transition> m_transitions;
-};
-
-}

Libraries/LibWeb/Forward.h

@@ -384,7 +384,6 @@ class TimePercentage;
 class TimeStyleValue;
 class Transformation;
 class TransformationStyleValue;
-class TransitionStyleValue;
 class TreeCountingFunctionStyleValue;
 class UnicodeRangeStyleValue;
 class UnresolvedStyleValue;

Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/parsing/transition-computed.txt

@@ -2,13 +2,12 @@ Harness status: OK
 
 Found 10 tests
 
-9 Pass
+10 Pass
-1 Fail
 Pass	Default transition value
 Pass	Property transition value '1s'
 Pass	Property transition value 'cubic-bezier(0, -2, 1, 3)'
 Pass	Property transition value '1s -3s'
-Fail	Property transition value 'none'
+Pass	Property transition value 'none'
 Pass	Property transition value 'top'
 Pass	Property transition value '1s -3s cubic-bezier(0, -2, 1, 3) top'
 Pass	Property transition value '1s -3s, cubic-bezier(0, -2, 1, 3) top'

Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/parsing/transition-valid.txt

@@ -2,12 +2,11 @@ Harness status: OK
 
 Found 10 tests
 
-9 Pass
+10 Pass
-1 Fail
 Pass	e.style['transition'] = "1s" should set the property value
 Pass	e.style['transition'] = "cubic-bezier(0, -2, 1, 3)" should set the property value
 Pass	e.style['transition'] = "1s -3s" should set the property value
-Fail	e.style['transition'] = "none" should set the property value
+Pass	e.style['transition'] = "none" should set the property value
 Pass	e.style['transition'] = "top" should set the property value
 Pass	e.style['transition'] = "1s -3s cubic-bezier(0, -2, 1, 3) top" should set the property value
 Pass	e.style['transition'] = "1s -3s, cubic-bezier(0, -2, 1, 3) top" should set the property value