[css-properties-values-api] Substitution behavior.#13193
Merged
chromium-wpt-export-bot merged 1 commit intomasterfrom Sep 25, 2018
Merged
[css-properties-values-api] Substitution behavior.#13193chromium-wpt-export-bot merged 1 commit intomasterfrom
chromium-wpt-export-bot merged 1 commit intomasterfrom
Conversation
wpt-pr-bot
approved these changes
Sep 24, 2018
Collaborator
wpt-pr-bot
left a comment
There was a problem hiding this comment.
Already reviewed downstream.
ce0e606 to
c3105cc
Compare
Currently, registered properties substitute into other values exactly like specified (like unregistered properties). This means that, for a <length>-registered property with a specified value "10em" (for instance), when that property is substituted via var()-reference, the tokens "10em" are inserted. This is not correct, and produces the wrong result with e.g. inherited values. This CL changes that, by implementing "absolutization" of registered custom properties: a process which calculates the computed value of the property, and then produces a token stream usable for substitution which is equivalent to the computed value. * Currently we resolve var()-references on all custom properties before applying high-priority properties. This is no longer possible, because the true value of a custom property (also unregistered) can not be known until the font has been updated. Consider: --reg-len: 1em; --unreg:var(--reg-len). Here, the computed value of --unreg should be "16px" (assuming a font-size of 16px), and not "1em". If we destructively resolve --unreg before the font size is known, we end up with the wrong tokens. Hence the resolution process has been moved to after the font has been updated. * Custom properties must also be usable from high-priority properties. Since they are no longer resolved beforehand, they are resolved "on the fly", non-destructively. "Non-destructively" means that resolved token streams are not stored on ComputedStyle, such that any var()-references are kept for the _real_ resolution pass after the font has been updated. This works, because the high-priority properties do not require proper "absolute substitution" to produce the correct value. Note that font-size is special, in that font-relative units may not be used if they arrive via a registered custom property. * There is a special resolving pass for registered custom properties (ComputeRegisteredVariables). This pass produces non-absolute CSSValues for calculation of animation. * Parsing of registered properties now happens entirely in CSSVariableResolver, and no longer in variable.cc. Having this in multiple places is just confusing. R=futhark@chromium.org Bug: 641877 Change-Id: Ic705d0808ffcea0ae5db02fb20870767175bb706 Reviewed-on: https://chromium-review.googlesource.com/1240274 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Anders Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#593902}
c3105cc to
2904a8e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, registered properties substitute into other values exactly
like specified (like unregistered properties). This means that, for
a <length>-registered property with a specified value "10em" (for
instance), when that property is substituted via var()-reference,
the tokens "10em" are inserted. This is not correct, and produces the
wrong result with e.g. inherited values.
This CL changes that, by implementing "absolutization" of registered
custom properties: a process which calculates the computed value of
the property, and then produces a token stream usable for substitution
which is equivalent to the computed value.
Currently we resolve var()-references on all custom properties
before applying high-priority properties. This is no longer
possible, because the true value of a custom property (also
unregistered) can not be known until the font has been updated.
Consider: --reg-len: 1em; --unreg:var(--reg-len). Here, the
computed value of --unreg should be "16px" (assuming a font-size
of 16px), and not "1em". If we destructively resolve --unreg
before the font size is known, we end up with the wrong tokens.
Hence the resolution process has been moved to after the font
has been updated.
Custom properties must also be usable from high-priority properties.
Since they are no longer resolved beforehand, they are resolved
"on the fly", non-destructively. "Non-destructively" means that
resolved token streams are not stored on ComputedStyle, such that
any var()-references are kept for the real resolution pass after
the font has been updated.
This works, because the high-priority properties do not require
proper "absolute substitution" to produce the correct value.
Note that font-size is special, in that font-relative units may
not be used if they arrive via a registered custom property.
There is a special resolving pass for registered custom properties
(ComputeRegisteredVariables). This pass produces non-absolute CSSValues
for calculation of animation.
Parsing of registered properties now happens entirely in
CSSVariableResolver, and no longer in variable.cc. Having this in
multiple places is just confusing.
R=futhark@chromium.org
Bug: 641877
Change-Id: Ic705d0808ffcea0ae5db02fb20870767175bb706
Reviewed-on: https://chromium-review.googlesource.com/1240274
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593902}