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 Keywords should be lowercased during serialization #7726

Merged

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Dec 16, 2022

5c14835

CSS Keywords should be lowercased during serialization
https://bugs.webkit.org/show_bug.cgi?id=249438

Reviewed by Sam Weinig.

CSS Keywords should be lowercased during serialization:
- https://www.w3.org/TR/cssom-1/#serialize-a-css-component-value

* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/color-interpolation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/pointer-events-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/shape-rendering-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/text-rendering-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-variables/variable-presentation-attribute-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/inheritance-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/parsing/color-interpolation-filters-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/parsing/color-interpolation-filters-parsing-valid-expected.txt:
Rebaseline WPT tests now that more checks are passing.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText const):
Call nameStringForSerialization() instead of nameString() since we are serializing.
This makes sure that the identifier is serialized as lowercase.

* Source/WebCore/css/typedom/CSSStyleValueFactory.cpp:
(WebCore::CSSStyleValueFactory::reifyValue):
Per https://drafts.css-houdini.org/css-typed-om-1/#reify-ident, we should create
a CSSKeywordValue from the *serialization* of the identifier, which means that
the identifier would become lowercase.

* Source/WebCore/css/process-css-values.py:
(GenerationContext):
Generate nameStringForSerialization() which provides the name string of the
keyword as lowercase, for the purpose of serialization.

Canonical link: https://commits.webkit.org/258024@main

0cc1187

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim   πŸ›  mac-AS-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl ⏳ πŸ§ͺ ios-wk2   πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
⏳ πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@cdumez cdumez self-assigned this Dec 16, 2022
@cdumez cdumez added the CSS Cascading Style Sheets implementation label Dec 16, 2022
@cdumez cdumez marked this pull request as draft December 16, 2022 00:29
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 16, 2022
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Dec 16, 2022
@cdumez cdumez force-pushed the 249438_keyword_lowercase branch 2 times, most recently from 83a1e20 to 0cc1187 Compare December 16, 2022 18:33
@cdumez cdumez marked this pull request as ready for review December 16, 2022 18:39
@darinadler
Copy link
Member

Havenβ€˜t looked over the change yet but occurs to me that during serialization we just need literals not actual strings since we are concatenations. So I would suggest a nameLiteralForSerialization function instead.

@cdumez
Copy link
Contributor Author

cdumez commented Dec 16, 2022

Havenβ€˜t looked over the change yet but occurs to me that during serialization we just need literals not actual strings since we are concatenations. So I would suggest a nameLiteralForSerialization function instead.

My patch introduces nameLiteralForSerialization() too. However, I am calling nameForSerialization in the CSS logic because of the prototype (and pre-existing code):

String CSSPrimitiveValue::formatNumberForCustomCSSText()
{
   // ...
    case CSSUnitType::CSS_VALUE_ID:
         // Per the specification, we should lowercase keywords during serialization:
         // https://www.w3.org/TR/cssom-1/#serialize-a-css-component-value
         return nameStringForSerialization(m_value.valueID);
}

@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Dec 16, 2022
https://bugs.webkit.org/show_bug.cgi?id=249438

Reviewed by Sam Weinig.

CSS Keywords should be lowercased during serialization:
- https://www.w3.org/TR/cssom-1/#serialize-a-css-component-value

* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/color-interpolation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/pointer-events-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/shape-rendering-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-typed-om/the-stylepropertymap/properties/text-rendering-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-variables/variable-presentation-attribute-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/inheritance-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/parsing/color-interpolation-filters-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/filter-effects/parsing/color-interpolation-filters-parsing-valid-expected.txt:
Rebaseline WPT tests now that more checks are passing.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText const):
Call nameStringForSerialization() instead of nameString() since we are serializing.
This makes sure that the identifier is serialized as lowercase.

* Source/WebCore/css/typedom/CSSStyleValueFactory.cpp:
(WebCore::CSSStyleValueFactory::reifyValue):
Per https://drafts.css-houdini.org/css-typed-om-1/#reify-ident, we should create
a CSSKeywordValue from the *serialization* of the identifier, which means that
the identifier would become lowercase.

* Source/WebCore/css/process-css-values.py:
(GenerationContext):
Generate nameStringForSerialization() which provides the name string of the
keyword as lowercase, for the purpose of serialization.

Canonical link: https://commits.webkit.org/258024@main
@webkit-commit-queue
Copy link
Collaborator

Committed 258024@main (5c14835): https://commits.webkit.org/258024@main

Reviewed commits have been landed. Closing PR #7726 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
6 participants