Skip to content

[css-variables] Should var(var(--x)) work? #11144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tabatkins opened this issue Nov 1, 2024 · 15 comments
Open

[css-variables] Should var(var(--x)) work? #11144

tabatkins opened this issue Nov 1, 2024 · 15 comments

Comments

@tabatkins
Copy link
Member

The Variables spec is currently not entirely clear whether var(var(--x)) is intended to work. As an example:

.foo {
 --color: green;
 --var-name: --color;
 color: var(var(--var-name));
}

Should this be valid, resulting in color: var(--color);, and ultimately color: green;?

Chrome's implementation, iiuc, currently allows nested var() references only in the <declaration-value> part, because that chunk isn't interpreted at all - they're processed as part of fallback processing. Per Variables (now Values), you have to verify that the var() is syntactically valid at parse time; our engineers interpreted that as "you must fully satisfy the specified grammar". This isn't an unreasonable position, but it does mean we're slightly limited in how var() can be used.

The larger issue is that this has knock-on effects for other arbitrary-substitution functions. For example, in if(), if we apply the same criteria, it's allowed to use a var() in the value of a clause (which is a <declaration-value>, so a var() there just isn't interpreted at parse-time), but not as part of the test (which has a specific grammar); --test: media(width < 600px); color: if(var(--test): green) would be invalid. I think that's pretty surprising, and authors would run into it and complain.

So, I believe we should fix up the definition a bit to allow for nested var() delaying the syntax checking of outer var()s.

/cc @LeaVerou @andruud

@Loirooriol Loirooriol added the css-variables-1 Current Work label Nov 2, 2024
@andruud
Copy link
Member

andruud commented Nov 4, 2024

I feel like there should be a problem with this proposal ... but I can't think of one.

I assume this would then extend generally to all substitution functions, so e.g. both var(attr(...)) and attr(var(...)) are valid?

@Loirooriol
Copy link
Contributor

Loirooriol commented Nov 4, 2024

Note this implies that you can't just build the full directed dependency graph before resolving var(). For example:

.foo {
  --a1: --b2;
  --a2: var(var(--a1));
  --b1: --a2;
  --b2: var(var(--b1));
}

--a2 and --b2 are cyclic but we don't know until we have started doing some substitutions.

@LeaVerou
Copy link
Member

LeaVerou commented Nov 4, 2024

I feel like there should be a problem with this proposal ... but I can't think of one.

I assume this would then extend generally to all substitution functions, so e.g. both var(attr(...)) and attr(var(...)) are valid?

Also counter(var(--foo)) (no idea if this works today).

@tabatkins
Copy link
Member Author

I assume this would then extend generally to all substitution functions, so e.g. both var(attr(...)) and attr(var(...)) are valid?

Yes, this would be a generic behavior for all arbitrary substitution functions.

Note this implies that you can't just build the full directed dependency graph before resolving var(). For example:

Hm, that's certainly a bit more annoying. @andruud, does that alter your opinion at all? I could see us specifically requiring var()'s property name argument being required to be literal, while otherwise allowing substitution functions to do whatever. (And similarly extend that to any other substitution functions that add dependencies to the graph; I think maybe only inherit() would be affected right now.)

But maybe it's fine and we can just do a later check. The effects of a dependency cycle don't kick in until resolution time, anyway.

Also counter(var(--foo)) (no idea if this works today).

counter() isn't an arbitrary substitution function, that should work just fine today.

@andruud
Copy link
Member

andruud commented Nov 5, 2024

Note this implies that you can't just build the full directed dependency graph before resolving var(). For example:

Hm, that's certainly a bit more annoying. @andruud, does that alter your opinion at all?

We don't rely on "statically" figuring out the dependency graph in Blink. It seems pretty hard to do that already, for example:

@layer {
  div {
    --x: var(--y);
  }
}
@layer {
  div {
    --x: var(--unknown, revert-layer); /* Cycle with --y, yet no literal mention of --y */
    --y: var(--x);
  }
}

We should check other implementations. @Loirooriol, do you know if there's an impl. that needs to build the dependency graph early?

@Loirooriol
Copy link
Contributor

I don't know, I just tried to think about possible implications.

@tabatkins
Copy link
Member Author

tabatkins commented Nov 5, 2024

Another wrinkle here: currently, if a property contains a substitution function, it's always considered valid at parse time (and will be considered actually valid or IACVT at computed-value time), unless the substitution function itself is grammatically invalid. That's the only way to have a var-containing property actually get considered invalid at parse time.

