[zoom] Make zoom continuously interpolatable behind a flag [chromium/src : main]
Jason Leo (Gerrit)
Jason Leo added 1 comment![]( "Open in Gerrit")
Related details
- Anders Hartvoll Ruud
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen added 3 comments![]( "Open in Gerrit")
if (!primitive_value || !primitive_value->IsPercentage()) {I'm not too familiar with which functions do what in the interpolation types, but we do we need a separate interpolation type for zoom, as opposed to opacity, because percentages are not turned into numbers at parse time?
}Isn't this code exercised? (missing coverage according to gerrit)
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/10872">I added "Needs Edits" to the issue. Ideally, this should be .tentative.html until the spec actually changes, but this is trivial enough to not do that.
Related details
- Jason Leo
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
一丝 (Gerrit)
一丝 added 1 comment![]( "Open in Gerrit")
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/10872">I added "Needs Edits" to the issue. Ideally, this should be .tentative.html until the spec actually changes, but this is trivial enough to not do that.
I can submit a PR to the CSSWG.
Jason Leo (Gerrit)
Jason Leo added 3 comments![]( "Open in Gerrit")
if (!primitive_value || !primitive_value->IsPercentage()) {I'm not too familiar with which functions do what in the interpolation types, but we do we need a separate interpolation type for zoom, as opposed to opacity, because percentages are not turned into numbers at parse time?
Right — opacity uses `ConsumeAlphaValue` → `ConsumeNumberOrPercent`, which normalizes a literal n% to a plain kNumber (val/100) at parse time, so by the time `CSSNumberInterpolationType::MaybeConvertValue` sees it, IsNumber() is true and the percent branch never executes.
`Zoom::ParseSingleValue` calls `ConsumePercent` directly (and separately `ConsumeNumber`), so percentages survive parse time as `kPercentage`. The base MaybeConvertValue rejects them with `!primitive_value->IsNumber()`. Hence the override here.
We could alternatively switch the parser to `ConsumeNumberOrPercent` and drop this subclass entirely, but that changes the specified-value representation of zoom: 50% (round-trips through getPropertyValue differently for parsed-but-not-yet-cascaded values). Keeping the override felt less invasive — happy to flip if you'd prefer the parser route.
Isn't this code exercised? (missing coverage according to gerrit)
Not sure, added another case of `sibling-index` for test `IsElementDependent` case
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/10872">一丝I added "Needs Edits" to the issue. Ideally, this should be .tentative.html until the spec actually changes, but this is trivial enough to not do that.
I can submit a PR to the CSSWG.
- Rune Lillesveen
- 一丝
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |