Summary: | [Web Animations] Dispatch DOM events for CSS Transitions and CSS Animations implemented as Web Animations | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, dbates, dino, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, kondapallykalyan, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari Technology Preview | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Antoine Quint
2018-03-20 09:44:01 PDT
Created attachment 336135 [details]
Patch
Comment on attachment 336135 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336135&action=review This is all good. It just needs to be split into smaller patches because it is doing multiple things. > Source/WebCore/animation/AnimationTimeline.cpp:148 > + // In case this element is newly getting a "display: none" we need to cancel all of its animations and disregard new ones. > + if (oldStyle && oldStyle->hasAnimations() && oldStyle->display() != NONE && newStyle.display() == NONE) { > + if (m_elementToCSSAnimationByName.contains(&element)) { > + for (const auto& cssAnimationsByNameMapItem : m_elementToCSSAnimationByName.take(&element)) > + cancelOrRemoveDeclarativeAnimation(cssAnimationsByNameMapItem.value); > + } > + return; > + } > + > + if (oldStyle && oldStyle->hasAnimations() && newStyle.hasAnimations() && *(oldStyle->animations()) == *(newStyle.animations())) > + return; This display:none work should be a separate patch. > Source/WebCore/animation/DeclarativeAnimation.cpp:72 > + else > + pause(); So should this. > Source/WebCore/animation/DeclarativeAnimation.cpp:106 > +void DeclarativeAnimation::cancel() > +{ > + auto cancelationTime = 0_s; > + if (auto animationEffect = effect()) > + cancelationTime = animationEffect->activeTime().value_or(0_s); > + > + WebAnimation::cancel(); > + > + invalidateDOMEvents(cancelationTime); > +} And maybe cancel() on declarative animations should be a separate patch too. I will land this as smaller patches. Actually, we can reuse this bug. Created attachment 336211 [details]
Patch
Created attachment 336216 [details]
Patch
Committed r229818: <https://trac.webkit.org/changeset/229818> |