Skip to content

[css-values-5] attr()'s "px"/etc keywords #11034

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

Closed
tabatkins opened this issue Oct 14, 2024 · 15 comments
Closed

[css-values-5] attr()'s "px"/etc keywords #11034

tabatkins opened this issue Oct 14, 2024 · 15 comments

Comments

@tabatkins
Copy link
Member

In #10437 (comment) we resolved to change attr()'s type argument from a bespoke list of keywords to the newly-minted "CSS grammar in CSS" syntax.

Most of the existing attr() type keywords translate over just fine (usually just wrapping them in <> characters), but the unit names don't. That is, under the current spec you can write width: attr(foo px), and given an element like <div foo=500>, it'll be parsed as a number and then be assumed to be a px value. There's no way to express this sort of behavior in CSS grammar, tho.

Note that this is basically just a convenience; one could instead write calc(1px * attr(foo <number>)) and get the same result. So the use-case is covered no matter what.

Do we want to try and allow this, tho? The issue is that bare keywords already have meaning in the <css-syntax> production (they represent those keywords, same as in any other CSS grammar), so it would be somewhat awkward to mix them in.

I propose that we just drop that functionality, and let calc() cover the issue of "interpret a number as a particular unit".

@fantasai
Copy link
Collaborator

I'm a bit ambivalent about it. I can see the theoretical purity of it, but realistically we're not going to use the full grammar syntax here, only a single <type> value--so there's no conflict. And being able to just write px is pretty handy: easy to understand, easy to use.

@tabatkins
Copy link
Member Author

It's actually fine to use the whole grammar here, now that attr() is an arbitrary-substitution function (aka var()-like). We don't need to know its type at parse time, so it's perfectly fine to write attr(foo <length> | <number>). This would be useful, for example, to pass a value to 'opacity', using <number> | <percentage> (which can't be done in the current spec).

And the spec already allows ident (spelled <ident> in the new syntax), to allow any identifier, so allowing only specific identifiers via literal idents seems just fine.

So I think we either need to drop the "specific unit" functionality, as suggested, or make the syntax part more distinguishable from other syntax. Or, I guess, add some branches to the <syntax> production to allow "number interpreted as unit" natively, like <number px> or something.

@Loirooriol
Copy link
Contributor

Loirooriol commented Oct 14, 2024

realistically we're not going to use the full grammar syntax here

Even if at some point we want to allow px as a syntax keyword instead of an unit, I guess we could use brackets to disambiguate (see https://drafts.csswg.org/css-values/#component-combinators). So attr(foo px) would parse as number and add the unit, while attr(foo [px]) would parse the attribute as the keyword or whatever is supposed to happen.

@tabatkins
Copy link
Member Author

To a little clearer, since @fantasai was confused in some personal conversation:

Right now, per spec, you can say display: attr(foo ident) and <div foo=block> will work. In the new resolved syntax, that would be display: attr(foo <ident>). But under the new syntax you could also say display: attr(foo block | flex | grid) to allow only those specific three keywords.

That's the ambiguity with px/etc that I'm worried about here. If we just go with the resolution exactly as stated in the f2f, then writing attr(foo px) means "attempt to parse the foo attribute as the ident px, and if successful, substitute that in".

I still think we can probably just drop that functionality entirely in favor of using calc(), but if we did want to preserve it, I think we'd do it with some special syntax on the number production, like <number px> to mean "parse this as a number, then give it the px unit".

@Loirooriol
Copy link
Contributor

I think this will be a popular thing to do, and calc(1px * attr(foo <number>)) is kinda cumbersome.

attr(foo <number px>) sounds good to me.

@yisibl
Copy link
Contributor

yisibl commented Oct 15, 2024

And being able to just write px is pretty handy: easy to understand, easy to use.

We absolutely need simpler syntax.

attr(foo <number px>) sounds good to me too.

@andruud
Copy link
Member

andruud commented Oct 15, 2024

<number px>

Will <px> not do here?

@tabatkins
Copy link
Member Author

Anything of that sort is possible, sure, but we should assume that anything we allow in this production might eventually show up in a CSS spec, or possibly collide with something in a CSS spec. Having an unbounded set of <px>, <cqw>, etc productions doesn't sound very safe in that regard, but a special syntax on the production itself (akin to the range restriction syntax) does.

And, personally, I think it communicates better. If I saw attr(foo <px>), I'd probably assume it was expecting a value like 5px (and would reject 5em). But attr(foo <number px>) is clearer that you're looking for a number, and involving px in some way; you might think of a value like 5px, but then you have to ask why it's not spelled <length> or even <length px> if we were being specific about the unit; the intended meaning (accept 5 like <number> would, but give it the px unit) is conceptually nearby and easy to hit by guessing.

@andruud
Copy link
Member

andruud commented Oct 23, 2024

Having an unbounded set of <px>, <cqw>? , etc productions doesn't sound very safe in that regard

Yeah, I agree with that.

And, personally, I think it [<number px>] communicates better

OK, I'll buy that too.


Ready for Agenda+?

@tabatkins
Copy link
Member Author

Yup, Agenda+ to approve the <number px> syntax to be added to the <syntax> production, meaning "parse it as a number, then give it the px unit" (or whatever).

Despite my earlier preference to just push this functionality out entirely, I think this is the right way to go, since you can accept multiple types with <syntax> and only one of them might need to be unit-ified. For example, attr(foo, <number px> | <percentage>) can't be reproduced without this feature; trying to do calc(attr(foo, <number> | <percentage>) * 1px) will fail when a percentage is actually given.

@tabatkins
Copy link
Member Author

Ah, an additional point from some internal conversation - the <number px> will become a <length> or whatever, semantically, depending on the unit.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 31, 2024
With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 31, 2024
With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 31, 2024
With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 31, 2024
With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}
@cdoublev
Copy link
Collaborator

