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-values-4][cssom] Serialization of <position> #8996

Closed
nt1m opened this issue Jun 21, 2023 · 7 comments
Closed

[css-values-4][cssom] Serialization of <position> #8996

nt1m opened this issue Jun 21, 2023 · 7 comments
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-values-4 Current Work cssom-1

Comments

@nt1m
Copy link
Member

nt1m commented Jun 21, 2023

https://drafts.csswg.org/css-values-4/#position-serialization says:

If only one component is specified:
The implied center keyword is added, and a 2-component value is serialized.

but https://drafts.csswg.org/cssom/#serializing-css-values says:

If component values can be omitted or replaced with a shorter representation without changing the meaning of the value, omit/replace them.

Which one is right? Personally I have a slight preference of omitting redundant values, unless there's a specific compat reason not to (which is mandated by the CSSOM spec), just for the sake of consistency with serialization of other types.

@nt1m
Copy link
Member Author

nt1m commented Jun 21, 2023

@Loirooriol
Copy link
Contributor

https://wiki.csswg.org/ideas/mistakes

background-size with one value should duplicate its value, not default the second one to auto. Ditto translate().

I guess that a reasonable expectation would be that background-position: 0px behaves as background-position: 0px 0px, so serializing as background-position: 0px center can avoid misunderstandings.

See whatwg/compat#28 and #7802.

@nt1m
Copy link
Member Author

nt1m commented Jun 21, 2023

The citations you linked is for background-size (which is 2 sizes), not background-position, not sure there's as much confusion for background-position

@Loirooriol
Copy link
Contributor

Oh I mixed them up in my mind, sorry. But it's still the case that background-position: 0px means 0px center instead of repeating. So there may still be confusion? I don't know.

@nt1m
Copy link
Member Author

nt1m commented Jun 21, 2023

Yes, background-position: 0px means 0px center, not 0px 0px

@cdoublev
Copy link
Collaborator

fantasai: a more important reason is the transform-origin syntax, which becomes ambiguous if you only have one value
fantasai: You wouldn’t be able to consider position as a single value without considering whether or not you have a Z component. That creates a serialization of position cannot be self-contained
fantasai: The proposal: always serializes at least two values

#2274 (comment)

@tabatkins
Copy link
Member

Ah, I forgot about that when I approved the test change that reduced some position serializations. We should capture that in the spec, since it's important.

tabatkins added a commit that referenced this issue Jun 22, 2023
…ation violates the shortest-serialization-principle. #8996
webkit-commit-queue pushed a commit to nt1m/WebKit that referenced this issue Sep 22, 2023
https://bugs.webkit.org/show_bug.cgi?id=258585
rdar://111750372

Reviewed by Oriol Brufau and Simon Fraser.

This was clarified in w3c/csswg-drafts#8996

It's ambiguous in some cases, especially when followed by length.

* LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style-expected.txt:
* LayoutTests/fast/backgrounds/background-shorthand-with-backgroundSize-style.html:
* LayoutTests/fast/css/background-position-serialize-expected.txt:
* LayoutTests/fast/css/background-position-serialize.html:
* LayoutTests/fast/masking/parsing-webkit-mask-expected.txt:
* LayoutTests/fast/masking/parsing-webkit-mask.html:
* LayoutTests/fast/masking/parsing-mask-expected.txt:
* LayoutTests/fast/masking/parsing-mask.html:
* LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-masking/parsing/mask-position-valid.html:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/parsing/offset-anchor-parsing-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/parsing/offset-parsing-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/parsing/offset-position-parsing-valid-expected.txt:
* Source/WebCore/css/ShorthandSerializer.cpp:
(WebCore::ShorthandSerializer::serializeLayered const):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumePosition):

Canonical link: https://commits.webkit.org/268291@main
@nt1m nt1m closed this as completed Sep 26, 2023
@nt1m nt1m added the Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. label Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-values-4 Current Work cssom-1
Projects
None yet
Development

No branches or pull requests

4 participants