Summary: | [CSS Container Queries] Invalidate animation keyframes using container units on when container size changes | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||||
Component: | CSS | Assignee: | Antti Koivisto <koivisto> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, glenn, graouts, kangil.han, kondapallykalyan, pdr, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 229659 | ||||||||||||
Attachments: |
|
Description
Antti Koivisto
2022-06-13 05:49:11 PDT
Created attachment 460197 [details]
Patch
Comment on attachment 460197 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460197&action=review > COMMIT_MESSAGE:6 > +Container size change changes the interpretation of container units used in keyframes.. Extra "." here. > Source/WebCore/rendering/style/KeyframeList.cpp:226 > + for (auto& keyframe : m_keyframes) { > + if (keyframe.style()->usesContainerUnits()) > + return true; > + } > + return false; I *think* this could be made to use std::any_of: return std::any_of(m_keyframes.begin(), m_keyframes.end(), [] (auto& it) { return it.value->usesContainerUnits(); }); > Source/WebCore/style/StyleTreeResolver.cpp:583 > + if (oldStyle && parent().needsUpdateQueryContainerDependentStyle) > + styleable.queryContainerDidChange(); While you're there, I think a FIXME to do something similar for viewport units would be good. > Source/WebCore/style/Styleable.cpp:636 > + auto* cssAnimation = dynamicDowncast<CSSAnimation>(animation.get()); > + if (!cssAnimation) > + continue; I *think* you should only check whether this is a `CSSTransition`, in which case I expect there is nothing to do here and the style invalidation mechanism would automatically update things based on the computed value changing. This would allow both `CSSAnimation` and vanilla `WebAnimation` objects to be updated. Then you'd need to add testing coverage for animations created using the JS API. Perfectly fine to do as a separate patch, in which case a FIXME to that effect would be nice. > I *think* you should only check whether this is a `CSSTransition`, in which
> case I expect there is nothing to do here and the style invalidation
> mechanism would automatically update things based on the computed value
> changing. This would allow both `CSSAnimation` and vanilla `WebAnimation`
> objects to be updated.
keyframesRuleDidChange is non-virtual function in CSSAnimation so I think that case requires something else.
Created attachment 460253 [details]
Patch for landing
Commit message contains (OOPS!), blocking PR #None Created attachment 460255 [details]
Patch for landing
Created attachment 460256 [details]
Patch for landing
Commit message contains (OOPS!), blocking PR #None Committed r295569 (251574@main): <https://commits.webkit.org/251574@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 460256 [details]. |