Implicit keyframe for a CSS Animation should always use the underlying style
Created attachment 448698 [details] Patch
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/32301
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment on attachment 448698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448698&action=review > Source/WebCore/rendering/style/KeyframeList.cpp:92 > KeyframeValue keyframeValue(keyframe.key(), RenderStyle::clonePtr(*keyframe.style())); > for (auto propertyId : keyframe.properties()) > keyframeValue.addProperty(propertyId); > + keyframeValue.setTimingFunction(keyframe.timingFunction()); > + keyframeValue.setCompositeOperation(keyframe.compositeOperation()); can we just add a copy operator/function to KeyframeValue?
The test failure is due to bug 235019. I'll re-run EWS once that other bug lands.
(In reply to Antti Koivisto from comment #4) > Comment on attachment 448698 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448698&action=review > > > Source/WebCore/rendering/style/KeyframeList.cpp:92 > > KeyframeValue keyframeValue(keyframe.key(), RenderStyle::clonePtr(*keyframe.style())); > > for (auto propertyId : keyframe.properties()) > > keyframeValue.addProperty(propertyId); > > + keyframeValue.setTimingFunction(keyframe.timingFunction()); > > + keyframeValue.setCompositeOperation(keyframe.compositeOperation()); > > can we just add a copy operator/function to KeyframeValue? Actually, the only two call sites for fillImplicitKeyframes() are places where we make the copies as well, so I think we can wrap this all up into a single function.
(In reply to Antoine Quint from comment #6) > (In reply to Antti Koivisto from comment #4) > > Comment on attachment 448698 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=448698&action=review > > > > > Source/WebCore/rendering/style/KeyframeList.cpp:92 > > > KeyframeValue keyframeValue(keyframe.key(), RenderStyle::clonePtr(*keyframe.style())); > > > for (auto propertyId : keyframe.properties()) > > > keyframeValue.addProperty(propertyId); > > > + keyframeValue.setTimingFunction(keyframe.timingFunction()); > > > + keyframeValue.setCompositeOperation(keyframe.compositeOperation()); > > > > can we just add a copy operator/function to KeyframeValue? > > Actually, the only two call sites for fillImplicitKeyframes() are places > where we make the copies as well, so I think we can wrap this all up into a > single function. Actually, that's not true. I'll attempt to refactor this in a separate patch.
Created attachment 448717 [details] Patch for landing
Committed r287827 (245879@main): <https://commits.webkit.org/245879@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448717 [details].
<rdar://problem/87314805>