10px10px incorrectly parsed as two tokens #12

Closed
opened 2024-01-17 16:43:12 +01:00 by pfaffe · 13 comments
pfaffe commented 2024-01-17 16:43:12 +01:00 (Migrated from github.com)

For example in margin: 10px10px, the value is incorrectly parsed as two separate tokens, where the result should be a single token (and the declaration should be invalid).

https://www.w3.org/TR/css-syntax-3/#consume-declaration
https://www.w3.org/TR/css-syntax-3/#consume-component-value

For example in `margin: 10px10px`, the value is incorrectly parsed as two separate tokens, where the result should be a single token (and the declaration should be invalid). https://www.w3.org/TR/css-syntax-3/#consume-declaration https://www.w3.org/TR/css-syntax-3/#consume-component-value
ergunsh commented 2024-01-29 15:49:50 +01:00 (Migrated from github.com)

Hey @marijnh, do you have any suggestions for this? We're slowly migrating to use lezer parser for rendering property declarations in the Styles pane in DevTools (crrev.com/c/5046552) and this is causing a bug for us.

Hey @marijnh, do you have any suggestions for this? We're slowly migrating to use lezer parser for rendering property declarations in the Styles pane in DevTools (crrev.com/c/5046552) and this is causing a bug for us.
marijnh commented 2024-01-29 17:38:52 +01:00 (Migrated from github.com)

Could you explain the problem a bit better? It seems in the spec, units are simply identifiers. So I guess technically px10px could be tokenized as a unit here. But I don't see how that helps.

Could you explain the problem a bit better? It seems in the [spec](https://www.w3.org/TR/css-values-3/#dimensions), units are simply identifiers. So I guess technically `px10px` could be tokenized as a unit here. But I don't see how that helps.
pfaffe commented 2024-02-19 10:08:58 +01:00 (Migrated from github.com)

I concur, tokenizing px10px as a unit looks like a good solution. I now realize my original phrasing was a little ambiguous, I shouldn't have called it tokens. 10px10px is currently parsed as two NumberLiterals (each of which has a Unit child). Parsing as a single NumberLiteral with a Unit SGTM.

I concur, tokenizing px10px as a unit looks like a good solution. I now realize my original phrasing was a little ambiguous, I shouldn't have called it tokens. 10px10px is currently parsed as two NumberLiterals (each of which has a Unit child). Parsing as a single NumberLiteral with a Unit SGTM.
marijnh commented 2024-02-19 10:14:33 +01:00 (Migrated from github.com)

I'd still like to understand the motivation behind this issue. What problem are you having with the current behavior?

I'd still like to understand the motivation behind this issue. What problem are you having with the current behavior?
pfaffe commented 2024-02-19 10:18:02 +01:00 (Migrated from github.com)

margin: 10px10px is not a valid declaration, and per spec 10px10px isn't two number literals with unit. The current parsing behavior makes it impossible to determine the input isn't valid.

`margin: 10px10px` is not a valid declaration, and per spec `10px10px` isn't two number literals with unit. The current parsing behavior makes it impossible to determine the input isn't valid.
marijnh commented 2024-02-19 10:24:12 +01:00 (Migrated from github.com)

Are you using parser output for some kind of linting? Or do you mean you want the highlighting to show there's something weird going on?

Are you using parser output for some kind of linting? Or do you mean you want the _highlighting_ to show there's something weird going on?
pfaffe commented 2024-02-19 10:26:37 +01:00 (Migrated from github.com)

We're using the parser output to add decorations and UI controls to property values in DevTools (crrev.com/c/5046552).

We're using the parser output to add decorations and UI controls to property values in DevTools (crrev.com/c/5046552).
marijnh commented 2024-02-19 11:15:36 +01:00 (Migrated from github.com)

Okay. But how, specifically, is the current parsing an issue with that?

Okay. But how, specifically, is the current parsing an issue with that?
pfaffe commented 2024-02-19 11:34:27 +01:00 (Migrated from github.com)

We cannot tell from the AST that the input was an invalid property declaration. We cannot validate the property value because the token sequence doesn't accurately represent the input.

We cannot tell from the AST that the input was an invalid property declaration. We cannot validate the property value because the token sequence doesn't accurately represent the input.
marijnh commented 2024-02-19 12:32:53 +01:00 (Migrated from github.com)

But does parsing this as a number with a weird unit help at all for that?

But does parsing this as a number with a weird unit help at all for that?
pfaffe commented 2024-02-19 12:37:33 +01:00 (Migrated from github.com)

Yes, it allows us to determine that the property value's AST does not represent a valid , for example. Note that we can't outright parse this as a syntax error, since something like --var: 10px10px is a perfectly valid declaration.

Yes, it allows us to determine that the property value's AST does not represent a valid <length>, for example. Note that we can't outright parse this as a syntax error, since something like `--var: 10px10px` is a perfectly valid declaration.
marijnh commented 2024-02-19 12:58:33 +01:00 (Migrated from github.com)

That makes sense. Attached patch implements this.

That makes sense. Attached patch implements this.
pfaffe commented 2024-02-19 13:41:11 +01:00 (Migrated from github.com)

Thanks a lot!

Thanks a lot!
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lezer/css#12
No description provided.