Transitions misbehave when a property is expressed in different units

[Benoit Marchant]

In the attached test case, the width doesn't animate because it's expressed as % at the beginning and as em when the transition should apply

[Benoit Marchant]

Created attachment 27370 [details]
Test case for the bug

[Deirdre Saoirse Moen]

rdar://problem/6561547

[Simon Fraser (smfr)]

*** Bug 54647 has been marked as a duplicate of this bug. ***

[Mike Lawther]

*** Bug 63309 has been marked as a duplicate of this bug. ***

[Ahmad Saleem]

I am not able to reproduce issue in reference to animation but the animation ends abruptly if mouse leaves container and come back in Safari Technology Preview 152 and it is not same as other browsers, where the animation ends back smoothly.

Do you want me to create separate bug for that and close this? Appreciate if someone can comment. Thanks!

[Alexey Proskuryakov]

Good catch. This problem with interrupted transition only reproduces when the units are different, so I would say that it's a part of the original bug, and can continue to be tracked here.

Re-titled a bit to that effect.

[Antoine Quint]

Created attachment 462331 [details]
Improved testcase

In the attached improved testcase, we automatically trigger, reverse the transition and log the values provided by the Web Animations API which are all out of whack in WebKit.

[Antoine Quint]

Firefox / Chrome (actual values may vary):

Transitioning width from 10% to 960px
Transitioning width from calc(6.63989% + 322.571px) to 10%

Safari:

Transitioning width from 151.1875px to 151.1875px
Transitioning width from 151.1875px to 151.1875px

What puzzles me is that the from/to values are the same for the initial transition even though we do the right thing in terms of rendering. The second one is consistent with the fact that we snap, although it makes no sense to transition between two equal values.

[Antoine Quint]

We are getting 10% and 960px values in the RenderStyle objects provided when creating the CSSTransition object.

[Antoine Quint]

Huh-ho, so for the getKeyframes() issue the problem is that we are calling this method to get the computed value from the style in the addPropertyToKeyframe lambda:

computedStyleExtractor.valueForPropertyInStyle(style, cssPropertyId, renderer)

But this absolutely does not use the provided style object to get the width instead using the current bounds for the renderer. Not good!

[Antoine Quint]

Filed bug 245179 for the getKeyframes() issue which is unrelated to the fact that we're not visibly animating for transition reversal.

[Antoine Quint]

The issue is under static Length blendMixedTypes(const Length& from, const Length& to, const BlendingContext& context) where we have from = calc(blend(10%, 960px, 0.33)) and to = 10% but the result is calc(blend(10%, 10%, …)).

[Antoine Quint]

I think this might be the culprit:

CalcExpressionBlendLength::CalcExpressionBlendLength(Length from, Length to, float progress)
    : CalcExpressionNode(CalcExpressionNodeType::BlendLength)
    , m_from(from)
    , m_to(to)
    , m_progress(progress)
{
    // Flatten nesting of CalcExpressionBlendLength as a speculative fix for rdar://problem/30533005.
    // CalcExpressionBlendLength is only used as a result of animation and they don't nest in normal cases.
    if (m_from.isCalculated() && m_from.calculationValue().expression().type() == CalcExpressionNodeType::BlendLength)
        m_from = downcast<CalcExpressionBlendLength>(m_from.calculationValue().expression()).from();
    if (m_to.isCalculated() && m_to.calculationValue().expression().type() == CalcExpressionNodeType::BlendLength)
        m_to = downcast<CalcExpressionBlendLength>(m_to.calculationValue().expression()).to();
}

[Antoine Quint]

Indeed, removing that code fixes the issue. But this fixed a crash so we probably need to find a better way to flatten blend calculation values here.