LibWeb: CSS variables don't support CSS-wide keywords correctly

[aplefull]

Currently css variables don't support second argument. An optional second argument to the function should work as a fallback value.

Example:

<style>
    :root {
        --a: initial;
    }

    body {
        background: var(--a, crimson);
    }
</style>

Chrome	Ladybird
	

[AtkinsSJ]

I don't understand this test - initial is a valid value, so it shouldn't use the fallback. But we're probably doing something wrong if Chrome and Firefox are behaving like that. :thinking:

[aplefull]

Oh wait, you are right, this is a weird example. This seems to happen with all CSS wide keywords: initial | inherit | unset | revert | revert-layer. For some reason, both Chrome and Firefox return an empty string when querying getComputedStyle(document.body).getPropertyValue('--a') when --a is set to any of the keywords above, which is probably why var uses a fallback value. Haven't figured out why this happens yet.

[AtkinsSJ]

Ah... From the spec:

The CSS-wide keywords can be used in custom properties, with the same meaning as in any another property.

Note: That is, they’re interpreted at cascaded-value time as normal, and are not preserved as the custom property’s value, and thus are not substituted in by the corresponding variable.

So, --a: inherit should inherit the parent's value of --a, and then that's what should be substituted in. In the example, there is no parent value for --a, so it becomes the "guaranteed-invalid value", and the fallback argument would be used.

So yeah, we're doing this wrong. Thanks for finding it! I guess during style computation we need to examine the values of any custom properties, and apply the CSS-wide keywords if they're set to them.

[Gingeh]

It seems like the problem comes from expanding the variables in the parser rather than at computed value time as specified, this could be solved by either moving variable expansion to the style computer or by attempting to handle them in the parser

[AtkinsSJ]

I think there are two parts to this:

1. In Parser::parse_css_value(), we should rearrange the code to still try and parse_builtin_value() even for custom properties.
2. Handle those while cascading, in cascade_custom_properties() probably. I noticed yesterday that we're already not cascading those quite right: we do a single pass, but should do one pass for regular variables and a second pass for !important variables afterwards.

There might be some rough edges with this, I don't know if we always expect custom_properties to hold UnresolvedStyleValues.