Bug 235503

Summary: Pass the Document through bindings to KeyframeEffect::setKeyframes()
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, darin, dino, esprehn+autocc, ews-watchlist, graouts, kangil.han, kondapallykalyan, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sam: review+, ews-feeder: commit-queue-

Antoine Quint
Reported 2022-01-24 01:33:42 PST
Pass the Document through bindings to KeyframeEffect::setKeyframes()
Attachments
Patch (16.41 KB, patch)
2022-01-24 01:41 PST, Antoine Quint 		sam: review+
ews-feeder: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2022-01-24 01:41:15 PST
Created attachment 449790 [details] Patch
Sam Weinig
Comment 2 2022-01-24 09:07:02 PST
Comment on attachment 449790 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449790&action=review > Source/WebCore/animation/KeyframeEffect.cpp:210 > - auto* scriptExecutionContext = jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject)->scriptExecutionContext(); > - if (is<Document>(scriptExecutionContext)) { > - if (downcast<Document>(*scriptExecutionContext).settings().webAnimationsCompositeOperationsEnabled()) > - baseProperties.composite = baseKeyframe.composite; > - } > + if (document.settings().webAnimationsCompositeOperationsEnabled()) > + baseProperties.composite = baseKeyframe.composite; This seems right, but in future, you can actually get settings directly from the ScriptExecutionContext via the settingsValues() accessor.
Antoine Quint
Comment 3 2022-01-24 10:34:56 PST
(In reply to Sam Weinig from comment #2) > Comment on attachment 449790 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449790&action=review > > > Source/WebCore/animation/KeyframeEffect.cpp:210 > > - auto* scriptExecutionContext = jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject)->scriptExecutionContext(); > > - if (is<Document>(scriptExecutionContext)) { > > - if (downcast<Document>(*scriptExecutionContext).settings().webAnimationsCompositeOperationsEnabled()) > > - baseProperties.composite = baseKeyframe.composite; > > - } > > + if (document.settings().webAnimationsCompositeOperationsEnabled()) > > + baseProperties.composite = baseKeyframe.composite; > > This seems right, but in future, you can actually get settings directly from > the ScriptExecutionContext via the settingsValues() accessor. Good to know! This patch could have just passed the ScriptExecutionContext in then, but as it turns out we also needed a Document further down to establish a CSSParserContext so it turned out to be handy.
Antoine Quint
Comment 4 2022-01-24 10:41:33 PST
Committed r288452 (246341@trunk): <https://commits.webkit.org/246341@trunk>
Radar WebKit Bug Importer
Comment 5 2022-01-24 10:42:19 PST
<rdar://problem/87978492>
Antoine Quint
Comment 6 2022-01-24 11:09:28 PST
Committed r288454 (246343@trunk): <https://commits.webkit.org/246343@trunk>
Note You need to log in before you can comment on or make changes to this bug.