LibWeb: Properly initialize KeyframeEffect contained in CSSTransition

This commit updates the CSSTransition constructor to:
 - Leave the KeyframeEffect start time unresolved
 - Set the KeyframeEffect start delay

Gains us 14 WPT passes but exposes one false positive in
properties-value-inherit-001.html
This commit is contained in:
Callum Law 2025-08-13 23:42:27 +12:00 committed by Sam Atkins
commit 84fb0b458f
Notes: github-actions[bot] 2025-08-18 10:19:50 +00:00
12 changed files with 57 additions and 51 deletions

View file

@ -20,11 +20,11 @@ namespace Web::CSS {
GC_DEFINE_ALLOCATOR(CSSTransition);
GC::Ref<CSSTransition> CSSTransition::start_a_transition(DOM::Element& element, Optional<PseudoElement> pseudo_element, PropertyID property_id,
size_t transition_generation, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value,
size_t transition_generation, double delay, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value,
NonnullRefPtr<StyleValue const> end_value, NonnullRefPtr<StyleValue const> reversing_adjusted_start_value, double reversing_shortening_factor)
{
auto& realm = element.realm();
return realm.create<CSSTransition>(realm, element, pseudo_element, property_id, transition_generation, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
return realm.create<CSSTransition>(realm, element, pseudo_element, property_id, transition_generation, delay, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
}
Animations::AnimationClass CSSTransition::animation_class() const
@ -76,13 +76,13 @@ Optional<int> CSSTransition::class_specific_composite_order(GC::Ref<Animations::
}
CSSTransition::CSSTransition(JS::Realm& realm, DOM::Element& element, Optional<PseudoElement> pseudo_element, PropertyID property_id, size_t transition_generation,
double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value, NonnullRefPtr<StyleValue const> end_value,
double delay, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value, NonnullRefPtr<StyleValue const> end_value,
NonnullRefPtr<StyleValue const> reversing_adjusted_start_value, double reversing_shortening_factor)
: Animations::Animation(realm)
, m_transition_property(property_id)
, m_transition_generation(transition_generation)
, m_start_time(start_time)
, m_end_time(end_time)
, m_start_time(start_time + delay)
, m_end_time(end_time + delay)
, m_start_value(move(start_value))
, m_end_value(move(end_value))
, m_reversing_adjusted_start_value(move(reversing_adjusted_start_value))
@ -95,14 +95,12 @@ CSSTransition::CSSTransition(JS::Realm& realm, DOM::Element& element, Optional<P
// when they transition out of the idle play state after being disassociated from their owning element. Transitions
// that have been disassociated from their owning element but are still idle do not have a defined composite order.
set_start_time(start_time - element.document().timeline()->current_time().value());
// Construct a KeyframesEffect for our animation
m_keyframe_effect->set_target(&element);
if (pseudo_element.has_value())
m_keyframe_effect->set_pseudo_element(Selector::PseudoElementSelector { pseudo_element.value() });
m_keyframe_effect->set_start_delay(start_time);
m_keyframe_effect->set_iteration_duration(m_end_time - start_time);
m_keyframe_effect->set_start_delay(delay);
m_keyframe_effect->set_iteration_duration(end_time - start_time);
m_keyframe_effect->set_timing_function(element.property_transition_attributes(pseudo_element, property_id)->timing_function);
auto key_frame_set = adopt_ref(*new Animations::KeyframeEffect::KeyFrameSet);

View file

@ -23,7 +23,7 @@ class CSSTransition : public Animations::Animation {
public:
static GC::Ref<CSSTransition> start_a_transition(DOM::Element&, Optional<PseudoElement>, PropertyID,
size_t transition_generation, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value,
size_t transition_generation, double delay, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value,
NonnullRefPtr<StyleValue const> end_value, NonnullRefPtr<StyleValue const> reversing_adjusted_start_value, double reversing_shortening_factor);
StringView transition_property() const { return string_from_property_id(m_transition_property); }
@ -53,7 +53,7 @@ public:
private:
CSSTransition(JS::Realm&, DOM::Element&, Optional<PseudoElement>, PropertyID, size_t transition_generation,
double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value, NonnullRefPtr<StyleValue const> end_value,
double delay, double start_time, double end_time, NonnullRefPtr<StyleValue const> start_value, NonnullRefPtr<StyleValue const> end_value,
NonnullRefPtr<StyleValue const> reversing_adjusted_start_value, double reversing_shortening_factor);
virtual void initialize(JS::Realm&) override;

View file

@ -1410,11 +1410,11 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
bool has_running_transition = existing_transition && !existing_transition->is_finished();
bool has_completed_transition = existing_transition && existing_transition->is_finished();
auto start_a_transition = [&](auto start_time, auto end_time, auto const& start_value, auto const& end_value, auto const& reversing_adjusted_start_value, auto reversing_shortening_factor) {
auto start_a_transition = [&](auto delay, auto start_time, auto end_time, auto const& start_value, auto const& end_value, auto const& reversing_adjusted_start_value, auto reversing_shortening_factor) {
dbgln_if(CSS_TRANSITIONS_DEBUG, "Starting a transition of {} from {} to {}", string_from_property_id(property_id), start_value->to_string(), end_value->to_string());
auto transition = CSSTransition::start_a_transition(element, pseudo_element, property_id,
document().transition_generation(), start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
document().transition_generation(), delay, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
// Immediately set the property's value to the transition's current value, to prevent single-frame jumps.
collect_animation_into(element, {}, as<Animations::KeyframeEffect>(*transition->effect()), new_style, AnimationRefresh::No);
};
@ -1440,8 +1440,11 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
element.remove_transition(pseudo_element, property_id);
// and start a transition whose:
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
auto delay = matching_transition_properties->delay;
// - start time is the time of the style change event plus the matching transition delay,
auto start_time = style_change_event_time + matching_transition_properties->delay;
auto start_time = style_change_event_time;
// - end time is the start time plus the matching transition duration,
auto end_time = start_time + matching_transition_properties->duration;
@ -1458,7 +1461,7 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
// - reversing shortening factor is 1.
double reversing_shortening_factor = 1;
start_a_transition(start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
start_a_transition(delay, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
}
// 2. Otherwise, if the element has a completed transition for the property
@ -1524,13 +1527,15 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
auto term_2 = 1 - existing_transition->reversing_shortening_factor();
double reversing_shortening_factor = clamp(abs(term_1 + term_2), 0.0, 1.0);
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
auto delay = (matching_transition_properties->delay >= 0
? (matching_transition_properties->delay)
: (reversing_shortening_factor * matching_transition_properties->delay));
// - start time is the time of the style change event plus:
// 1. if the matching transition delay is nonnegative, the matching transition delay, or
// 2. if the matching transition delay is negative, the product of the new transitions reversing shortening factor and the matching transition delay,
auto start_time = style_change_event_time
+ (matching_transition_properties->delay >= 0
? (matching_transition_properties->delay)
: (reversing_shortening_factor * matching_transition_properties->delay));
auto start_time = style_change_event_time;
// - end time is the start time plus the product of the matching transition duration and the new transitions reversing shortening factor,
auto end_time = start_time + (matching_transition_properties->duration * reversing_shortening_factor);
@ -1541,7 +1546,7 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
// - end value is the value of the property in the after-change style,
auto const& end_value = after_change_value;
start_a_transition(start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
start_a_transition(delay, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
}
// 4. Otherwise,
@ -1553,8 +1558,11 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
// running or completed transition for a property at once.
element.remove_transition(pseudo_element, property_id);
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
auto delay = matching_transition_properties->delay;
// - start time is the time of the style change event plus the matching transition delay,
auto start_time = style_change_event_time + matching_transition_properties->delay;
auto start_time = style_change_event_time;
// - end time is the start time plus the matching transition duration,
auto end_time = start_time + matching_transition_properties->duration;
@ -1571,7 +1579,7 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
// - reversing shortening factor is 1.
double reversing_shortening_factor = 1;
start_a_transition(start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
start_a_transition(delay, start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor);
}
}
}

View file

@ -2,10 +2,10 @@ Harness status: OK
Found 5 tests
1 Pass
4 Fail
2 Pass
3 Fail
Pass currentTime can be used to seek a CSS transition
Fail Skipping forwards through transition
Fail Skipping backwards through transition
Pass Skipping backwards through transition
Fail Setting currentTime to null on a CSS transition throws
Fail Transition reversing behavior respects currentTime and uses the transition's current position.

View file

@ -2,8 +2,8 @@ Harness status: OK
Found 10 tests
6 Pass
4 Fail
7 Pass
3 Fail
Pass After setting a transition's effect to null, it still reports the original transition property
Pass After setting a transition's effect to null, it becomes finished
Pass After setting a transition's effect to null, style is updated
@ -11,6 +11,6 @@ Fail After setting a transition's effect to null, a new transition can be starte
Fail After setting a transition's effect to null, it should be possible to interrupt that transition
Pass After setting a new keyframe effect with a shorter duration, the transition becomes finished
Pass After setting a new keyframe effect targeting different properties, the transition continues to report the original transition property
Fail After setting a new keyframe effect on a play-pending transition, the transition remains pending
Pass After setting a new keyframe effect on a play-pending transition, the transition remains pending
Pass A transition with no effect still returns the original transitionProperty
Fail A transition with a replaced effect still exhibits the regular transition reversing behavior

View file

@ -2,6 +2,7 @@ Harness status: OK
Found 2 tests
2 Fail
1 Pass
1 Fail
Fail ready promise is rejected when a transition is canceled by updating transition-property
Fail ready promise is rejected when a transition is canceled by changing the transition property to something not interpolable
Pass ready promise is rejected when a transition is canceled by changing the transition property to something not interpolable

View file

@ -2,10 +2,10 @@ Harness status: OK
Found 5 tests
3 Pass
2 Fail
4 Pass
1 Fail
Pass The start time of a newly-created transition is unresolved
Fail The start time of transitions is based on when they are generated
Pass The start time of a transition can be set
Pass The start time can be set to seek a transition
Fail Seeking a transition using start time dispatches transition events
Pass Seeking a transition using start time dispatches transition events

View file

@ -2,27 +2,27 @@ Harness status: OK
Found 30 tests
6 Pass
24 Fail
12 Pass
18 Fail
Pass Idle -> Pending or Before
Pass Idle -> Before
Fail Idle or Pending -> Active
Pass Idle or Pending -> Active
Pass Idle or Pending -> After
Fail Before -> Idle (display: none)
Fail Before -> Idle (Animation.timeline = null)
Fail Before -> Active
Fail Before -> After
Pass Before -> Active
Pass Before -> After
Fail Active -> Idle, no delay (display: none)
Fail Active -> Idle, no delay (Animation.timeline = null)
Fail Active -> Idle, with positive delay (display: none)
Fail Active -> Idle, with positive delay (Animation.timeline = null)
Fail Active -> Idle, with negative delay (display: none)
Fail Active -> Idle, with negative delay (Animation.timeline = null)
Fail Active -> Before
Fail Active -> After
Pass Active -> Before
Pass Active -> After
Pass After -> Before
Pass After -> Active
Fail Calculating the interval start and end time with negative start delay.
Pass Calculating the interval start and end time with negative start delay.
Fail Calculating the interval start and end time with negative end delay.
Fail Call Animation.cancel after canceling transition.
Fail Restart transition after canceling transition immediately

View file

@ -2,5 +2,5 @@ Harness status: OK
Found 1 tests
1 Fail
Fail Transitioned height, explicitly inherited down two DOM levels, should inherit correctly
1 Pass
Pass Transitioned height, explicitly inherited down two DOM levels, should inherit correctly

View file

@ -2,8 +2,8 @@ Harness status: OK
Found 560 tests
557 Pass
3 Fail
558 Pass
2 Fail
Pass background-color color(rgba) / values
Pass background-color color(rgba) / events
Pass background-position-x length(pt) / values
@ -560,7 +560,7 @@ Pass vertical-align percentage(%) / values
Pass vertical-align percentage(%) / events
Pass opacity number[0,1](zero-to-one) / values
Pass opacity number[0,1](zero-to-one) / events
Fail visibility visibility(keyword) / values
Pass visibility visibility(keyword) / values
Pass visibility visibility(keyword) / events
Fail z-index integer(integer) / values
Fail z-index integer(integer) / events

View file

@ -502,7 +502,7 @@ Pass text-indent length(in) / values
Pass text-indent length(in) / events
Pass text-indent percentage(%) / values
Pass text-indent percentage(%) / events
Pass text-shadow shadow(shadow) / values
Fail text-shadow shadow(shadow) / values
Fail text-shadow shadow(shadow) / events
Pass outline-color color(rgba) / values
Pass outline-color color(rgba) / events
@ -560,7 +560,7 @@ Pass vertical-align percentage(%) / values
Pass vertical-align percentage(%) / events
Pass opacity number[0,1](zero-to-one) / values
Pass opacity number[0,1](zero-to-one) / events
Fail visibility visibility(keyword) / values
Pass visibility visibility(keyword) / values
Pass visibility visibility(keyword) / events
Pass z-index integer(integer) / values
Pass z-index integer(integer) / events

View file

@ -2,8 +2,7 @@ Harness status: OK
Found 560 tests
559 Pass
1 Fail
560 Pass
Pass background-color color(rgba) / values
Pass background-color color(rgba) / events
Pass background-position-x length(pt) / values
@ -560,7 +559,7 @@ Pass vertical-align percentage(%) / values
Pass vertical-align percentage(%) / events
Pass opacity number[0,1](zero-to-one) / values
Pass opacity number[0,1](zero-to-one) / events
Fail visibility visibility(keyword) / values
Pass visibility visibility(keyword) / values
Pass visibility visibility(keyword) / events
Pass z-index integer(integer) / values
Pass z-index integer(integer) / events