With this change, the concept of "invalid at parse time" becomes harder to reach. Unless we add some even more special behavior (which I'd prefer not to), having a var() in the fallback will cause the outer var() to not be syntax checked until later. That is:

.invalid-child {
  color: var(--foo, var(1));
  /* today: invalid at parse time (due to `var(1)`)
     after change: still invalid at parse time, same reason */
}
.invalid-parent {
  color: var(1, var(--foo));
  /* today: invalid at parse time (due to `var(1, ...)`)
     after change: valid at parse time, IACVT later */
}
.unknown-invalid {
  color: var(var(--foo));
  /* today: invalid at parse time
     after change: valid at parse time, might be valid or IACVT later, depending on --foo value */
}

This is a lot more arbitrary than I first thought when I wrote that invalidity requirement. I think we should drop it, and make the presence of a substitution function just always treat the property as valid, regardless of what's inside its parens. This would make all of the above be valid at parse time, then the first two will be IACVT (and the third may or may not be).

This would mean that color: var(1); would be considered valid at parse time even tho it's clearly going to fail, but I think that's an acceptable tradeoff vs gaining predictability in whether something is parse-invalid or IACVT. This is similar to color: definitely-invalid 1 2 3 var(--foo); being valid at parse time and just IACVT, even tho we can theoretically tell that it's impossible for it to be valid even at parse time.

This would only be a behavior change for code that's currently invalid; we usually consider this kind of behavior change to be safe to make by default.

@tabatkins
Copy link
Member Author

tabatkins commented Nov 5, 2024

So the actual processing model then becomes:

  1. The presence of a substitution function (identified solely by the function name, not its contents) in a value causes it to be valid at parse time, and left otherwise uninterpreted until substitution time.
  2. During substitution, we substitute from the inside out - if substitution functions are nested, the innermost one is processed, then its parent sees those results and gets processed, etc.
  3. If, once a subsitution function has had any nested functions replaced, it's now grammatically invalid, it immediately resolves to the guaranteed-invalid value.
  4. The guaranteed-invalid value is, in fact, perfectly valid as part of a <declaration-value> or similar production, so var(--foo, var(--does-not-exist)) works correctly if --foo exists.
  5. We slightly tweak variable processing so that, rather than an invalid variable immediately triggering IACVT directly, the IACVT happens just due to the presence of a guaranteed-invalid value in the property's value causing it to fail to parse correctly.

This is also more forwards-compatible. Today, you could write var(--foo, if(invalid)) and it would be just fine; the if(invalid) just looks like some arbitrary uninterpreted content. If we didn't make this change, tho, then when if() became recognized as a new substitution function, it would start getting grammar-checked, and cause the whole property to be invalid at parse time.

Similarly, if authors are using custom properties today to shuttle around random non-CSS values (for example, JS...) and those values contain something that happens to look like a substitution function (like if(x < 5) {...}), without this change that would suddenly switch from being perfectly valid to being invalid at parse time, once if() support was added. With this change it'll still be IACVT, so it won't inherit, but it'll at least stick around on the element as a specified value. Never mind, since the guaranteed-invalid value is now valid as part of <declaration-value>, the custom property will, itself, remain valid and will inherit normally. It's just attempting to use it in CSS via a var() will treat it as invalid (because it contains the guaranteed-invalid value) and trigger the var()'s fallback instead. Yay, that is perfect future compat!

@andruud
Copy link
Member

andruud commented Nov 5, 2024

That mostly makes sense, but I don't understand why (4) and (5) are needed, and what exactly what you're describing here.

Are you saying that for:

#parent {
  --x: foo;
}
#child {
  --x: var(--unknown);
}

the computed value of --x on #child is now <guaranteed-invalid> rather than foo?

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Nov 5, 2024

@andruud That is already the case, unless ‑‑x is a registered custom property with a non‑universal syntax definition.

@tabatkins
Copy link
Member Author

tabatkins commented Nov 6, 2024

4 is necessary because the timing of substitution of var()s in fallback would change. Right now, with var(--foo, var(--bar)), you know exactly what the bounds of the fallback is, so you can avoid actually interpreting it unless it's needed. If var(--bar) doesn't exist or whatever, you won't even know unless fallback actually gets triggered and you have to process the fallback tokens.

With this change, you won't always be able to do that, so you'll have to do the substitution eagerly, as part of processing the outer var(). You have to do substitution inside-out, so var(--foo, var(--bar)) might turn into var(--foo, guaranteed-invalid), and that needs to still be grammatically valid so that it can sub in --foo's value if possible; the invalidity of the fallback shouldn't matter unless you actually use it.

5 is a consequence of that, just removing the currently specified behavior that the presence of a guaranteed-invalid value immediately makes the property IACVT.

Are you saying that for [...] the computed value of --x on #child is now rather than foo?

Right, like @ExE-Boss says, that's luckily already the case; see https://drafts.csswg.org/css-values-5/#invalid-substitution. Here's a testcase demonstrating it:

<!DOCTYPE html>
<style>
#parent {
  --x: green;
}
#child {
  --x: var(--unknown);
  background: var(--x);
}
div { padding: 20px; margin: 20px; border: thin dotted; }
</style>
<div id=parent><div id=child></div></div>

(The #child ends up without a background color, because --x is invalid on it. It doesn't inherit the --x: green value from its parent.

I think, to be a little more specific, the rule would be that if a substitution function fails to parse, it's substitutes as itself, but with all of its tokens "marked as meaningless" - meaningless tokens don't have any behavior and don't match any grammar except the "anything goes" ones like <declaration-value>. If a substitution function's "result" (from its substitution algo) contains any meaningless tokens, it triggers fallback if it also returned a "fallback" value; if no fallback was specified, it just substitutes as normal (but with some meaningless tokens).

So, yeah, the computed value will be var(--unknown), but it won't be reinterpreted on a child that happens to have a --unknown defined; it'll always be a meaningless var(--unknown) set of tokens.

This has a few effects, all of which are good imo:

  • It means we can abstract the full contents of a var() into a custom prop and substitute it in later, with no behavior change.

    That is, --foo: green; color: var(--foo, var(--unknown)); works today; this change ensures that abstracting that to --foo: green; --contents: --foo, var(--unknown); color: var(var(--contents)); also works exactly the same way. When we resolve --contents, the var(--unknown) fails, causing the computed value of --contents to remain exactly --foo, var(--unknown), but with the var(--unknown) tokens marked as "meaningless". Then substitution into color produces color: var(--foo, var(--unknown)); since the "meaningless" tokens are in the fallback's <declaration-value>, that still grammar-checks successfully, and then substitution succeeds.

    Because the tokens are now "meaningless", they're not actually treated as a substitution function in color; importantly, if --contents is inherited into a child who does have --unknown defined, it won't suddenly start successfully substituting.

  • There's now no meaningful difference in behavior between --foo: rgb(invalid stuff); and --foo: var(invalid stuff);; authors no longer really see a behavior difference for invalid content based on the class of function.

  • Existing custom props containing non-CSS content that happens to look like a CSS function usually won't suddenly change behavior if we make one of those functions a substitution function. The only exceptions are (a) if they're unlucky enough to actually match the function's grammar, or (b) they don't match the function's grammar, and are substituted into another universal-syntax custom prop using var() with a fallback. So a random custom prop containing --foo: if(1 < 2) { ... }; and one containing --foo: my-if(1 < 2) { ... }; will continue to be treated the same after we start parsing if() as a substitution function (since that's invalid syntax for it) unless you do var(--foo, something-else) (in which case they'll start triggering the fallback when the previously wouldn't).

  • Failed substitution can still be caught by var() with a fallback argument (or similarly, inherit() with a fallback), so existing code expecting such a failure to cause the custom property to become the guaranteed-invalid value (as the spec says today) and replacing it with a fallback will still work.

@andruud
Copy link
Member

andruud commented Nov 6, 2024

@andruud That is already the case, unless ‑‑x is a registered custom property with a non‑universal syntax definition.

@ExE-Boss Ah, right, I forgot that we changed to that behavior in #6006. (I even changed it myself ...)

That's all good, then.

Right now, with var(--foo, var(--bar)), you know exactly what the bounds of the fallback is, so you can avoid actually interpreting it unless it's needed. If var(--bar) doesn't exist or whatever, you won't even know unless fallback actually gets triggered and you have to process the fallback tokens.

That's not true. We (currently) have to resolve any var() in the fallback to look for cycles, even when the fallback is not taken. However, the fallback being (or containing) guaranteed-invalid has no effect unless the fallback is taken.

the invalidity of the fallback shouldn't matter unless you actually use it.

It is already the case.

I think, to be a little more specific, the rule would be that if a substitution function fails to parse, it's substitutes as itself, but with all of its tokens "marked as meaningless" - meaningless tokens don't have any behavior and don't match any grammar except the "anything goes" ones like <declaration-value>. If a substitution function's "result" (from its substitution algo) contains any meaningless tokens, it triggers fallback if it also returned a "fallback" value; if no fallback was specified, it just substitutes as normal (but with some meaningless tokens).

This sounds awful, and likely a non-starter. But I also don't see why it's needed.

It's possible that we actually agree on how this should all work, but with entirely different mental models. I need to process what you wrote a bit more and see if I can come up with an alternative way to describe the behavior ...

@tabatkins
Copy link
Member Author

This sounds awful, and likely a non-starter. But I also don't see why it's needed.

