Skip to content
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

[cssom-1] How to serialize specified value of properties with a var()? #6484

Open
tabatkins opened this issue Jul 29, 2021 · 13 comments
Open
Labels
cssom-1 Current Work

Comments

@tabatkins
Copy link
Member

(continuing topic from #5685)

It's not currently well-specified how serialization for specified values of non-custom properties should behave when they have a var() reference.

That's a lot of words! I'm referring to this example:

<!DOCTYPE html>
<body style="color: /* foo */ var(--foo); --foo: blue;">
<script>
document.body.textContent = document.body.style.getPropertyValue("color");
</script>

Currently, Chrome serializes this as var(--bar), while Firefox serializes it as /* foo */ var(--bar). (I'm not sure what WebKit does.)

All browser, when given a property without a variable reference, like color: /* foo */ red;, serialize the specified value as just the keyword, without the comments.

In the f2f earlier today, @emilio argued that Firefox's behavior is because it holds onto the original string for properties with variable references, and serializing using it is simplest. I think that Chrome's behavior is because it stores props with var() references as a token stream (throwing away comments), and just reserializes from the token stream.

@tabatkins tabatkins added the cssom-1 Current Work label Jul 29, 2021
@emilio
Copy link
Collaborator

emilio commented Jul 30, 2021

Another interesting test-case is something not involving comments like:

<!DOCTYPE html>
<body style="color: 1e3 var(--foo); --foo: blue;">
<script>
document.body.textContent = document.body.style.getPropertyValue("color");
</script>

Chrome serializes 1000 var(--foo) while Firefox serializes 1e3 var(--foo).

@tabatkins
Copy link
Member Author

Yeah that follows from the "parse as token stream then serialize directly from the tokens" strategy.

@andruud
Copy link
Member

andruud commented Aug 10, 2021

I think that Chrome's behavior is because it stores props with var() references as a token stream (throwing away comments)

Yeah, that's right.

Since we (should) serialize --color: /* foo */ var(--foo) as the original string, it seems consistent to serialize color: /* foo */ var(--foo) as the original string as well.

@tabatkins
Copy link
Member Author

I'm fine with specifying that, it just needed to be made clear. If you're willing to make the impl change in Chrome, then Firefox can stay as-is, and @emilio and I can figure out between the two of us whether this requires spec edits to CSSOM or Variables. ^_^

@andruud
Copy link
Member

andruud commented Aug 11, 2021

cc @kbabbitt

@cdoublev
Copy link
Collaborator

cdoublev commented Aug 25, 2021

I notice that Firefox trims leading whitespace(s) of the original string and that other consecutive whitespace(s) are kept, (consecutive whitespaces are replaced by __ in the following example, because the Markdown parser merges them) eg. color:__var(__--blue__)__ becomes color: var(__--blue__)__. I know that leading whitespace(s) are trimmed when tokenizing but I do not know a procedure in CSSOM, CSS Syntax, or CSS Variable, that defines trimming leading whitespaces (before tokenizing), though.

EDIT: I should have read #774, #881 and #6345 before.

I understand that your intent is to define that the original string (and its leading whitespaces?) should be kept.

To serialize a CSS value, the declaration value should be represented as a list of CSS component values components that, when parsed according to the property’s grammar, would represent that value, and a component value is defined as one of the preserved tokens, a function, or a simple block. Does the intended change(s) in the spec(s) mean that a CSS component value can be a string in case the specified value is a var()-containing value?

@tabatkins
Copy link
Member Author

Right, as per those issues you linked in your edit, leading (and trailing) whitespace is trimmed by the Syntax spec before anything else has a chance to see the value, and then the serialization algo puts a single space after the colon before the property value. So Firefox is correct.

Does the intended change(s) in the spec(s) mean that a CSS component value can be a string in case the specified value is a var()-containing value?

I'm sorry, I'm not quite sure what you mean by this question. Could you elaborate?

@cdoublev
Copy link
Collaborator

cdoublev commented Aug 30, 2021

Right, as per those issues you linked in your edit, leading (and trailing) whitespace is trimmed by the Syntax spec [...]. So Firefox is correct.

Then the trailing whitespace should be removed when serializing color:__var(__--blue__)__, isn't it? The output in Firefox is color: var(__--blue__)__.

EDIT: following Emilo's answer, I filled this bug.

Does the intended change(s) in the spec(s) mean that a CSS component value can be a string in case the specified value is a var()-containing value?

I'm sorry, I'm not quite sure what you mean by this question. Could you elaborate?

I understand from the procedure to serialize a CSS value that it expects component value(s) to be used. The current definition of a component value does not include a string, but only preserverd tokens, function, and block. And I understand from the above comments that a var()-containg value will have to be serialized using the original string, as is already the case of a custom property value.

Therefore I was wondering if a component value can be string. But I realize that I can just understand that these cases (custom property and var()-containing value) should ignore what the serialization procedure defines.

@emilio
Copy link
Collaborator

emilio commented Aug 30, 2021

Then the trailing whitespace should be removed when serializing color:var(--blue__), isn't it? The output in Firefox is color: var(--blue__)__.

Yeah, I think that is a Gecko bug, mind filing it?

@tabatkins
Copy link
Member Author

<string-token> is a kind of component value, sure. But the subject at hand is "the original string", aka the actual character data pre-tokenization. This is a minor layering violation; it requires reaching back into the original stylesheet text (or carrying that stylesheet text fragment forward). The specs (intentionally) don't define precisely how this is done.

@cdoublev
Copy link
Collaborator

Right, as per those issues you linked in your edit, leading (and trailing) whitespace is trimmed by the Syntax spec before anything else has a chance to see the value [...].

style.setProperty(prop, value) (or style[prop] = value) does not seem to run consume a declaration (the linked procedure), which seems to be only used for style.cssText = 'prop: value', or any other procedure that would trim trailing whitespaces. Do they need to be trimmed for both interfaces?

e1.style.color = 'var(--color)  '
e2.style = 'color: var(--color)  ;'
e1.style.color === e2.style.color // ?

Am I missing something or is it missing from parse a list of component values?

 1. Normalize input, and set input to the result.
- 2. Repeatedly consume a component value from input until an `<EOF-token>` is returned, appending the returned values (except the final `<EOF-token>`) into a list. Return the list.
+ 2. Repeatedly consume a component value from input until an `<EOF-token>` is returned, appending the returned values (except any trailing whitespaces and the final `<EOF-token>`) into a list. Return the list.

@cdoublev
Copy link
Collaborator

cdoublev commented Mar 6, 2022

Please allow me to explain the issue more clearly:

Case 1: style.cssText = 'color: var(--color) ;'

  1. parse a CSS declaration block
  2. parse a list of declarations
  3. consume a list of declarations
  4. consume a declaration

Result: 'var(--color)', ie. the leading and trailing whitespaces are trimmed.

Case 2: style.color = ' var(--color) ' or (alias) style.setProperty('color', ' var(--color) ')

  1. parse a css value
  2. parse a list of component values
  3. repeatedly consume a component value

Result: ' var(--color) ', ie. the leading and trailing whitespaces are not trimmed.


I suggested a change in my previous comment that removes the trailing whitespace in case 2, but it is missing consume a run of whitespace or something to trim any leading whitespace.

I assume that a side effect of the suggested change is that leading/trailing comments would be removed, eg. /* comment */ var(--custom)/* comment */px /* comment */ would be serialize to var(--custom)/* comment */px.

@cdoublev
Copy link
Collaborator

cdoublev commented Mar 16, 2023

In addition to my comment in #8533 (only <declaration-value> could serialize as specified by the author, without comments and consecutive whitespaces), I think a var()-containing value requires to serialize exactly as specified by the author only in a custom property value: opacity: var(--x , 1e1) could serialize to opacity: var(--x, 10).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cssom-1 Current Work
Projects
None yet
Development

No branches or pull requests

4 participants