[webkit-reviews] review granted: [Bug 241546] [CSS Container Queries] Invalidate animation keyframes using container units on when container size changes : [Attachment 460197] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 13 07:05:30 PDT 2022


Antoine Quint <graouts at webkit.org> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 241546: [CSS Container Queries] Invalidate animation keyframes using
container units on when container size changes
https://bugs.webkit.org/show_bug.cgi?id=241546

Attachment 460197: Patch

https://bugs.webkit.org/attachment.cgi?id=460197&action=review




--- Comment #2 from Antoine Quint <graouts at webkit.org> ---
Comment on attachment 460197
  --> https://bugs.webkit.org/attachment.cgi?id=460197
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.


More information about the webkit-reviews mailing list