lol. Well, the requirements I'm trying to satisfy are, roughly in decreasing priority:

  1. No easily observable change to current invalid var() behavior - if a custom property contains an invalid var(), and is then substituted into a real property with var(), either that real property should become IACVT (if there is no fallback), or it should trigger fallback (if there was some).
  2. Moving arbitrary contents out of a substitution function into a custom property, and then using var() to sub it back into that substitution function, should work in all cases. (That is, behavior of an invalid or failed substitution function shouldn't depend on whether it's known to be in a <declaration-value> grammar or not.)
  3. As much as possible, a busted substitution function should act similarly to a busted non-substitution function.
  4. Existing custom properties holding non-CSS stuff should change as little as possible if we add a new substitution function matching something they contain, especially if they're now invalid per the new function's grammar.

There's many ways to satisfy (1), but I think (2) is a lot stricter, and (3)/(4) basically force the same requirements.

Let me rephrase the proposal a bit, maybe you'll like it better this way:

  • Substitution is now defined to be recursive, rather than iterative; we resolve starting from the innermost and moving outward. (Rather than today's behavior, where all "active" substitution functions are top-level, and fallback can drop more substitution functions in which then have to be resolved as well.) We drop the grammar-checking of substitution functions at parse time entirely - they grammar-check themselves during resolution only.

  • Whenever a substitution function is either grammatically invalid, or is valid but fails to substitute (say, it's a cyclic or animation-tainted var()) and doesn't have a fallback, it's replaced by a guaranteed-invalid value that serializes to the same string as the entire original function. (This changes the final "Otherwise" branch of the substitution algo.)

  • The guaranteed-invalid value is, in fact, valid in <declaration-value> and similar "anything goes" grammars. This includes the grammar of universal-syntax custom properties; we remove the rule that explicitly makes them IACVT when there's a substitution failure. It continues to be invalid for all other grammars. (Basically it's a new type of token that just won't match any grammars except those wide ones.)

  • We change the substitution algo to allow result and fallback to contain guaranteed-invalid values: if there is a fallback, and result has a GIV, then we replace with the fallback; otherwise (either no fallback, or the result doesn't have a GIV) we replace with the result. A GIV otherwise doesn't influence substitution directly; eventually it'll get grammar-checked (either by a containing substitution function, or by the containing property) and cause something to fail. (This changes the first two branches of the substitution algo.)

  • We similarly tweak the resolution algos (like for var()) to correctly interface with this.

    Specifically, var()'s resolution algo would become:

    1. Let |result| be the value of the custom property named by the function’s first argument, on the element the function’s property is being applied to.
    2. Let |fallback| be the value of the function's second argument, if specified; otherwise, let |fallback| be nothing.
    3. If the custom property named by the var()’s first argument is animation-tainted, and the var() is being used in a property that is not animatable, set result to the guaranteed-invalid value.
    4. Result |result| and |fallback|.

    (Only step 2 changed a little bit, to make an unspecified fallback actually be missing.)

This does change the serialization of properties that include a failing substitution function; currently they're specified to be the empty string. We could probably special-case that to preserve the behavior specifically for serializing a custom property (while substitution continues using the actual values, which will serialize as a non-trivial string).

@AtkinsSJ
Copy link
Contributor

AtkinsSJ commented Mar 5, 2025

Bumped into this today while trying to implement the Arbitrary Substitution Function concept. I don't know how useful my opinion is but I'll add it anyway. 😅

  • Intuitively I expected ASFs to be able to go inside any part of another ASF, which is how I ended up finding this issue in the first place.
  • Grammar checking ASFs at parse time didn't seem to have much benefit. IMO it's clearer to authors if using an ASF always makes the declaration valid and cascaded until it's checked at computed-value time.
  • What Tab's proposed seems to solve a lot of issues and I don't see any problems with it. (Though admittedly not all the details are entirely clear in my head.)
  • Defining an algorithm for substituting depth-first is probably not going to be fun, but it'd be nice to have it also be more concrete. The current algorithm's "For each arbitrary substitution function func in value:" is a bit vague for my liking. Maybe this isn't true in all engines, but at least in Ladybird the internal representation for a declaration value containing ASFs is the list of component-values, and I assume other engines do something similar. In which case it could be written in terms of iterating through CVs.

tabatkins added a commit that referenced this issue Mar 26, 2025
…endency graph with them tracking a stack of dependencies as they evaluate. (Should be editorial, but it's a deep rewrite.) Give arb-sub functions a 2-stage grammar, allowing them to control when their bits are evaluated. Add spread syntax to arb-sub functions. #11144 #11500 #10679
@tabatkins
Copy link
Member Author

Per the resolution in #11500, which is now editted into the spec, var(var(--x)) now works. (As does var(...var(--x)), if --x is meant to supply both the property name and the fallback.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants