Bug 289992 – [scroll-animations] Presence of an animation with a custom property breaks font-size that depends on rem

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

[Roman Komarov]

Created attachment 474617 [details]
Saved version of CodePen

Steps to reproduce:

1. Open https://codepen.io/kizu/pen/xbxWxmp in Safari TP 215 with scroll-driven animations feature flag enabled or the attached file.
2. The text should be 1rem in size, but it very big.

The conditions are rather complex, I stumbled upon this bug when testing how scroll-driven animations work in my blog. Saw this issue with my sidenotes (for example, here: https://kizu.dev/scroll-driven-state-transfer/), then reduced to the smallest test case I could.

[Radar WebKit Bug Importer]

<rdar://problem/147872254>

[Antoine Quint]

Inspecting the animation's computed timing and computed keyframes shows nothing unexpected. And indeed, removing the custom property in the 0% keyframe fixes the rendering… but it's really unclear why that has to do with anything since a custom property that is not used for anything.

[Antoine Quint]

It appears the issue is that we're using the computed style from the parent rather than the one set on the element itself, but only if we have this unregistered custom property set.

[Antoine Quint]

Created attachment 474791 [details]
Test

[Antoine Quint]

The `animatedStyle` computed in the `applyAnimations()` lambda within `Style::TreeResolver::createAnimatedElementUpdate()` has the expected value until `applyCascadeAfterAnimation` is called.

[Antoine Quint]

Here's the call stack when we get to the spot where we compute the value incorrectly in `Style::computeNonCalcLengthDouble()`:

#0	0x00000003072c98f0 in WebCore::Style::computeNonCalcLengthDouble at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/values/primitives/StyleLengthResolution.cpp:273
#1	0x00000003043ed048 in WebCore::CSSPrimitiveValue::resolveAsLengthDouble at /Users/antoine/Code/safari/OpenSource/Source/WebCore/css/CSSPrimitiveValue.cpp:621
#2	0x00000003043ed178 in WebCore::CSSPrimitiveValue::resolveAsLength<float> at /Users/antoine/Code/safari/OpenSource/Source/WebCore/css/CSSPrimitiveValue.cpp:581
#3	0x0000000302b37b90 in WebCore::Style::BuilderCustom::applyValueFontSize at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilderCustom.h:1686
#4	0x0000000302b283e0 in WebCore::Style::BuilderGenerated::applyProperty at /Users/antoine/Builds/Debug/DerivedSources/WebCore/StyleBuilderGenerated.cpp:6028
#5	0x0000000307173124 in WebCore::Style::Builder::applyProperty at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:409
#6	0x00000003071928c4 in WebCore::Style::Builder::applyCascadeProperty(WebCore::Style::PropertyCascade::Property const&)::'lambda'(WebCore::SelectorChecker::LinkMatchMask)::operator()(WebCore::SelectorChecker::LinkMatchMask) const at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:264
#7	0x0000000307171b38 in WebCore::Style::Builder::applyCascadeProperty at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:268
#8	0x00000003071b7438 in void WebCore::Style::Builder::applyPropertiesImpl<(WebCore::Style::Builder::CustomPropertyCycleTracking)1>(int, int)::'lambda'(unsigned long)::operator()(unsigned long) const at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:173
#9	0x0000000307171c94 in WebCore::Style::Builder::applyPropertiesImpl<(WebCore::Style::Builder::CustomPropertyCycleTracking)1> at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:185
#10	0x0000000307171758 in WebCore::Style::Builder::applyProperties at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:152
#11	0x00000003071715b8 in WebCore::Style::Builder::applyHighPriorityProperties at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:124
#12	0x00000003071714cc in WebCore::Style::Builder::applyAllProperties at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleBuilder.cpp:110
#13	0x00000003071fc314 in WebCore::Style::TreeResolver::applyCascadeAfterAnimation at /Users/antoine/Code/safari/OpenSource/Source/WebCore/style/StyleTreeResolver.cpp:922

While we correctly deal with a `1rem` value we get the incorrect `FontDescription` inside `computeUnzoomedNonCalcLengthDouble` where the value is `96px`, which is the computed value on the parent element, not the root element.

I don't think animations are really a factor here, this look like a CSS bug with `rem` unit resolution.

[Antoine Quint]

I'm not sure why we enter the `applyAnimations()` branch which calls `applyCascadeAfterAnimation()` since in this case animations have no effect at all. This was changed in 271268@main.

[Antti Koivisto]

Pull request: https://github.com/WebKit/WebKit/pull/45596

[EWS]

Committed 295395@main (c7d5be16b583): <https://commits.webkit.org/295395@main>

Reviewed commits have been landed. Closing PR #45596 and removing active labels.