Bug 234823

Summary: [Web Animations] changing the timing of a transition such that it's finished should no longer have it marked as running
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, dino, graouts, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 234829    
Bug Blocks: 235130    
Attachments:
Description Flags
Patch
none
Patch none

Antoine Quint
Reported 2022-01-03 08:40:17 PST
[Web Animations] changing the effect of a transition should no longer have it marked as running
Attachments
Patch (5.10 KB, patch)
2022-01-03 08:40 PST, Antoine Quint 		no flags
DetailsFormatted DiffDiff
Patch (6.21 KB, patch)
2022-01-03 14:25 PST, Antoine Quint 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2022-01-03 08:40:51 PST
Created attachment 448244 [details] Patch
Dean Jackson
Comment 2 2022-01-03 08:43:05 PST
Comment on attachment 448244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448244&action=review > Source/WebCore/animation/DeclarativeAnimation.cpp:242 > + auto effectChanged = effect() != newEffect; > + > + WebAnimation::setEffect(WTFMove(newEffect)); Why would you call setEffect if !effectChanged?
Antoine Quint
Comment 3 2022-01-03 08:44:49 PST
(In reply to Dean Jackson from comment #2) > Comment on attachment 448244 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448244&action=review > > > Source/WebCore/animation/DeclarativeAnimation.cpp:242 > > + auto effectChanged = effect() != newEffect; > > + > > + WebAnimation::setEffect(WTFMove(newEffect)); > > Why would you call setEffect if !effectChanged? Yeah, I suppose we could return early here, but the idea was that it made no assumption as to what WebAnimation::setEffect() did with that value.
Antoine Quint
Comment 4 2022-01-03 10:49:37 PST
Committed r287550 (245685@trunk): <https://commits.webkit.org/245685@trunk>
Radar WebKit Bug Importer
Comment 5 2022-01-03 10:50:22 PST
<rdar://problem/87069661>
WebKit Commit Bot
Comment 6 2022-01-03 13:45:07 PST
Re-opened since this is blocked by bug 234829
Antoine Quint
Comment 7 2022-01-03 14:25:33 PST
Created attachment 448257 [details] Patch
Darin Adler
Comment 8 2022-01-03 14:44:13 PST
Where’s the explanation of why we had to revert? It says "caused a regression"?
Antoine Quint
Comment 9 2022-01-03 14:59:55 PST
(In reply to Darin Adler from comment #8) > Where’s the explanation of why we had to revert? It says "caused a > regression"? This was due not knowing how to use webkitbot correctly on the slack channel. The reason the original patch was wrong was that it regressed another sub test in this WPT test.
EWS
Comment 10 2022-01-04 08:12:30 PST
Committed r287568 (245703@main): <https://commits.webkit.org/245703@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448257 [details].
Note You need to log in before you can comment on or make changes to this bug.