Bug 231443 – [css-ui] Fix interpolation of accent-color

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Aditya Keerthi]

...

[Aditya Keerthi]

rdar://84037162

[Aditya Keerthi]

Created attachment 440652 [details]
Patch

[Antoine Quint]

Comment on attachment 440652 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440652&action=review

Very nice! Thanks for catching this and fixing it Aditya.

> Source/WebCore/animation/CSSPropertyAnimation.cpp:1490
> +        auto& blendingRenderStyle = context.progress < 0.5 ? from : to;

If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`.

[Antoine Quint]

Comment on attachment 440652 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440652&action=review

Very nice! Thanks for catching this and fixing it Aditya.

>> Source/WebCore/animation/CSSPropertyAnimation.cpp:1490
>> +        auto& blendingRenderStyle = context.progress < 0.5 ? from : to;
> 
> If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`.

If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`.

[Aditya Keerthi]

Created attachment 440865 [details]
Patch for landing

[Aditya Keerthi]

(In reply to Antoine Quint from comment #4)
> Comment on attachment 440652 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440652&action=review
> 
> Very nice! Thanks for catching this and fixing it Aditya.

Thanks for the review!

> >> Source/WebCore/animation/CSSPropertyAnimation.cpp:1490
> >> +        auto& blendingRenderStyle = context.progress < 0.5 ? from : to;
> > 
> > If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`.
> 
> If canInterpolate(from, to) is false, then progress should have already been
> set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you
> should ASSERT() that you're only dealing with 0 or 1 here and change this
> line to `auto& blendingRenderStyle = context.progress ? from : to;`.

Added the assertion.

I think you meant `auto& blendingRenderStyle = context.progress ? to : from;` :)

[Antoine Quint]

(In reply to Aditya Keerthi from comment #6)
> (In reply to Antoine Quint from comment #4)
> > Comment on attachment 440652 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=440652&action=review
> I think you meant `auto& blendingRenderStyle = context.progress ? to :
> from;` :)

Absolutely!

[EWS]

Committed r283980 (242826@main): <https://commits.webkit.org/242826@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440865 [details].