We resolve calc expressions and use appropriate units by computing
ToComputedValue trait, computing each component's value, then converting
it back to that component's type, before making a new VariableValue from
the computed value's CSS string to store in the computed properties map.
Details
- Reviewers
fredw zsun emilio - Commits
- rMOZILLACENTRAL2609670e6e11: Bug 1846632 - [css-properties-values-api] Use canonical values for font…
- Bugzilla Bug ID
- 1846632
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Build Status
Buildable 586527 Build 684534: arc lint + arc unit
Event Timeline
This revision requires a Testing Policy Project Tag to be set before landing. Please apply one of testing-approved, testing-exception-unchanged, testing-exception-ui, testing-exception-elsewhere, testing-exception-other. Tip: this Firefox add-on makes it easy!
| servo/components/style/properties_and_values/value.rs | ||
|---|---|---|
| 69 | It's very weird that we implement ToComputedValue returning Self as the computed value type. The right way to do this is making the type generic over the types that are different between computed and specified. So you'd have something like: enum GenericValueComponent<Length, Number, Percentage, LengthPercentage, Color, Image, Url, Integer, Angle, Time, Resolution, Transform> {And specialize it for specified and computed types. Did you try that, and is there a reason that wouldn't work? This is fine for now but eventually we should probably do something along those lines, perhaps. | |
| 148 | There shouldn't be a need to have an intermediate vec here right? | |
| 152 | Can this just clone the list (self.clone()) rather than allocating a new one? | |
| 157 | This is not a slice either. | |
| 258 | Yeah, thanks for filing the follow-up. | |
| servo/components/style/properties_and_values/value.rs | ||
|---|---|---|
| 69 |
I was thinking of the specified and computed values as the same type because ComputedValue::compute effectively both accepts and returns token streams, but separating the specified and computed types would definitely be an improvement.
Added a TODO for bug 1857716 and submitted D190382/D190383. | |
| 148 | Right, intermediate vec removed. | |
| 152 | It can indeed, changed to computed.clone() (but changed it back in D190383). | |
| servo/components/style/properties_and_values/value.rs | ||
|---|---|---|
| 148 | I don't think you need .into_iter() | |
| servo/components/style/properties_and_values/value.rs | ||
|---|---|---|
| 148 | .into_iter() removed. | |