⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 263729 in webkit


Ignore:
Timestamp:
Jun 29, 2020, 10:56:53 PM (6 years ago)
Author:
graouts@webkit.org
Message:

[Web Animations] REGRESSION: Bootstrap Carousel component v4.1 regressed with Web Animations
​https://bugs.webkit.org/show_bug.cgi?id=213376
<rdar://problem/64531242>

Reviewed by Dean Jackson.

Source/WebCore:

An older version of the Bootstrap CSS and JS library had a rather odd way to implement a completion callback
for a transition: it would register a "transitionend" event but also set a timeout of the transition's duration
and use whichever came first as a callback to run completion tasks for the transition.

Additionally, in that callback, it would set the transitioned value to the same computed value but using a different
specified value, for instance setting the "transform" CSS property to "translateX(0)" instead of "translateY(0)".

In our implementation this would make the completed transition repeat. Indeed, we would first incorrectly assume that
the transition was still "running" and not "finished", per the CSS Transitions spec terminology as we only update
that status when we update animations under Page::updateRendering(). We now update an existing transition's
status first in AnimationTimeline::updateCSSTransitionsForElementAndProperty().

Another issue is that when we considered the existing transition to be running, even though it was finished, we would
use the "timeline time at creation" to compute its current progress, which would yield situations where we computed
the before-change style to be the existing transition's current computed value, except that transition's progress was
0 since the "timeline time at creation" happens before the transition's resolved start time. We now only use the
"timeline time at creation" in the situations it was designed to be used: either when the transition has not yet had
a resolved start time, or its resolved start time is the current timeline time (ie. it was just set).

To be able to compare the transition's resolved start time and the current timeline time, we also updated the internal
start time getter and setter methods to use Seconds instead of double which is only needed for the JS bindings.

Test: webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout.html

  • animation/AnimationTimeline.cpp:

(WebCore::AnimationTimeline::updateCSSTransitionsForElementAndProperty):

  • animation/DeclarativeAnimation.cpp:

(WebCore::DeclarativeAnimation::bindingsStartTime const):
(WebCore::DeclarativeAnimation::setBindingsStartTime):

  • animation/WebAnimation.cpp:

(WebCore::WebAnimation::bindingsStartTime const):
(WebCore::WebAnimation::setBindingsStartTime):
(WebCore::WebAnimation::setStartTime):
(WebCore::WebAnimation::startTime const): Deleted.

  • animation/WebAnimation.h:

(WebCore::WebAnimation::startTime const):
(WebCore::WebAnimation::bindingsStartTime const): Deleted.

LayoutTests:

Add a test that uses a timeout instead of a "transitionend" event to retarget a transition upon completion
to the same computed value but not the same specified value to check that we generate a transition that has
no visual effect.

  • webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout-expected.txt: Added.
  • webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r263728 r263729  
     12020-06-29  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] REGRESSION: Bootstrap Carousel component v4.1 regressed with Web Animations
     4        https://bugs.webkit.org/show_bug.cgi?id=213376
     5        <rdar://problem/64531242>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Add a test that uses a timeout instead of a "transitionend" event to retarget a transition upon completion
     10        to the same computed value but not the same specified value to check that we generate a transition that has
     11        no visual effect.
     12
     13        * webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout-expected.txt: Added.
     14        * webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout.html: Added.
     15
    1162020-06-29  Wenson Hsieh  <wenson_hsieh@apple.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r263727 r263729  
     12020-06-29  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] REGRESSION: Bootstrap Carousel component v4.1 regressed with Web Animations
     4        https://bugs.webkit.org/show_bug.cgi?id=213376
     5        <rdar://problem/64531242>
     6
     7        Reviewed by Dean Jackson.
     8
     9        An older version of the Bootstrap CSS and JS library had a rather odd way to implement a completion callback
     10        for a transition: it would register a "transitionend" event but also set a timeout of the transition's duration
     11        and use whichever came first as a callback to run completion tasks for the transition.
     12
     13        Additionally, in that callback, it would set the transitioned value to the same computed value but using a different
     14        specified value, for instance setting the "transform" CSS property to "translateX(0)" instead of "translateY(0)".
     15
     16        In our implementation this would make the completed transition repeat. Indeed, we would first incorrectly assume that
     17        the transition was still "running" and not "finished", per the CSS Transitions spec terminology as we only update
     18        that status when we update animations under Page::updateRendering(). We now update an existing transition's
     19        status first in AnimationTimeline::updateCSSTransitionsForElementAndProperty().
     20
     21        Another issue is that when we considered the existing transition to be running, even though it was finished, we would
     22        use the "timeline time at creation" to compute its current progress, which would yield situations where we computed
     23        the before-change style to be the existing transition's current computed value, except that transition's progress was
     24        0 since the "timeline time at creation" happens before the transition's resolved start time. We now only use the
     25        "timeline time at creation" in the situations it was designed to be used: either when the transition has not yet had
     26        a resolved start time, or its resolved start time is the current timeline time (ie. it was just set).
     27
     28        To be able to compare the transition's resolved start time and the current timeline time, we also updated the internal
     29        start time getter and setter methods to use Seconds instead of double which is only needed for the JS bindings.
     30
     31        Test: webanimations/css-transition-retargeting-to-same-value-upon-completion-with-timeout.html
     32
     33        * animation/AnimationTimeline.cpp:
     34        (WebCore::AnimationTimeline::updateCSSTransitionsForElementAndProperty):
     35        * animation/DeclarativeAnimation.cpp:
     36        (WebCore::DeclarativeAnimation::bindingsStartTime const):
     37        (WebCore::DeclarativeAnimation::setBindingsStartTime):
     38        * animation/WebAnimation.cpp:
     39        (WebCore::WebAnimation::bindingsStartTime const):
     40        (WebCore::WebAnimation::setBindingsStartTime):
     41        (WebCore::WebAnimation::setStartTime):
     42        (WebCore::WebAnimation::startTime const): Deleted.
     43        * animation/WebAnimation.h:
     44        (WebCore::WebAnimation::startTime const):
     45        (WebCore::WebAnimation::bindingsStartTime const): Deleted.
     46
    1472020-06-29  Brady Eidson  <beidson@apple.com>
    248

  • trunk/Source/WebCore/animation/AnimationTimeline.cpp

    r263464 r263729  
    386386    }
    387387
     388    // A CSS Transition might have completed since the last time animations were updated so we must
     389    // update the running and completed transitions membership in that case.
     390    if (is<CSSTransition>(animation) && element.hasRunningTransitionsForProperty(property) && animation->playState() == WebAnimation::PlayState::Finished) {
     391        element.ensureCompletedTransitionsByProperty().set(property, element.ensureRunningTransitionsByProperty().take(property));
     392        animation = nullptr;
     393    }
     394
    388395    // https://drafts.csswg.org/css-transitions-1/#before-change-style
    389396    // Define the before-change style as the computed values of all properties on the element as of the previous style change event, except with
    … …  
    392399        if (animation && animation->isRelevant()) {
    393400            auto animatedStyle = RenderStyle::clone(currentStyle);
    394             animation->resolve(animatedStyle, is<CSSTransition>(animation) ? downcast<CSSTransition>(*animation).timelineTimeAtCreation() : WTF::nullopt);
     401            // If a transition has not yet started or started when animations were last updated, use the timeline time at its creation
     402            // as its start time to ensure that it will produce a style with progress > 0.
     403            bool shouldUseTimelineTimeAtCreation = is<CSSTransition>(animation) && (!animation->startTime() || *animation->startTime() == currentTime());
     404            animation->resolve(animatedStyle, shouldUseTimelineTimeAtCreation ? downcast<CSSTransition>(*animation).timelineTimeAtCreation() : WTF::nullopt);
    395405            return animatedStyle;
    396406        }

  • trunk/Source/WebCore/animation/DeclarativeAnimation.cpp

    r260671 r263729  
    133133{
    134134    flushPendingStyleChanges();
    135     return WebAnimation::startTime();
     135    return WebAnimation::bindingsStartTime();
    136136}
    137137
    … …  
    139139{
    140140    flushPendingStyleChanges();
    141     return WebAnimation::setStartTime(startTime);
     141    return WebAnimation::setBindingsStartTime(startTime);
    142142}
    143143

  • trunk/Source/WebCore/animation/WebAnimation.cpp

    r263464 r263729  
    302302}
    303303
    304 Optional<double> WebAnimation::startTime() const
     304Optional<double> WebAnimation::bindingsStartTime() const
    305305{
    306306    if (!m_startTime)
    307307        return WTF::nullopt;
    308     return secondsToWebAnimationsAPITime(m_startTime.value());
    309 }
    310 
    311 void WebAnimation::setBindingsStartTime(Optional<double> startTime)
    312 {
    313     setStartTime(startTime);
    314 }
    315 
    316 void WebAnimation::setStartTime(Optional<double> startTime)
     308    return secondsToWebAnimationsAPITime(*m_startTime);
     309}
     310
     311void WebAnimation::setBindingsStartTime(Optional<double> newStartTime)
     312{
     313    if (newStartTime)
     314        setStartTime(Seconds::fromMilliseconds(*newStartTime));
     315    else
     316        setStartTime(WTF::nullopt);
     317}
     318
     319void WebAnimation::setStartTime(Optional<Seconds> newStartTime)
    317320{
    318321    // 3.4.6 The procedure to set the start time of animation, animation, to new start time, is as follows:
    319322    // https://drafts.csswg.org/web-animations/#setting-the-start-time-of-an-animation
    320 
    321     Optional<Seconds> newStartTime;
    322     if (!startTime)
    323         newStartTime = WTF::nullopt;
    324     else
    325         newStartTime = Seconds::fromMilliseconds(startTime.value());
    326323
    327324    // 1. Let timeline time be the current time value of the timeline that animation is associated with. If

  • trunk/Source/WebCore/animation/WebAnimation.h

    r263464 r263729  
    105105    ExceptionOr<void> commitStyles();
    106106
    107     virtual Optional<double> bindingsStartTime() const { return startTime(); }
     107    virtual Optional<double> bindingsStartTime() const;
    108108    virtual void setBindingsStartTime(Optional<double>);
    109     Optional<double> startTime() const;
    110     void setStartTime(Optional<double>);
     109    Optional<Seconds> startTime() const { return m_startTime; }
     110    void setStartTime(Optional<Seconds>);
    111111    virtual Optional<double> bindingsCurrentTime() const;
    112112    virtual ExceptionOr<void> setBindingsCurrentTime(Optional<double>);
Note: See TracChangeset for help on using the changeset viewer.