Amphtml: Validator: inline css custom variable

[bcrazydreamer]

There is a bug during inline style

<style amp-custom>
  .test{
    color: var(--clr);
  }
</style>

```html

````

I feel this kind of code is not invalid as per css and html rules

[sparhami]

What is the bug you are encountering? Is this failing to validate with the AMP validator?

[jridgewell]

/to @Gregable

[honeybadgerdontcare]

Inline styles has a whitelisted set of css properties. Using style="--clr: #f00" does cause a validation issue due to this (The property '--clr' in attribute 'style' in tag 'div' is disallowed).

We may want to consider adding additional properties.

[nightpool]

Why make CSS variables subject to a whitelist? It's really useful to be able to apply CSS variables at the inline level (for example, for dynamic content / background images) and then use them elsewhere in your styles.

[Gregable]

Quick follow up to describe what's going on here currently in the validator for future reference.

Validator rules for <style amp-custom> were added much earlier in the validator lifetime than the rules for style= attributes. When style= attributes were later added, we introduced new constraints for CSS in this scope. These constraints we'd like to apply to <style amp-custom> as well, but are limited in doing so due to backwards compatibility.

One of those new constraints is an allow-list of supported CSS declaration names. For example foo: bar is allowed inside a rule in <style amp-custom> for backwards-compatibility, but not inside style= attributes.

The validator sees --clr: #foo as a css declaration name that isn't defined in the allow-list. In neither context is the validator aware that this is a CSS variable.

It shouldn't be too difficult to add support for variable declarations, it would simply require adding support for arbitrary declarations starting with -- prefix. That said, I imagine that it's uncommon for someone to _define_ variables inside a style attribute, instead of a style tag, so I consider this issue of lowish priority.

[jridgewell]

When we do get around to this, please reserve the prefix --i-amphtml for our use.

[Gregable]

@jridgewell, you want to reserve it as a variable name prefix?

[honeybadgerdontcare]

@Gregable yes we should. Anything that could be named within html/css should automatically have i-amphtml reserved.

[bcrazydreamer]

@jridgewell is that reserve prefix mean this??

.test{
    color: var(--i-amphtml-clr);
    font-size : var(--i-amphtml-fontsize);
    margin : var(--i-amphtml-margin);
}

md5-5ee9f35c12e67820db03498c74ea1c20

[Gregable]

@bcrazydreamer , My understanding of @jridgewell 's comment is that the validator wouldn't allow the user style (either <style amp-custom> or style=) to use variables with the prefix --i-amphtml, so your examples would not be allowed, but if you used --i-foo-*, they would.

[bcrazydreamer]

@Gregable

.test{
    color: var(--i-foo-clr);
    font-size : var(--i-foo-fontsize);
    margin : var(--i-foo-margin);
}

md5-3cafc4fe3203d63f4c3bb328cd40b621

its also invalid case by validator but valid by CSS and HTML should be handled by validator
#[Category: Validator]

[jridgewell]

Yes. We'll make --i-foo and --i-foo-x valid, but --i-amphtml and --i-amphtml-x will be invalid.

[itsmsc]

What is the time horizon for changing the validator rules?
I have integrated an editor for AMP stories based on the CSS variables into a CMS. In the end I had to find out that these are not valid AMP stories :-(

[newmuis]

/cc @mszylkowski this is the issue for using custom CSS properties in inline styles

For context, this would potentially be very useful for stories, as we produce several AMP components that are in shadow DOM, with custom properties for configuration

[Rich-Harris]

This is a very significant omission, and I really hope you can prioritise it soon.

That said, I imagine that it's uncommon for someone to _define_ variables inside a style attribute, instead of a style tag, so I consider this issue of lowish priority.

@Gregable this is not true at all; being able to control values per-element (or per-subtree) is one of the main reasons to use CSS custom properties!

[honeybadgerdontcare]

We're tracking this and seeing which AMP formats may be interested. b/177269431.

I'd appreciate any feedback around restricting the naming of custom properties. Would a regex of --[\w]?[\w-]* be sufficient for most use cases?

[holm]

It’s unclear to me reading this issue why there is any whitelist at all? Why not just remove it completely?

[honeybadgerdontcare]

@holm for security / privacy requirements of AMP documents.

[jridgewell]

@honeybadgerdontcare: --[\w]?[\w-]* seems odd. --[\w][\w-]* should capture every valid case, and excludes -- which is invalid. (And we need to exclude --i-amphtml*, so:

--(?!i-amphtml)[\w][\w-]*

[honeybadgerdontcare]

We've gone a slightly different route but it will enable custom properties for inline styles. It should be going live middle of next week. cl/355888443

[bcrazydreamer]

@honeybadgerdontcare can you mark the MR that include this issue fix or its under progress?

[honeybadgerdontcare]

It'll be pushed to GitHub by Tuesday/Wednesday.