From 9a99a6e4bfcbae72223a6fc35ba735d0adf5a65a Mon Sep 17 00:00:00 2001 From: Kevin Ellis Date: Thu, 2 Jul 2020 19:20:47 +0000 Subject: [PATCH] Bug 1641952 [wpt PR 23860] - Fix calculation of before change style for CSS transitions, a=testonly Automatic update from web-platform-tests Fix calculation of before change style for CSS transitions This patch performs a lazy calculation of the before change style once it has been determined that a transition is being retargeted. In doing so, the current position is correctly updated to reflect changes made via the web-animation API. The path also addresses a bug in the calculation of current time, in the case of a paused or play-pending animation. Bug: 1082401, 888661, 547609 Change-Id: I4b76879d840b482da8ebf23c1aad41b881fafce8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220263 Reviewed-by: Anders Hartvoll Ruud Reviewed-by: Robert Flack Commit-Queue: Kevin Ellis Cr-Commit-Position: refs/heads/master@{#783675} -- wpt-commits: ebe69e97a1ea77b2eb30d7b40ffef2d67455a805 wpt-pr: 23860 --- .../CSSTransition-currentTime.tentative.html | 22 ++++---- .../CSSTransition-effect.tentative.html | 2 +- .../KeyframeEffect-setKeyframes.tentative.html | 59 ++++++++++++++++++---- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/testing/web-platform/tests/css/css-transitions/CSSTransition-currentTime.tentative.html b/testing/web-platform/tests/css/css-transitions/CSSTransition-currentTime.tentative.html index 5800659e31be..d663ab644794 100644 --- a/testing/web-platform/tests/css/css-transitions/CSSTransition-currentTime.tentative.html +++ b/testing/web-platform/tests/css/css-transitions/CSSTransition-currentTime.tentative.html @@ -112,26 +112,25 @@ test(t => { div.style.left = '0px'; getComputedStyle(div).transitionProperty; - div.style.transition = 'left 100s'; + div.style.transition = 'left 100s ease-in'; div.style.left = '100px'; const transition = div.getAnimations()[0]; - // Seek to the middle and get the portion. - // - // We deliberately DON'T set transition-timing-function to linear so that we - // can test that it is applied correctly. + // Seek to the middle. Note, this is not equivalent to 50% progress since the + // timing-function is non-linear. transition.currentTime = 50 * MS_PER_SEC; const portion = transition.effect.getComputedTiming().progress; - // Reverse transition + // Reverse the transition. div.style.left = '0px'; const reversedTransition = div.getAnimations()[0]; - // If the transition reversing behavior does not advange the previous transition - // to the time set by currentTime, start and end values will both be 0px and - // no transition will be produced. - assert_not_equals(reversedTransition, undefined, "A reversed transition is produced"); + // If the transition reversing behavior does not advance the previous + // transition to the time set by currentTime, start and end values will both + // be 0px and no transition will be produced. + assert_not_equals(reversedTransition, undefined, + "A reversed transition is produced"); const expectedDuration = 100 * MS_PER_SEC * portion; assert_approx_equals( @@ -140,6 +139,7 @@ test(t => { 1, "The reversed transition has correctly reduced duration" ); -}, "Transition reversing behavior respects currentTime and uses the transition's current position."); +}, "Transition reversing behavior respects currentTime and uses the " + + "transition's current position."); diff --git a/testing/web-platform/tests/css/css-transitions/CSSTransition-effect.tentative.html b/testing/web-platform/tests/css/css-transitions/CSSTransition-effect.tentative.html index 5ccb201082e3..b58c93d2e69f 100644 --- a/testing/web-platform/tests/css/css-transitions/CSSTransition-effect.tentative.html +++ b/testing/web-platform/tests/css/css-transitions/CSSTransition-effect.tentative.html @@ -90,7 +90,7 @@ promise_test(async t => { const new_transition = div.getAnimations()[0]; await new_transition.ready; - assert_equals(getComputedStyle(div).left, '0px'); + assert_equals(getComputedStyle(div).left, '100px'); }, 'After setting a transition\'s effect to null, a new transition can be started'); // This is a regression test for https://crbug.com/992668, where Chromium would diff --git a/testing/web-platform/tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative.html b/testing/web-platform/tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative.html index 3b151a66546b..91ebd556281b 100644 --- a/testing/web-platform/tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative.html +++ b/testing/web-platform/tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative.html @@ -96,18 +96,21 @@ retarget_test("A transition with replaced keyframes animating nothing " + test(t => { const div = addDiv(t); - // initial conditions + + // Initialize the style. div.style.left = '100px'; div.style.top = '100px'; div.style.transition = 'left 100s linear, top 100s linear'; getComputedStyle(div).left; - // start some transitions + // Start some transitions. div.style.left ='200px'; div.style.top = '200px'; const transitions = div.getAnimations(); - // hand control of the left property over to top's transition + // Hand control of the left property over to top's transition. The composite + // ordering of the animations is 'left' then 'top' since the transitions were + // generated in the same transition generation and follow Unicode ordering. assert_equals(transitions[0].transitionProperty, 'left'); transitions[0].effect.setKeyframes({}); transitions[1].effect.setKeyframes([ @@ -115,26 +118,60 @@ test(t => { {left: '400px', top: '200px'}]) getComputedStyle(div).left; - // these form a single style change, equivalent to setting times and then setting left + // These updates form a single style change, equivalent to setting times and + // then setting left. transitions[0].currentTime = 50 * MS_PER_SEC; div.style.left ='100px'; transitions[1].currentTime = 60 * MS_PER_SEC; - const reversedTransition = div.getAnimations()[1] + // As there was a style change event, the new 'left' transition now has a + // higher value for the transition generation than the 'top' transition, + // reversing the order of the transitions returned by getAnimations. + const reversedTransition = div.getAnimations()[1]; assert_equals(reversedTransition.transitionProperty, 'left', "A reversed transition on left is produced"); + assert_approx_equals( reversedTransition.effect.getComputedTiming().activeDuration, 50 * MS_PER_SEC, 1, - "The reversed transition has correctly reduced duration (based on the original left transition)." - ); + "The reversed transition has correctly reduced duration (based on the " + + "original left transition)."); + assert_equals(reversedTransition.effect.getKeyframes()[0].left, '280px', - "The reversed transition gets its start value from the other transition controlling left"); + "The reversed transition gets its start value from the other " + + "transition controlling left"); -}, "A transition with replaced keyframes animating nothing on a property being controlled by another " + - "modified transition exhibits normal reversing behavior and reverses from the other " + - "transition's current value."); +}, "A transition with replaced keyframes animating nothing on a property " + + "being controlled by another modified transition exhibits normal " + + "reversing behavior and reverses from the other transition's current " + + "value."); +test(t => { + const div = addDiv(t); + + // Initialize the style. + div.style.left = '100px'; + div.style.transition = 'left 100s linear'; + getComputedStyle(div).left; + + // Start the transtion. + div.style.left ='200px'; + const transition = div.getAnimations()[0]; + + // Update the keyframes and advance to the midpoint of the animation. + transition.effect.setKeyframes([ + { offset: 0, left: '-50px', composite: 'add', easing: 'linear'}]); + transition.currentTime = 50 * MS_PER_SEC; + assert_equals(getComputedStyle(div).left, '175px', + 'The computed style is based on the updated keyframes'); + + div.style.left = '100px'; + const reversedTransition = div.getAnimations()[0]; + assert_equals(reversedTransition.effect.getKeyframes()[0].left, '175px', + 'The start value matches the \'before change\' value'); +}, 'A transition with replaced kefyrames and composite \'add\' exhibits ' + + 'normal reversing behavior, and the effect is not double counted when ' + + 'calculating the before change style'); -- 2.11.4.GIT