Bug 194749

Summary: Relative font size values (em) within CSS animations compound
Product: WebKit Reporter: Scott Kellum <scott>
Component: AnimationsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: dino, graouts, graouts, koivisto, litherum, mail, scott, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 12   
Hardware: All   
OS: All   
URL: https://codepen.io/scottkellum/pen/7c4e82c2744e9dfc0caf706bc525d83f?editors=1100
See Also: https://bugs.webkit.org/show_bug.cgi?id=156684
https://bugs.webkit.org/show_bug.cgi?id=194765
https://bugs.webkit.org/show_bug.cgi?id=213125
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch
none
patch
none
patch none

Scott Kellum
Reported 2019-02-16 14:05:18 PST
If the font size is set in `em` and the font size within the keyframes of an animation are set in `em` then the values will compound instead of interpolating. Here is an example: https://codepen.io/scottkellum/pen/7c4e82c2744e9dfc0caf706bc525d83f Behavior in Safari 12 (iOS and Desktop): the text grows and shrinks. Expected behavior: the animation is between 2em and 2em so it should do nothing. No change in size should occur.
Attachments
patch (6.25 KB, patch)
2020-06-10 08:20 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (6.28 KB, patch)
2020-06-10 08:21 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (130.57 KB, patch)
2020-06-11 05:24 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (128.62 KB, patch)
2020-06-11 07:13 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (132.18 KB, patch)
2020-06-12 04:15 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (137.57 KB, patch)
2020-06-12 05:52 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Scott Kellum
Comment 1 2019-02-16 14:11:08 PST
Note on importance: This is a blocker/critical for me because I’m building a fluid typesetting system on top of animations and users want to set text in ems, preventing easy integrations into some websites.
Scott Kellum
Comment 2 2019-02-17 13:29:05 PST
Possibly related bug: https://bugs.webkit.org/show_bug.cgi?id=194765
Simon Fraser (smfr)
Comment 3 2019-02-18 11:34:15 PST
Different bug. This one reproduces without Web Animations enabled. We're not resolving 'em' styles correctly in animation keyframes.
Radar WebKit Bug Importer
Comment 4 2019-02-18 11:34:35 PST
<rdar://problem/48171898>
Kevin Lorenz
Comment 5 2019-04-02 06:35:38 PDT
This has already caused a lot of frustration on my end. Typo centered layout design is dependent on this to work correctly :(
Antti Koivisto
Comment 6 2020-06-10 08:20:00 PDT
Created attachment 401543 [details] patch
Antti Koivisto
Comment 7 2020-06-10 08:21:20 PDT
Created attachment 401544 [details] patch
Antoine Quint
Comment 8 2020-06-10 10:20:10 PDT
Sooo many new WPT PASS results!
Antti Koivisto
Comment 9 2020-06-11 05:24:27 PDT
Created attachment 401635 [details] patch
Antti Koivisto
Comment 10 2020-06-11 07:13:26 PDT
Created attachment 401640 [details] patch
Antti Koivisto
Comment 11 2020-06-12 04:15:51 PDT
Created attachment 401720 [details] patch
Antti Koivisto
Comment 12 2020-06-12 05:52:46 PDT
Created attachment 401721 [details] patch
Antoine Quint
Comment 13 2020-06-12 06:11:22 PDT
Comment on attachment 401721 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=401721&action=review > LayoutTests/animations/keyframe-em-unit.html:5 > +div { > + font-size: 2em; > + animation: div 3s infinite alternate; > +} You can make the duration any value and add `freeze` to ensure the animated style will override the base style. You can remove `alternate` for sure. > Source/WebCore/style/StyleTreeResolver.cpp:324 > + // FIXME: Remove this hack and pass the parent style via updateCSSAnimationsForElement. Can you file a bug and assign it to me to do this refactor?
EWS
Comment 14 2020-06-12 07:40:42 PDT
Committed r262946: <https://trac.webkit.org/changeset/262946> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401721 [details].
Note You need to log in before you can comment on or make changes to this bug.