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

[css-contain-3] Contain specs disagree about canonical order of contain keywords #8600

Closed
dholbert opened this issue Mar 15, 2023 · 4 comments

Comments

@dholbert
Copy link
Member

dholbert commented Mar 15, 2023

css-contain-1 defined the grammar for contain with size at the start when using the longhand syntax, like so:

Value: none | strict | content | [ size || layout || paint ]

...whereas css-contain-3 flips the order to put size (and the new value inline-size) at the end:

New values: layout || style || paint || [ size | inline-size ]

The order matters for serialization, per https://drafts.csswg.org/cssom/#serialize-a-css-value , which says:

If certain component values can appear in any order without changing the meaning of the value (a pattern typically represented by a double bar || in the value syntax), reorder the component values to use the canonical order of component values as given in the property definition table.

If you're looking at css-contain-1, you would expect size (and hence inline-size) to serialize at the start of the list. If you're looking at css-contain-3, you'd expect them to serialize them at the end of the list.

Looking at what immplementations do & WPT tests expect:

So probably the ordering in css-contain-3 should be fixed to put [size | inline-size] at the start.

(Perhaps we should also add none | strict | content keywords to it as well, while we're at it? I'm not sure whether it's problematic for those to be omitted from "New values" or not, given that we're repeating most of the preexisting grammar there except for those ones.)

@cdoublev
Copy link
Collaborator

I'm not sure whether it's problematic for those to be omitted from "New values" or not

It is. CSS Contain 3 should redefine the entire value definition (with the right canonical order), in my opinion.

As noted in this comment, all other uses of New values in other specs assume concatenation with | as the glue, which would give none | strict | content | [ size || layout || style || paint ] | layout || style || paint || [ size | inline-size ], which is wrong, obviously.

@dholbert
Copy link
Member Author

Yeah, css contain 3 should probably say "Value: [...]" and include the whole grammar, instead of specifying "New values" with a subset of the preexisting grammar.

Also, it looks like I forgot to link to the relevant spec sections. Here they are (including level 2 as well for completeness):
Level 1: https://drafts.csswg.org/css-contain-1/#contain-property
Value: none | strict | content | [ size || layout || paint ]

Level 2: https://drafts.csswg.org/css-contain-2/#contain-property
Value: none | strict | content | [ size || layout || style || paint ]

Level 3: https://drafts.csswg.org/css-contain-3/#contain-property
New values: layout || style || paint || [ size | inline-size ]

That last one is the one that needs fixing.

@mirisuzanne
Copy link
Contributor

Pushed a fix. Let me know if there are still issues there.

@dholbert
Copy link
Member Author

dholbert commented May 4, 2023

Thanks! So with that fix, the Level 3 definition now reads:

Value: none | strict | content | [ [size | inline-size] || layout || style || paint ]

i.e. the same ordering as in Level 2, but with size expanded to [size | inline-size].

That make sense to me, and it means the spec matches the tests that I linked in the initial comment here. (Firefox still mistakenly serializes inline-size at the end, but we'll fix that in https://bugzilla.mozilla.org/show_bug.cgi?id=1744309 or a dependency.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants