Bug 266863
| Summary: | Consider lowercasing CSS keywords at source | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Anne van Kesteren <annevk> |
| Component: | CSS | Assignee: | Sam Weinig <sam> |
| Status: | NEW | ||
| Severity: | Normal | CC: | karlcow, ntim, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Anne van Kesteren
It would probably be best if CSS keywords were lowercased in CSSValueKeywords.in rather than requiring them to be lowercased upon serialization (e.g., that's how we did currentcolor).
This would allow for removing the lowercasing code introduced in bug 249438.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Tim Nguyen (:ntim)
That's probably fine, but there should be an assert somewhere to ensure everything is lowercase.
Radar WebKit Bug Importer
<rdar://problem/120323932>
Sam Weinig
Pull request: https://github.com/WebKit/WebKit/pull/39295
Sam Weinig
Doing this makes the following tests fail:
imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-inline-value.html
imported/w3c/web-platform-tests/css/css-transforms/parsing/transform-valid.html
imported/w3c/web-platform-tests/css/css-transforms/transform-important.html
transforms/2d/transform-value-types.html
(see https://ews-build.s3-us-west-2.amazonaws.com/macOS-Ventura-Release-WK1-Tests-EWS/20cb8caf-31948/results.html).
We should figure out if we think there is a compatibility concern here before proceeding.
Karl Dubost
Spec reference for
translateX(), translateY(), scaleX(), scaleY(), skewX(), skewY()
https://drafts.csswg.org/css-transforms/#two-d-transform-functions
Karl Dubost
https://codepen.io/webcompat/pen/xbKaLav
uppercase and lowercase are working the same without issues.
So the compat risk would be really on the side of other tools and JavaScript libraries expecting a match on the casing of these names.
Karl Dubost
if I manually change the test to be lowercase the error is about the canonical serialization.
assert_equals("translateX(-4px)", "translatex(-4px)", "serialization should be canonical")
Sam Weinig
Yes, the only compat issue is of course sites that depend on the specific serialization.
Sam Weinig
The parser is agnostic to case for identifiers. This bug is only about serialization.
Anne van Kesteren
I guess we should not do this unilaterally then. I was thinking in particular about the keywords bug 249438 was fixing, such as 'srgb' and 'optimizespeed'. Or do we also have cases where those need to be serialized as 'sRGB' and 'optimizeSpeed' and so there's no real canonical representation?
Sam Weinig
I think the only cases we know of where this is an issue is the transform ones.
I think it's quite unlikely to be a real compat issue, and updating the tests makes sense (the cases of sites relying on specific capitalization of serialization seems quite unlikely).
So, my proposal would be to just update the tests to match the spec, which is quite clear in saying identifiers should be serialized as all lowercase.
I am happy to make this fix if no one objects.
Anne van Kesteren
I see, I didn't realize. That sounds fine to me. I'll ping Elika just in case.
Anne van Kesteren
Elika pointed out that this isn't really defined. I filed https://github.com/w3c/csswg-drafts/issues/11556
Sam Weinig
Oh, interesting! Thanks for filing!