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.

Sign up for GitHub

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

Jump to bottom

[css-color-4] Clarify serialization of rgb()/rgba() functions that have none components #6959

Closed
weinig opened this issue Jan 17, 2022 · 4 comments
Closed

[css-color-4] Clarify serialization of rgb()/rgba() functions that have none components #6959

weinig opened this issue Jan 17, 2022 · 4 comments
Labels
Closed Accepted as Obvious Bugfix css-color-4 Current Work

Comments

@weinig
Copy link
Contributor

weinig commented Jan 17, 2022

CSS Color 4 indicates that none components are only allowed in the space separated syntax for rgb() / rgba() functions, but since rgb() / rgba() functions are serialized in the comma separated style, this leads to a weird case where the serialized form is not reparseable if any of the components are none. For example:

rgb(255 0 none)

should currently serialize as:

rgb(255, 0, none)

which is an invalid value.

Seems like either none should be allowed in the comma separated legacy form or serialization should be updated to specify a different serialization if none is used.
@svgeesus
Copy link
Contributor

Good catch!

Because the comma-separated form is preferred mainly due to greatest backward compatibility, and because none is new, it would seem to make sense to serialize in the non-legacy form in that case.

On the other hand, allowing none everywhere is a bit of a heavy hammer and I would also be fine with just saying that none serializes as 0 here because I can't really see a use case for preserving none in RGB values. And in HSL or HWB it will serialize as rgb() in any case, with conversion to 0 for the hue if that was none.

@svgeesus svgeesus added the css-color-4 Current Work label Jan 17, 2022
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Jan 18, 2022
@weinig
[CSS Color 4] Add support for "Missing"/"none" color components
8131fab 
https://bugs.webkit.org/show_bug.cgi?id=233526
<rdar://problem/86026087>

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Update color pasing tests to include 'none' components.

* web-platform-tests/css/css-color/parsing/color-computed-expected.txt:
* web-platform-tests/css/css-color/parsing/color-computed.html:
* web-platform-tests/css/css-color/parsing/color-invalid-expected.txt:
* web-platform-tests/css/css-color/parsing/color-invalid.html:
* web-platform-tests/css/css-color/parsing/color-valid-expected.txt:
* web-platform-tests/css/css-color/parsing/color-valid.html:
* web-platform-tests/css/css-color/parsing/relative-color-computed-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-computed.html:
* web-platform-tests/css/css-color/parsing/relative-color-invalid-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-invalid.html:
* web-platform-tests/css/css-color/parsing/relative-color-valid-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-valid.html:

Source/WebCore:

Adds support for parsing an identifier, 'none', as a CSS color component for all specified
grammars, including rgb() (space separated), hsl() (space separated), hwb(), color(), lab(),
lch(), oklab(), oklch() as well as all the respective relative forms.

For color types that were already being stored using float components, the 'none' value
is stored as NaN. For the types that were not (rgb(), hsl() and hwb()) we now store the
value using float components if and only if there are any 'none' components (bounded SRGBA<float>
for rgb(), HSLA<float> for hsl() and HWBA<float> for hwb()). This is necessary for both
serialization, which has been updated to handle the new values, as well as interpolation,
which is not included in this change.

* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::NoneRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::AngleOrNumberRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::NumberOrPercentRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::IdentityTransformer::transform):
(WebCore::CSSPropertyParserHelpers::RawIdentityTransformer::transform):
(WebCore::CSSPropertyParserHelpers::RawVariantTransformerBase::transform):
(WebCore::CSSPropertyParserHelpers::consumeNumberRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumePercentRaw):
(WebCore::CSSPropertyParserHelpers::consumePercentRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeLengthRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleRaw):
(WebCore::CSSPropertyParserHelpers::consumeLengthOrPercentRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleOrNumberOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleOrNumberOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumePercentOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumePercentOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeOptionalAlpha):
(WebCore::CSSPropertyParserHelpers::consumeOptionalAlphaAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentToSRGBAByte):
(WebCore::CSSPropertyParserHelpers::consumeRGBOrHSLOptionalAlpha):
(WebCore::CSSPropertyParserHelpers::parseRelativeRGBParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeRGBParameters):
(WebCore::CSSPropertyParserHelpers::colorByNormalizingHSLComponents):
(WebCore::CSSPropertyParserHelpers::parseRelativeHSLParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeHSLParameters):
(WebCore::CSSPropertyParserHelpers::parseHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseLabParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeLabParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeLabParameters):
(WebCore::CSSPropertyParserHelpers::parseLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseColorFunctionForRGBTypes):
(WebCore::CSSPropertyParserHelpers::parseRelativeColorFunctionForRGBTypes):
(WebCore::CSSPropertyParserHelpers::parseColorFunctionForXYZTypes):
(WebCore::CSSPropertyParserHelpers::parseRelativeColorFunctionForXYZTypes):
(WebCore::CSSPropertyParserHelpers::AngleOrNumberRawToDegressTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::LengthOrPercentRawTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::consumeNumberAllowingSymbolTableIdent): Deleted.
(WebCore::CSSPropertyParserHelpers::consumePercentAllowingSymbolTableIdent): Deleted.
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentNumber): Deleted.
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentPercentage): Deleted.
(WebCore::CSSPropertyParserHelpers::RGBNormalizingTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::consumeRelativeRGBComponent): Deleted.
(WebCore::CSSPropertyParserHelpers::clampRGBComponent): Deleted.
* css/parser/CSSPropertyParserHelpers.h:
* platform/graphics/ColorNormalization.h:
(WebCore::normalizeClampedWhitenessBlacknessDisallowingNone):
(WebCore::normalizeClampedWhitenessBlacknessAllowingNone):
Pipe through support for parsing and normalizing 'none' components.

* platform/graphics/ColorSerialization.cpp:
(WebCore::legacyRGBComponent):
(WebCore::numericComponent):
(WebCore::percentageComponent):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::length const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::buffer const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::length const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::buffer const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::length const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::buffer const):
Add helper StringTypeAdapter to efficiently encode "none" or the number
when used with makeString().

(WebCore::serializationUsingColorFunction):
(WebCore::serializationForCSS):
(WebCore::serializationForHTML):
(WebCore::serializationForRenderTreeAsText):
Add support for serializing bounded SRGBA<float> in the rgba() form rather
than the color function form for the case when it is used to store NaNs. This
serialization is a bit different than the normal 8-bit serialization as it uses
the whitespace syntax rather than comma syntax to allow round tripping of 'none'
components. This is currently unders discussion with the editors:
    - w3c/csswg-drafts#6959

* platform/graphics/ColorTypes.h:
(WebCore::assertInRange):
Update bounds assertion to allow NaN for all components regardless of bounds.



Canonical link: https://commits.webkit.org/246143@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288143 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@tabatkins
Copy link
Member

Yeah, just serializing as 0 in rgb() seems reasonable to me, too.

annulen pushed a commit to qtwebkit/qtwebkit that referenced this issue Jan 19, 2022
[CSS Color 4] Add support for "Missing"/"none" color components
02a7051 
https://bugs.webkit.org/show_bug.cgi?id=233526
<rdar://problem/86026087>

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Update color pasing tests to include 'none' components.

* web-platform-tests/css/css-color/parsing/color-computed-expected.txt:
* web-platform-tests/css/css-color/parsing/color-computed.html:
* web-platform-tests/css/css-color/parsing/color-invalid-expected.txt:
* web-platform-tests/css/css-color/parsing/color-invalid.html:
* web-platform-tests/css/css-color/parsing/color-valid-expected.txt:
* web-platform-tests/css/css-color/parsing/color-valid.html:
* web-platform-tests/css/css-color/parsing/relative-color-computed-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-computed.html:
* web-platform-tests/css/css-color/parsing/relative-color-invalid-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-invalid.html:
* web-platform-tests/css/css-color/parsing/relative-color-valid-expected.txt:
* web-platform-tests/css/css-color/parsing/relative-color-valid.html:

Source/WebCore:

Adds support for parsing an identifier, 'none', as a CSS color component for all specified
grammars, including rgb() (space separated), hsl() (space separated), hwb(), color(), lab(),
lch(), oklab(), oklch() as well as all the respective relative forms.

For color types that were already being stored using float components, the 'none' value
is stored as NaN. For the types that were not (rgb(), hsl() and hwb()) we now store the
value using float components if and only if there are any 'none' components (bounded SRGBA<float>
for rgb(), HSLA<float> for hsl() and HWBA<float> for hwb()). This is necessary for both
serialization, which has been updated to handle the new values, as well as interpolation,
which is not included in this change.

* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::NoneRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::AngleOrNumberRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::NumberOrPercentRawKnownTokenTypeIdentConsumer::consume):
(WebCore::CSSPropertyParserHelpers::IdentityTransformer::transform):
(WebCore::CSSPropertyParserHelpers::RawIdentityTransformer::transform):
(WebCore::CSSPropertyParserHelpers::RawVariantTransformerBase::transform):
(WebCore::CSSPropertyParserHelpers::consumeNumberRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumePercentRaw):
(WebCore::CSSPropertyParserHelpers::consumePercentRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeLengthRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleRaw):
(WebCore::CSSPropertyParserHelpers::consumeLengthOrPercentRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleOrNumberOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeAngleOrNumberOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumePercentOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumePercentOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentOrNoneRaw):
(WebCore::CSSPropertyParserHelpers::consumeNumberOrPercentOrNoneRawAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::consumeOptionalAlpha):
(WebCore::CSSPropertyParserHelpers::consumeOptionalAlphaAllowingSymbolTableIdent):
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentToSRGBAByte):
(WebCore::CSSPropertyParserHelpers::consumeRGBOrHSLOptionalAlpha):
(WebCore::CSSPropertyParserHelpers::parseRelativeRGBParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeRGBParameters):
(WebCore::CSSPropertyParserHelpers::colorByNormalizingHSLComponents):
(WebCore::CSSPropertyParserHelpers::parseRelativeHSLParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeHSLParameters):
(WebCore::CSSPropertyParserHelpers::parseHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeHWBParameters):
(WebCore::CSSPropertyParserHelpers::parseLabParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeLabParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeLabParameters):
(WebCore::CSSPropertyParserHelpers::parseLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseRelativeLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseNonRelativeLCHParameters):
(WebCore::CSSPropertyParserHelpers::parseColorFunctionForRGBTypes):
(WebCore::CSSPropertyParserHelpers::parseRelativeColorFunctionForRGBTypes):
(WebCore::CSSPropertyParserHelpers::parseColorFunctionForXYZTypes):
(WebCore::CSSPropertyParserHelpers::parseRelativeColorFunctionForXYZTypes):
(WebCore::CSSPropertyParserHelpers::AngleOrNumberRawToDegressTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::LengthOrPercentRawTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::consumeNumberAllowingSymbolTableIdent): Deleted.
(WebCore::CSSPropertyParserHelpers::consumePercentAllowingSymbolTableIdent): Deleted.
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentNumber): Deleted.
(WebCore::CSSPropertyParserHelpers::normalizeRGBComponentPercentage): Deleted.
(WebCore::CSSPropertyParserHelpers::RGBNormalizingTransformer::transform): Deleted.
(WebCore::CSSPropertyParserHelpers::consumeRelativeRGBComponent): Deleted.
(WebCore::CSSPropertyParserHelpers::clampRGBComponent): Deleted.
* css/parser/CSSPropertyParserHelpers.h:
* platform/graphics/ColorNormalization.h:
(WebCore::normalizeClampedWhitenessBlacknessDisallowingNone):
(WebCore::normalizeClampedWhitenessBlacknessAllowingNone):
Pipe through support for parsing and normalizing 'none' components.

* platform/graphics/ColorSerialization.cpp:
(WebCore::legacyRGBComponent):
(WebCore::numericComponent):
(WebCore::percentageComponent):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::length const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::buffer const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::length const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::NumericComponent>::buffer const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::StringTypeAdapter):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::length const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::is8Bit const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::writeTo const):
(WTF::StringTypeAdapter<WebCore::PercentageComponent>::buffer const):
Add helper StringTypeAdapter to efficiently encode "none" or the number
when used with makeString().

(WebCore::serializationUsingColorFunction):
(WebCore::serializationForCSS):
(WebCore::serializationForHTML):
(WebCore::serializationForRenderTreeAsText):
Add support for serializing bounded SRGBA<float> in the rgba() form rather
than the color function form for the case when it is used to store NaNs. This
serialization is a bit different than the normal 8-bit serialization as it uses
the whitespace syntax rather than comma syntax to allow round tripping of 'none'
components. This is currently unders discussion with the editors:
    - w3c/csswg-drafts#6959

* platform/graphics/ColorTypes.h:
(WebCore::assertInRange):
Update bounds assertion to allow NaN for all components regardless of bounds.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288143 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@svgeesus
Copy link
Contributor

Does this apply to resolving values, or just to serialization? (I assume the latter but just wanted to check first)

@weinig
Copy link
Contributor Author

weinig commented Jan 20, 2022

I hope just serialization. Doesn't seem like there is a compelling reason here to change value resolution.

svgeesus added a commit that referenced this issue Jan 21, 2022
@svgeesus
[css-color-4] Clarify serialization of "none", see #6959
9401006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Closed Accepted as Obvious Bugfix css-color-4 Current Work
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tabatkins @weinig @svgeesus