Bug 241546 – [CSS Container Queries] Invalidate animation keyframes using container units on when container size changes

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

Fix a WPT

[Antti Koivisto]

Created attachment 460197 [details]
Patch

[Antoine Quint]

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.

[Antti Koivisto]

> 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.

[Antti Koivisto]

Created attachment 460253 [details]
Patch for landing

[EWS]

Commit message contains (OOPS!), blocking PR #None

[Antti Koivisto]

Created attachment 460255 [details]
Patch for landing

[Antti Koivisto]

Created attachment 460256 [details]
Patch for landing

[EWS]

Commit message contains (OOPS!), blocking PR #None

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/95231683>