cdoublev commented Nov 1, 2024

To re-iterate and elaborate on my comment in the <boolean[<test>]> related issue, it might be worth considering syntax component attribute names now.

<number [0,1]> is not a valid <syntax> (or @property/syntax) at the moment, but I assume it will become valid if <number px> becomes valid. People may ask for other syntax component attributes. For example, <ident range="foo BAR" case-sensitive> could match a range of identifiers case-sensitively (related: #11124).

<number px> is shorter than <number unit=px> but it might prevent such extension to the CSS value definition syntax.

@tabatkins
Copy link
Member Author

I think that's fine. CSS is always grammatically flexible, and those examples of future growth looks just fine to me. If and when we add enough specifiers for it to start getting confusing, we can worry about named arguments; until then, we can add them ad-hoc because it's shorter and easier.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 5, 2024
…a=testonly

Automatic update from web-platform-tests
Use css syntax stream parser in attr()

With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}

--

wpt-commits: e0217e09fb4256ee8a6f80b0b70d07a0db249d6f
wpt-pr: 48909
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 6, 2024
…a=testonly

Automatic update from web-platform-tests
Use css syntax stream parser in attr()

With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}

--

wpt-commits: e0217e09fb4256ee8a6f80b0b70d07a0db249d6f
wpt-pr: 48909
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Nov 6, 2024
…a=testonly

Automatic update from web-platform-tests
Use css syntax stream parser in attr()

With recent modifications in attr() spec [0], <attr-type> is now
replaced with <syntax> [1], so replaced css_attr_type with css syntax
stream parser.
Since <dimension-unit> is not part of <syntax>, temporary remove tests
with dimension unit types until the spec issue [2] will be resolved.

[0] https://drafts.csswg.org/css-values-5/#attr-notation>
[1] https://drafts.csswg.org/css-values-5/#css-syntax
[2] w3c/csswg-drafts#11034

Bug: 369858674, 40320391
Change-Id: I865c8cf5c848339a9766be0a04c5dc3f94983878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5975668
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Munira Tursunova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376296}

--

wpt-commits: e0217e09fb4256ee8a6f80b0b70d07a0db249d6f
wpt-pr: 48909
@tabatkins tabatkins removed the Agenda+ label Nov 13, 2024
@tabatkins
Copy link
Member Author

We're now resolved on not doing <number px>, and instead just letting attr() accept px as a keyword again (now that that's not ambiguous with <syntax>). So, closing.

@Loirooriol
Copy link
Contributor

Resolution: #11035 (comment)

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

7 participants
@andruud @tabatkins @fantasai @yisibl @Loirooriol @cdoublev and others