diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index a021e4ea1bc..ed22ac6fff8 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -92,6 +92,14 @@ bool ComputedProperties::is_animated_property_inherited(PropertyID property_id) return m_animated_property_inherited[n / 8] & (1 << (n % 8)); } +bool ComputedProperties::is_animated_property_result_of_transition(PropertyID property_id) const +{ + VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id); + + size_t n = to_underlying(property_id) - to_underlying(first_longhand_property_id); + return m_animated_property_result_of_transition[n / 8] & (1 << (n % 8)); +} + void ComputedProperties::set_property_inherited(PropertyID property_id, Inherited inherited) { VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id); @@ -114,6 +122,17 @@ void ComputedProperties::set_animated_property_inherited(PropertyID property_id, m_animated_property_inherited[n / 8] &= ~(1 << (n % 8)); } +void ComputedProperties::set_animated_property_result_of_transition(PropertyID property_id, AnimatedPropertyResultOfTransition animated_value_result_of_transition) +{ + VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id); + + size_t n = to_underlying(property_id) - to_underlying(first_longhand_property_id); + if (animated_value_result_of_transition == AnimatedPropertyResultOfTransition::Yes) + m_animated_property_result_of_transition[n / 8] |= (1 << (n % 8)); + else + m_animated_property_result_of_transition[n / 8] &= ~(1 << (n % 8)); +} + void ComputedProperties::set_property(PropertyID id, NonnullRefPtr value, Inherited inherited, Important important) { VERIFY(id >= first_longhand_property_id && id <= last_longhand_property_id); @@ -149,10 +168,11 @@ void ComputedProperties::set_display_before_box_type_transformation(Display valu m_display_before_box_type_transformation = value; } -void ComputedProperties::set_animated_property(PropertyID id, NonnullRefPtr value, Inherited inherited) +void ComputedProperties::set_animated_property(PropertyID id, NonnullRefPtr value, AnimatedPropertyResultOfTransition animated_property_result_of_transition, Inherited inherited) { m_animated_property_values.set(id, move(value)); set_animated_property_inherited(id, inherited); + set_animated_property_result_of_transition(id, animated_property_result_of_transition); } void ComputedProperties::remove_animated_property(PropertyID id) @@ -172,8 +192,8 @@ StyleValue const& ComputedProperties::property(PropertyID property_id, WithAnima { VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id); - // Important properties override animated properties - if (!is_property_important(property_id) && return_animated_value == WithAnimationsApplied::Yes) { + // Important properties override animated but not transitioned properties + if ((!is_property_important(property_id) || is_animated_property_result_of_transition(property_id)) && return_animated_value == WithAnimationsApplied::Yes) { if (auto animated_value = m_animated_property_values.get(property_id); animated_value.has_value()) return *animated_value.value(); } diff --git a/Libraries/LibWeb/CSS/ComputedProperties.h b/Libraries/LibWeb/CSS/ComputedProperties.h index 60d47fb2501..faafa7d97e5 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.h +++ b/Libraries/LibWeb/CSS/ComputedProperties.h @@ -26,6 +26,11 @@ namespace Web::CSS { +enum class AnimatedPropertyResultOfTransition : u8 { + No, + Yes +}; + class WEB_API ComputedProperties final : public JS::Cell { GC_CELL(ComputedProperties, JS::Cell); GC_DECLARE_ALLOCATOR(ComputedProperties); @@ -55,13 +60,15 @@ public: bool is_property_important(PropertyID property_id) const; bool is_property_inherited(PropertyID property_id) const; bool is_animated_property_inherited(PropertyID property_id) const; + bool is_animated_property_result_of_transition(PropertyID property_id) const; void set_property_important(PropertyID, Important); void set_property_inherited(PropertyID, Inherited); void set_animated_property_inherited(PropertyID, Inherited); + void set_animated_property_result_of_transition(PropertyID, AnimatedPropertyResultOfTransition); void set_property(PropertyID, NonnullRefPtr value, Inherited = Inherited::No, Important = Important::No); void set_property_without_modifying_flags(PropertyID, NonnullRefPtr value); - void set_animated_property(PropertyID, NonnullRefPtr value, Inherited = Inherited::No); + void set_animated_property(PropertyID, NonnullRefPtr value, AnimatedPropertyResultOfTransition, Inherited = Inherited::No); void remove_animated_property(PropertyID); enum class WithAnimationsApplied { No, @@ -292,6 +299,7 @@ private: Array m_property_important {}; Array m_property_inherited {}; Array m_animated_property_inherited {}; + Array m_animated_property_result_of_transition {}; HashMap> m_animated_property_values; diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index 8cdbe0edee4..69945eeb806 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -952,7 +952,7 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element auto const& specified_value_with_css_wide_keywords_applied = [&]() -> StyleValue const& { if (longhand_value.is_inherit() || (longhand_value.is_unset() && is_inherited_property(longhand_id))) { if (auto inherited_animated_value = get_animated_inherit_value(longhand_id, abstract_element); inherited_animated_value.has_value()) - return inherited_animated_value.value(); + return inherited_animated_value->value; return get_non_animated_inherit_value(longhand_id, abstract_element); } @@ -1072,6 +1072,8 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element VERIFY_NOT_REACHED(); }; + auto is_result_of_transition = animation->is_css_transition() ? AnimatedPropertyResultOfTransition::Yes : AnimatedPropertyResultOfTransition::No; + auto start_composite_operation = to_composite_operation(keyframe_values.composite); auto end_composite_operation = to_composite_operation(keyframe_end_values.composite); @@ -1081,7 +1083,7 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element if (!resolved_end_property) { if (resolved_start_property) { - computed_properties.set_animated_property(it.key, *resolved_start_property); + computed_properties.set_animated_property(it.key, *resolved_start_property, is_result_of_transition); dbgln_if(LIBWEB_CSS_ANIMATION_DEBUG, "No end property for property {}, using {}", string_from_property_id(it.key), resolved_start_property->to_string(SerializationMode::Normal)); } continue; @@ -1096,7 +1098,9 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element auto start = resolved_start_property.release_nonnull(); auto end = resolved_end_property.release_nonnull(); - if (computed_properties.is_property_important(it.key)) { + // OPTIMIZATION: Values resulting from animations other than CSS transitions are overriden by important + // properties so there's no need to calculate them + if (!animation->is_css_transition() && computed_properties.is_property_important(it.key)) { continue; } @@ -1109,11 +1113,11 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element if (auto next_value = interpolate_property(*effect->target(), it.key, *start, *end, progress_in_keyframe, AllowDiscrete::Yes)) { dbgln_if(LIBWEB_CSS_ANIMATION_DEBUG, "Interpolated value for property {} at {}: {} -> {} = {}", string_from_property_id(it.key), progress_in_keyframe, start->to_string(SerializationMode::Normal), end->to_string(SerializationMode::Normal), next_value->to_string(SerializationMode::Normal)); - computed_properties.set_animated_property(it.key, *next_value); + computed_properties.set_animated_property(it.key, *next_value, is_result_of_transition); } else { // If interpolate_property() fails, the element should not be rendered dbgln_if(LIBWEB_CSS_ANIMATION_DEBUG, "Interpolated value for property {} at {}: {} -> {} is invalid", string_from_property_id(it.key), progress_in_keyframe, start->to_string(SerializationMode::Normal), end->to_string(SerializationMode::Normal)); - computed_properties.set_animated_property(PropertyID::Visibility, KeywordStyleValue::create(Keyword::Hidden)); + computed_properties.set_animated_property(PropertyID::Visibility, KeywordStyleValue::create(Keyword::Hidden), is_result_of_transition); } } } @@ -1608,7 +1612,7 @@ NonnullRefPtr StyleComputer::get_non_animated_inherit_value(Pr return parent_element->computed_properties()->property(property_id, ComputedProperties::WithAnimationsApplied::No); } -Optional> StyleComputer::get_animated_inherit_value(PropertyID property_id, DOM::AbstractElement abstract_element) +Optional StyleComputer::get_animated_inherit_value(PropertyID property_id, DOM::AbstractElement abstract_element) { auto parent_element = abstract_element.element_to_inherit_style_from(); @@ -1616,7 +1620,12 @@ Optional> StyleComputer::get_animated_inherit_va return {}; if (auto animated_value = parent_element->computed_properties()->animated_property_values().get(property_id); animated_value.has_value()) - return *animated_value.value(); + return AnimatedInheritValue { + .value = *animated_value.value(), + .is_result_of_transition = parent_element->computed_properties()->is_animated_property_result_of_transition(property_id) + ? AnimatedPropertyResultOfTransition::Yes + : AnimatedPropertyResultOfTransition::No + }; return {}; } @@ -2525,7 +2534,6 @@ GC::Ref StyleComputer::compute_properties(DOM::AbstractEleme auto property_id = static_cast(i); auto value = cascaded_properties.property(property_id); auto inherited = ComputedProperties::Inherited::No; - Optional> animated_value; // NOTE: We've already handled font-size above. if (property_id == PropertyID::FontSize && !value && new_font_size) @@ -2547,17 +2555,17 @@ GC::Ref StyleComputer::compute_properties(DOM::AbstractEleme // FIXME: Logical properties should inherit from their parent's equivalent unmapped logical property. if (should_inherit) { - value = get_non_animated_inherit_value(property_id, abstract_element); - animated_value = get_animated_inherit_value(property_id, abstract_element); inherited = ComputedProperties::Inherited::Yes; + value = get_non_animated_inherit_value(property_id, abstract_element); + + if (auto animated_value = get_animated_inherit_value(property_id, abstract_element); animated_value.has_value()) + computed_style->set_animated_property(property_id, animated_value->value, animated_value->is_result_of_transition, ComputedProperties::Inherited::Yes); } if (!value || value->is_initial() || value->is_unset()) value = property_initial_value(property_id); computed_style->set_property(property_id, value.release_nonnull(), inherited, cascaded_properties.is_property_important(property_id) ? Important::Yes : Important::No); - if (animated_value.has_value()) - computed_style->set_animated_property(property_id, animated_value.value(), inherited); } // Compute the value of custom properties diff --git a/Libraries/LibWeb/CSS/StyleComputer.h b/Libraries/LibWeb/CSS/StyleComputer.h index 690018a3c14..9eb11b5b286 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Libraries/LibWeb/CSS/StyleComputer.h @@ -110,7 +110,11 @@ class WEB_API StyleComputer final : public GC::Cell { public: static void for_each_property_expanding_shorthands(PropertyID, StyleValue const&, Function const& set_longhand_property); static NonnullRefPtr get_non_animated_inherit_value(PropertyID, DOM::AbstractElement); - static Optional> get_animated_inherit_value(PropertyID, DOM::AbstractElement); + struct AnimatedInheritValue { + NonnullRefPtr value; + AnimatedPropertyResultOfTransition is_result_of_transition; + }; + static Optional get_animated_inherit_value(PropertyID, DOM::AbstractElement); static Optional user_agent_style_sheet_source(StringView name); diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 63d27e9f88a..c814dcf1f87 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -860,7 +860,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_inherited_style() if (computed_properties->is_animated_property_inherited(property_id) || !computed_properties->animated_property_values().contains(property_id)) { if (auto new_animated_value = CSS::StyleComputer::get_animated_inherit_value(property_id, { *this }); new_animated_value.has_value()) - computed_properties->set_animated_property(property_id, new_animated_value.value(), CSS::ComputedProperties::Inherited::Yes); + computed_properties->set_animated_property(property_id, new_animated_value->value, new_animated_value->is_result_of_transition, CSS::ComputedProperties::Inherited::Yes); else if (computed_properties->animated_property_values().contains(property_id)) computed_properties->remove_animated_property(property_id); } diff --git a/Libraries/LibWeb/Forward.h b/Libraries/LibWeb/Forward.h index a540babc052..0f0920062eb 100644 --- a/Libraries/LibWeb/Forward.h +++ b/Libraries/LibWeb/Forward.h @@ -400,6 +400,7 @@ enum class MediaFeatureID : u8; enum class PropertyID : u16; enum class PaintOrder : u8; enum class ValueType : u8; +enum class AnimatedPropertyResultOfTransition : u8; struct BackgroundLayerData; struct CalculationContext; diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-important.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-important.txt new file mode 100644 index 00000000000..77a513caee4 --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-important.txt @@ -0,0 +1,6 @@ +Harness status: OK + +Found 1 tests + +1 Pass +Pass !important should not override transition \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-important.html b/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-important.html new file mode 100644 index 00000000000..6faa74dee15 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-important.html @@ -0,0 +1,24 @@ + +CSS Transitions Test: !important property + + + + +
+