1864736 - [css-properties-values-api] ComputedPropertyValue should be an enum

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Zach Hoffman [:zrhoffman]]

Currently, computed registered custom properties are computed when substituting variable references, but the computed value is converted back to a String to be stored as a ComputedPropertyValue (VariableValue) in the custom properties map, only for the value to converted back to its computed type when applying longhands.

Instead, ComputedPropertyValue should be something like

enum { Unparsed { css, url_data, serialization_types, references }, Parsed(..) }

so the computed value can be reused later.

Comment 1

[Zach Hoffman [:zrhoffman]]

Emilio, how should substitutions work if we store the computed representation as-is? For example, if we have something like

@property --my-property {
 syntax: '<length>';
 inherits: true;
 initial-value: 5px;
}
.my-class {
 --another: calc(4px + var(--my-property));
 width: var(--another)
}

, we would still need a String representation of --my-property when performing a substitution for --another, unless the value of --another is computed when performing that substitution.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Yeah, I was assuming we'd serialize the internal value for substitution. But maybe we can just parse/compute the values when interpolating instead, if that makes it easier.

Comment 3

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1864736 - Rustfmt sources changed in bug 1864736. r=#style

Comment 4

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1864736 - Add URL data to registered custom property struct. r=zsun,fredw,#style

The URL data is necessary to uncompute the value for animation. This was
handled previously by adding the URL data to CustomAnimatedValue.
However, now that a registered custom property is passed to
CustomAnimatedValue::from_computed instead of a VariableValue, that
registered custom property should include URL data instead.

Comment 5

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1864736 - Eliminate redundant URL data. r=zsun,fredw,#style

Now that registered custom property values contain URL data, URL data is
eliminated as an argument in some places, and the ComputedValue.url_data
field is removed.

Comment 6

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1864736 - Use ComputedRegisteredValue for custom property maps. r=zsun,fredw,#style

Although the inner value of the Parsed variant is the
ComputedRegisteredValueInner type, custom property maps hold only the
ComputedRegisteredValueInner::Universal variant to keep behavior
unchanged for now.

Comment 7

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1864736 Remove Arc from CustomPropertiesMap value r=zsun,fredw,#style

An Arc is still around VariableValues, and the extra Arc would be
redundant.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cddfe0bce3a
Rustfmt sources changed in bug 1864736. r=firefox-style-system-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/eefa88f79557
Add URL data to registered custom property struct. r=zsun
https://hg.mozilla.org/integration/autoland/rev/cc6c51d8d712
Eliminate redundant URL data. r=zsun
https://hg.mozilla.org/integration/autoland/rev/3f19bcf6a34f
Use ComputedRegisteredValue for custom property maps. r=firefox-style-system-reviewers,emilio

Comment 9

[Cristian Tuns]

Backed out for causing build bustages.

• Backout link
• Push with failures
• Failure Log
• Failure line: error[E0599]: no method named is_parsed found for reference &Value<GenericValueComponent<CSSPixelLength, f32, Percentage, ..., ..., ..., ..., ..., ..., ..., ..., ...>> in the current scope

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6078a631438d
Rustfmt sources changed in bug 1864736. r=firefox-style-system-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/9e5d71c9324f
Add URL data to registered custom property struct. r=zsun
https://hg.mozilla.org/integration/autoland/rev/f7ddfd619c51
Eliminate redundant URL data. r=zsun
https://hg.mozilla.org/integration/autoland/rev/63d81375ed6e
Use ComputedRegisteredValue for custom property maps. r=firefox-style-system-reviewers,emilio

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6078a631438d
https://hg.mozilla.org/mozilla-central/rev/9e5d71c9324f
https://hg.mozilla.org/mozilla-central/rev/f7ddfd619c51
https://hg.mozilla.org/mozilla-central/rev/63d81375ed6e

Comment 12

[Emilio Cobos Álvarez (:emilio)]

You rock, Zach!