Auto merge of #26074 - jdm:transition-fix, r=SimonSapin

Avoid infinitely looping CSS transitions.

This change addresses the long-standing issue of CSS transitions not ending appropriately. It does not fundamentally change the way we process transitions/animations.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20379
- [x] There are tests for these changes
This commit is contained in:
bors-servo 2020-04-01 15:19:15 -04:00 committed by GitHub
commit 516279e24f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 61 additions and 20 deletions

View file

@ -110,6 +110,7 @@ pub fn update_animation_state<E>(
.unwrap(); .unwrap();
} }
debug!("expiring animation for {:?}", running_animation);
expired_animations expired_animations
.entry(*key) .entry(*key)
.or_insert_with(Vec::new) .or_insert_with(Vec::new)

View file

@ -425,6 +425,11 @@ pub fn start_transitions_if_applicable(
// above. // above.
property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0); property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0);
debug!(
"checking {:?} for matching end value",
possibly_expired_animations
);
// Per [1], don't trigger a new transition if the end state for that // Per [1], don't trigger a new transition if the end state for that
// transition is the same as that of a transition that's already // transition is the same as that of a transition that's already
// running on the same node. // running on the same node.
@ -852,26 +857,18 @@ pub fn complete_expired_transitions(
node: OpaqueNode, node: OpaqueNode,
style: &mut Arc<ComputedValues>, style: &mut Arc<ComputedValues>,
context: &SharedStyleContext, context: &SharedStyleContext,
) -> bool { expired_animations: &mut Vec<crate::animation::PropertyAnimation>,
let had_animations_to_expire; ) {
{ let mut all_expired_animations = context.expired_animations.write();
let all_expired_animations = context.expired_animations.read(); if let Some(animations) = all_expired_animations.remove(&node) {
let animations_to_expire = all_expired_animations.get(&node); debug!("removing expired animations for {:?}", node);
had_animations_to_expire = animations_to_expire.is_some(); for animation in animations {
if let Some(ref animations) = animations_to_expire {
for animation in *animations {
debug!("Updating expired animation {:?}", animation); debug!("Updating expired animation {:?}", animation);
// TODO: support animation-fill-mode // TODO: support animation-fill-mode
if let Animation::Transition(_, _, ref frame) = *animation { if let Animation::Transition(_, _, frame) = animation {
frame.property_animation.update(Arc::make_mut(style), 1.0); frame.property_animation.update(Arc::make_mut(style), 1.0);
expired_animations.push(frame.property_animation);
} }
} }
} }
}
if had_animations_to_expire {
context.expired_animations.write().remove(&node);
}
had_animations_to_expire
} }

View file

@ -607,7 +607,12 @@ trait PrivateMatchMethods: TElement {
// Finish any expired transitions. // Finish any expired transitions.
let this_opaque = self.as_node().opaque(); let this_opaque = self.as_node().opaque();
animation::complete_expired_transitions(this_opaque, style, context); animation::complete_expired_transitions(
this_opaque,
style,
context,
possibly_expired_animations,
);
// Merge any running animations into the current style, and cancel them. // Merge any running animations into the current style, and cancel them.
let had_running_animations = context let had_running_animations = context

View file

@ -0,0 +1,4 @@
[transitionend_event.html]
expected: TIMEOUT
[transitionend_event]
expected: TIMEOUT

View file

@ -13031,6 +13031,13 @@
{} {}
] ]
], ],
"transitionend_event.html": [
"71b88117a0280fbffcf3ab77105c0460317c66c8",
[
null,
{}
]
],
"white-space-pre-line-long-line.html": [ "white-space-pre-line-long-line.html": [
"bf0d0085fef0f1639637b2e652a7fb857cd51bf6", "bf0d0085fef0f1639637b2e652a7fb857cd51bf6",
[ [

View file

@ -0,0 +1,27 @@
<html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#test {
width: 10px;
height: 10px;
background: black;
transition: 1ms linear transform;
}
.transform {
transform:scale(1.2);
}
</style>
<div id="test" class="transform"></div>
<script>
async_test(function(t) {
let d = document.querySelector('div');
// Verify that we only receive a single transitionend event once the transition is complete.
d.ontransitionend = t.step_func(() => {
d.ontransitionend = t.unreached_func();
t.step_timeout(() => t.done(), 100);
});
t.step_timeout(() => d.className = "", 10);
});
</script>