WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
183781
[Web Animations] Dispatch DOM events for CSS Transitions and CSS Animations implemented as Web Animations
https://bugs.webkit.org/show_bug.cgi?id=183781
Summary
[Web Animations] Dispatch DOM events for CSS Transitions and CSS Animations i...
Antoine Quint
Reported
2018-03-20 09:44:01 PDT
Now that we've implemented CSS Animations and CSS Transitions as Web Animations (
webkit.org/b/183504
) we need to dispatch DOM events for them as well, as specified by
https://drafts.csswg.org/css-animations-2/#events
and
https://drafts.csswg.org/css-transitions-2/#transition-events
.
Attachments
Patch
(90.54 KB, patch)
2018-03-20 11:02 PDT
,
Antoine Quint
no flags
Details
Formatted Diff
Diff
Patch
(27.31 KB, patch)
2018-03-21 11:05 PDT
,
Antoine Quint
no flags
Details
Formatted Diff
Diff
Patch
(29.00 KB, patch)
2018-03-21 12:00 PDT
,
Antoine Quint
dino
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Antoine Quint
Comment 1
2018-03-20 11:02:09 PDT
Created
attachment 336135
[details]
Patch
Dean Jackson
Comment 2
2018-03-20 11:34:07 PDT
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.
Antoine Quint
Comment 3
2018-03-21 07:48:47 PDT
I will land this as smaller patches.
Antoine Quint
Comment 4
2018-03-21 11:01:06 PDT
Actually, we can reuse this bug.
Antoine Quint
Comment 5
2018-03-21 11:05:46 PDT
Created
attachment 336211
[details]
Patch
Antoine Quint
Comment 6
2018-03-21 12:00:48 PDT
Created
attachment 336216
[details]
Patch
Antoine Quint
Comment 7
2018-03-21 12:41:33 PDT
Committed
r229818
: <
https://trac.webkit.org/changeset/229818
>
Radar WebKit Bug Importer
Comment 8
2018-03-21 12:42:28 PDT
<
rdar://problem/38718784
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug