Animation of font-size with rem values is incorrect

[Scott Kellum]

Rem units are intermittently miscalculated on load within animations. This happens in Safari 12 both on my Mac and iOS. Occasionally it will render correctly but on refreshes it more often than not renders incorrectly.

Replicate the bug:
https://codepen.io/scottkellum/pen/28d147aa4b21d84367ce832a986cd549?editors=1100


Possibly related: https://bugs.webkit.org/show_bug.cgi?id=194749


If the animation is toggled off and back on or set late with javascript it will work again.

[Radar WebKit Bug Importer]

<rdar://problem/48171742>

[Simon Fraser (smfr)]

Seems to only reproduce with Web Animations enabled.

[Antoine Quint]

Created attachment 399640 [details]
Test

[Antoine Quint]

If you remove the animation-delay and animation-play-state values in the animation shorthand you can see the animation running and using what looks like `font-size: 1rem` for the 50% keyframe.

[Antoine Quint]

If you change the keyframes to this:

@keyframes anim {

    0% {
        font-size: 2rem;
    }

    50% {
        font-size: 6rem;
    }

    100% {
        font-size: 2rem;
    }

}

This sets the value to what looks like 0.5rem for the duration of the entire animation. So setting explicit 0% and 100% keyframes show different results.

[Antoine Quint]

The way we compute the styles on the KeyframeListValue objects is all wrong, which explains why we get no animation for the explicit keyframes case, they're all resolving to the same value.

[Antoine Quint]

This happens because CSSPrimitiveValue::computeNonCalcLengthDouble() has no conversionData.rootStyle().

[Antoine Quint]

Created attachment 399643 [details]
Test

[Antti Koivisto]

Created attachment 399733 [details]
patch

[Antoine Quint]

Comment on attachment 399733 [details]
patch

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

> LayoutTests/animations/keyframe-rem-unit.html:20
> +    testRunner.waitUntilDone();
> +    await document.getAnimations()[0].finished;
> +    testRunner.notifyDone();

If you write an animation that use a rem unit in the "from" keyframe then you could write this to only wait for the animation's "ready" promise which would run the test one animation frame faster I think. Hopefully that works.

[Antoine Quint]

Does this patch also fix bug 194749?

[Antti Koivisto]

Created attachment 399735 [details]
patch

[Antti Koivisto]

(In reply to Antoine Quint from comment #11)
> Does this patch also fix bug 194749?

Nope, but I think it is related. The base style for 'em' resolution is similarly wrong (it should be the parent element style).

[EWS]

Committed r261861: <https://trac.webkit.org/changeset/261861>

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