| Summary: | [web-animations] line-height should not transition from default value to a number | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||
| Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | dino, graouts, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://github.com/web-platform-tests/wpt/pull/38416 | ||||||
| Attachments: |
|
||||||
|
Description
Antoine Quint
2023-02-08 02:27:27 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.
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 :) 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. Pull request: https://github.com/WebKit/WebKit/pull/9807 Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/38416 Committed 260028@main (221eb1d58b67): <https://commits.webkit.org/260028@main> Reviewed commits have been landed. Closing PR #9807 and removing active labels. |