WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
251911
[web-animations] line-height should not transition from default value to a number
https://bugs.webkit.org/show_bug.cgi?id=251911
Summary
[web-animations] line-height should not transition from default value to a nu...
Antoine Quint
Reported
2023-02-08 02:27:27 PST
Created
attachment 464906
[details]
Reduction We incorrectly interpolate the "line-height" property when it animates from its initial value (normal) to a number. This was reported in
https://stackoverflow.com/questions/75151629/css-transitions-in-safari/
.
Attachments
Reduction
(424 bytes, text/html)
2023-02-08 02:27 PST
,
Antoine Quint
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Antoine Quint
Comment 1
2023-02-08 02:27:47 PST
rdar://104346766
Antoine Quint
Comment 2
2023-02-08 02:33:24 PST
The problem is that BuilderConverter::convertLineHeight() loses the `<number>` type here: if (primitiveValue.isNumber()) return Length(primitiveValue.doubleValue() * 100.0, LengthType::Percent); Additionally, the `normal` value is also converted to a percent value here: if (valueID == CSSValueNormal) return RenderStyle::initialLineHeight(); Where RenderStyle::initialLineHeight() builds this value: // Returning -100% percent here means the line-height is not set. static Length initialLineHeight() { return Length(-100.0f, LengthType::Percent); } This means that when we get to deciding whether it's OK to interpolate between `normal` and `0`, which is what the original Stack Overflow example does, we consider two percent values and decide it's OK to interpolate. We are going to need to: 1. identify the `normal` value with special logic 2. preserve the `number` type somehow Finally, looking at the spec for `line-height` I see it takes in `<length-percentage>` as a value and yet the animation wrapper does not specify this. So there's likely another bug here where we would fail to yield a transition between "line-height: 10px" and "line-height: 10%" I expect.
Antoine Quint
Comment 3
2023-02-08 02:38:01 PST
Actually, a transition between "line-height: 10px" and "line-height: 10%" works because we convert the "10%" to a Fixed value as well so we end up having the same type. This code is pretty weird :)
Antoine Quint
Comment 4
2023-02-08 03:16:00 PST
Alright, so all we need to do is to have a custom `canInterpolate()` override for `line-height` and return false if any value maps to RenderStyle::initialLineHeight(). The default LengthPropertyWrapper logic will otherwise apply since <number> values map to LengthType::Percent and <length-percentage> values map to LengthType::Fixed.
Antoine Quint
Comment 5
2023-02-08 04:55:26 PST
Pull request:
https://github.com/WebKit/WebKit/pull/9807
Antoine Quint
Comment 6
2023-02-08 04:55:53 PST
Submitted web-platform-tests pull request:
https://github.com/web-platform-tests/wpt/pull/38416
EWS
Comment 7
2023-02-08 12:34:04 PST
Committed
260028@main
(221eb1d58b67): <
https://commits.webkit.org/260028@main
> Reviewed commits have been landed. Closing PR #9807 and removing active labels.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug