Bug 209682

Summary: [Web Animations] Crash under `KeyframeEffect::getKeyframes` for a `DeclarativeAnimation`
Product: WebKit Reporter: Devin Rousso <hi>
Component: AnimationsAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, graouts, graouts, hi, joepeck, saagar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Devin Rousso
Reported 2020-03-27 14:52:47 PDT
# STEPS TO REPRODUCE: 1. inspect any page with a CSS animation/transition 2. select a node with an active CSS animation/transition in the Elements Tab 3. evaluate: ``` let effect = $0.getAnimations()[0].effect; effect.getKeyframes(); // should not crash effect.target = null; effect.getKeyframes(); // will crash ```
Attachments
Patch (7.36 KB, patch)
2020-03-27 14:56 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Devin Rousso
Comment 1 2020-03-27 14:53:06 PDT
<rdar://problem/60962458>
Devin Rousso
Comment 2 2020-03-27 14:56:25 PDT
Created attachment 394761 [details] Patch
Antoine Quint
Comment 3 2020-03-28 02:40:54 PDT
Comment on attachment 394761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394761&action=review Very nice, thanks for spotting – and fixing – that! > LayoutTests/webanimations/css-animation-effect-target-change-and-get-keyframes-crash.html:1 > +<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] --> We really don't need this but anymore, this is the default and there's No Going Back.
Devin Rousso
Comment 4 2020-03-30 10:04:59 PDT
Comment on attachment 394761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394761&action=review Thanks for the review Antoine! >> LayoutTests/webanimations/css-animation-effect-target-change-and-get-keyframes-crash.html:1 >> +<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] --> > > We really don't need this but anymore, this is the default and there's No Going Back. I appreciate that, but given that the flag still exists, I think it's safe to still have it. Not to mention, it adds more information about what this test is actually testing.
EWS
Comment 5 2020-03-30 10:15:19 PDT
Committed r259205: <https://trac.webkit.org/changeset/259205> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394761 [details].
Devin Rousso
Comment 6 2020-04-07 17:05:00 PDT
*** Bug 210161 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.