Skip to content

Commit 3f49c5d

Browse files
NickGerlemanfacebook-github-bot
NickGerleman
authored and
facebook-github-bot
committed
More spec compliant rgb function parsing (#48839)
Summary: In the last diff I mixed and matched `<legacy-rgb-syntax>` and `<modern-rgb-syntax>` a bit to keep compatiblity with `normalze-color`. Spec noncompliant values have only been allowed since #34600 with the main issue being that legacy syntax rgb functions are allowed to use the `/` based alpha syntax, and commas can be mixed with whitespace. This seems like an exceedingly rare real-world scenario (there are currently zero usages of slash syntax in RKJSModules validated by `rgb\([^\)]*/`), so I'm going to instead just follow the spec for more sanity. Another bit that I missed was that modern RGB functions allow individual components to be `<percentage>` or `<number>` compared to legacy functions which only allow the full function to accept one or the other (`normalize-color` doesn't support `<percentage>` at all), so I fixed that as well. I started sharing a little bit more of the logic here, to make things more readable when adding more functions. Changelog: [Internal] Reviewed By: javache Differential Revision: D68468275
1 parent 2405c54 commit 3f49c5d

File tree

4 files changed

+169
-72
lines changed

4 files changed

+169
-72
lines changed

‎packages/react-native/ReactCommon/react/renderer/css/CSSColorFunction.h‎

Lines changed: 134 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <react/renderer/css/CSSPercentage.h>
1818
#include <react/renderer/css/CSSSyntaxParser.h>
1919
#include <react/renderer/css/CSSValueParser.h>
20+
#include <react/utils/PackTraits.h>
2021
#include <react/utils/fnv1a.h>
2122

2223
namespace facebook::react {
@@ -33,72 +34,157 @@ constexpr uint8_t clamp255Component(float f) {
3334
return static_cast<uint8_t>(std::clamp(ceiled, 0, 255));
3435
}
3536

37+
constexpr std::optional<float> normalizeNumberComponent(
38+
const std::variant<std::monostate, CSSNumber>& component) {
39+
if (std::holds_alternative<CSSNumber>(component)) {
40+
return std::get<CSSNumber>(component).value;
41+
}
42+
43+
return {};
44+
}
45+
46+
template <typename... ComponentT>
47+
requires(
48+
(std::is_same_v<CSSNumber, ComponentT> ||
49+
std::is_same_v<CSSPercentage, ComponentT>) &&
50+
...)
51+
constexpr std::optional<float> normalizeComponent(
52+
const std::variant<std::monostate, ComponentT...>& component,
53+
float baseValue) {
54+
if constexpr (traits::containsType<CSSPercentage, ComponentT...>()) {
55+
if (std::holds_alternative<CSSPercentage>(component)) {
56+
return std::get<CSSPercentage>(component).value / 100.0f * baseValue;
57+
}
58+
}
59+
60+
if constexpr (traits::containsType<CSSNumber, ComponentT...>()) {
61+
if (std::holds_alternative<CSSNumber>(component)) {
62+
return std::get<CSSNumber>(component).value;
63+
}
64+
}
65+
66+
return {};
67+
}
68+
69+
template <CSSDataType... FirstComponentAllowedTypesT>
70+
constexpr bool isLegacyColorFunction(CSSSyntaxParser& parser) {
71+
auto lookahead = parser;
72+
auto next = parseNextCSSValue<FirstComponentAllowedTypesT...>(lookahead);
73+
if (std::holds_alternative<std::monostate>(next)) {
74+
return false;
75+
}
76+
77+
return lookahead.consumeComponentValue<bool>(
78+
CSSDelimiter::OptionalWhitespace, [](CSSPreservedToken token) {
79+
return token.type() == CSSTokenType::Comma;
80+
});
81+
}
82+
3683
/**
37-
* Parses an rgb() or rgba() function and returns a CSSColor if it is valid.
38-
* Some invalid syntax (like mixing commas and whitespace) are allowed for
39-
* backwards compatibility with normalize-color.
40-
* https://www.w3.org/TR/css-color-4/#funcdef-rgb
84+
* Parses a legacy syntax rgb() or rgba() function and returns a CSSColor if it
85+
* is valid.
86+
* https://www.w3.org/TR/css-color-4/#typedef-legacy-rgb-syntax
4187
*/
4288
template <typename CSSColor>
43-
constexpr std::optional<CSSColor> parseRgbFunction(CSSSyntaxParser& parser) {
44-
auto firstValue = parseNextCSSValue<CSSNumber, CSSPercentage>(parser);
45-
if (std::holds_alternative<std::monostate>(firstValue)) {
89+
constexpr std::optional<CSSColor> parseLegacyRgbFunction(
90+
CSSSyntaxParser& parser) {
91+
auto rawRed = parseNextCSSValue<CSSNumber, CSSPercentage>(parser);
92+
bool usesNumber = std::holds_alternative<CSSNumber>(rawRed);
93+
94+
auto red = normalizeComponent(rawRed, 255.0f);
95+
if (!red.has_value()) {
4696
return {};
4797
}
4898

49-
float redNumber = 0;
50-
float greenNumber = 0;
51-
float blueNumber = 0;
99+
auto green = usesNumber
100+
? normalizeNumberComponent(
101+
parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::Comma))
102+
: normalizeComponent(
103+
parseNextCSSValue<CSSPercentage>(parser, CSSDelimiter::Comma),
104+
255.0f);
105+
if (!green.has_value()) {
106+
return {};
107+
}
52108

53-
if (std::holds_alternative<CSSNumber>(firstValue)) {
54-
redNumber = std::get<CSSNumber>(firstValue).value;
109+
auto blue = usesNumber
110+
? normalizeNumberComponent(
111+
parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::Comma))
112+
: normalizeComponent(
113+
parseNextCSSValue<CSSPercentage>(parser, CSSDelimiter::Comma),
114+
255.0f);
115+
if (!blue.has_value()) {
116+
return {};
117+
}
55118

56-
auto green =
57-
parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::CommaOrWhitespace);
58-
if (!std::holds_alternative<CSSNumber>(green)) {
59-
return {};
60-
}
61-
greenNumber = std::get<CSSNumber>(green).value;
119+
auto alpha = normalizeComponent(
120+
parseNextCSSValue<CSSNumber, CSSPercentage>(parser, CSSDelimiter::Comma),
121+
1.0f);
62122

63-
auto blue =
64-
parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::CommaOrWhitespace);
65-
if (!std::holds_alternative<CSSNumber>(blue)) {
66-
return {};
67-
}
68-
blueNumber = std::get<CSSNumber>(blue).value;
69-
} else {
70-
redNumber = std::get<CSSPercentage>(firstValue).value * 2.55f;
123+
return CSSColor{
124+
.r = clamp255Component(*red),
125+
.g = clamp255Component(*green),
126+
.b = clamp255Component(*blue),
127+
.a = alpha.has_value() ? clamp255Component(*alpha * 255.0f)
128+
: static_cast<uint8_t>(255u),
129+
};
130+
}
71131

72-
auto green = parseNextCSSValue<CSSPercentage>(
73-
parser, CSSDelimiter::CommaOrWhitespace);
74-
if (!std::holds_alternative<CSSPercentage>(green)) {
75-
return {};
76-
}
77-
greenNumber = std::get<CSSPercentage>(green).value * 2.55f;
132+
/**
133+
* Parses a modern syntax rgb() or rgba() function and returns a CSSColor if it
134+
* is valid.
135+
* https://www.w3.org/TR/css-color-4/#typedef-modern-rgb-syntax
136+
*/
137+
template <typename CSSColor>
138+
constexpr std::optional<CSSColor> parseModernRgbFunction(
139+
CSSSyntaxParser& parser) {
140+
auto red = normalizeComponent(
141+
parseNextCSSValue<CSSNumber, CSSPercentage>(parser), 255.0f);
142+
if (!red.has_value()) {
143+
return {};
144+
}
78145

79-
auto blue = parseNextCSSValue<CSSPercentage>(
80-
parser, CSSDelimiter::CommaOrWhitespace);
81-
if (!std::holds_alternative<CSSPercentage>(blue)) {
82-
return {};
83-
}
84-
blueNumber = std::get<CSSPercentage>(blue).value * 2.55f;
146+
auto green = normalizeComponent(
147+
parseNextCSSValue<CSSNumber, CSSPercentage>(
148+
parser, CSSDelimiter::Whitespace),
149+
255.0f);
150+
if (!green.has_value()) {
151+
return {};
85152
}
86153

87-
auto alphaValue = parseNextCSSValue<CSSNumber, CSSPercentage>(
88-
parser, CSSDelimiter::CommaOrWhitespaceOrSolidus);
154+
auto blue = normalizeComponent(
155+
parseNextCSSValue<CSSNumber, CSSPercentage>(
156+
parser, CSSDelimiter::Whitespace),
157+
255.0f);
158+
if (!blue.has_value()) {
159+
return {};
160+
}
89161

90-
float alphaNumber = std::holds_alternative<std::monostate>(alphaValue) ? 1.0f
91-
: std::holds_alternative<CSSNumber>(alphaValue)
92-
? std::get<CSSNumber>(alphaValue).value
93-
: std::get<CSSPercentage>(alphaValue).value / 100.0f;
162+
auto alpha = normalizeComponent(
163+
parseNextCSSValue<CSSNumber, CSSPercentage>(
164+
parser, CSSDelimiter::SolidusOrWhitespace),
165+
1.0f);
94166

95167
return CSSColor{
96-
.r = clamp255Component(redNumber),
97-
.g = clamp255Component(greenNumber),
98-
.b = clamp255Component(blueNumber),
99-
.a = clamp255Component(alphaNumber * 255.0f),
168+
.r = clamp255Component(*red),
169+
.g = clamp255Component(*green),
170+
.b = clamp255Component(*blue),
171+
.a = alpha.has_value() ? clamp255Component(*alpha * 255.0f)
172+
: static_cast<uint8_t>(255u),
100173
};
101174
}
175+
176+
/**
177+
* Parses an rgb() or rgba() function and returns a CSSColor if it is valid.
178+
* https://www.w3.org/TR/css-color-4/#funcdef-rgb
179+
*/
180+
template <typename CSSColor>
181+
constexpr std::optional<CSSColor> parseRgbFunction(CSSSyntaxParser& parser) {
182+
if (isLegacyColorFunction<CSSNumber, CSSPercentage>(parser)) {
183+
return parseLegacyRgbFunction<CSSColor>(parser);
184+
} else {
185+
return parseModernRgbFunction<CSSColor>(parser);
186+
}
187+
}
102188
} // namespace detail
103189

104190
/**

‎packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ enum class CSSDelimiter {
9393
Whitespace,
9494
OptionalWhitespace,
9595
Solidus,
96+
SolidusOrWhitespace,
9697
Comma,
9798
CommaOrWhitespace,
98-
CommaOrWhitespaceOrSolidus,
9999
None,
100100
};
101101

@@ -313,10 +313,9 @@ struct CSSComponentValueVisitorDispatcher {
313313
return true;
314314
}
315315
return false;
316-
case CSSDelimiter::CommaOrWhitespaceOrSolidus:
317-
if (parser.peek().type() == CSSTokenType::Comma ||
318-
(parser.peek().type() == CSSTokenType::Delim &&
319-
parser.peek().stringValue() == "/")) {
316+
case CSSDelimiter::SolidusOrWhitespace:
317+
if (parser.peek().type() == CSSTokenType::Delim &&
318+
parser.peek().stringValue() == "/") {
320319
parser.consumeToken();
321320
parser.consumeWhitespace();
322321
return true;

‎packages/react-native/ReactCommon/react/renderer/css/tests/CSSColorTest.cpp‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,9 @@ TEST(CSSColor, rgb_rgba_values) {
122122
EXPECT_EQ(std::get<CSSColor>(modernSyntaxValue).a, 255);
123123

124124
auto mixedDelimeterValue = parseCSSProperty<CSSColor>("rgb(255,255 255)");
125-
EXPECT_TRUE(std::holds_alternative<CSSColor>(mixedDelimeterValue));
126-
EXPECT_EQ(std::get<CSSColor>(mixedDelimeterValue).r, 255);
127-
EXPECT_EQ(std::get<CSSColor>(mixedDelimeterValue).g, 255);
128-
EXPECT_EQ(std::get<CSSColor>(mixedDelimeterValue).b, 255);
129-
EXPECT_EQ(std::get<CSSColor>(mixedDelimeterValue).a, 255);
125+
EXPECT_TRUE(std::holds_alternative<std::monostate>(mixedDelimeterValue));
130126

131-
auto mixedSpacingValue = parseCSSProperty<CSSColor>("rgb( 5 4,3)");
127+
auto mixedSpacingValue = parseCSSProperty<CSSColor>("rgb( 5 4 3)");
132128
EXPECT_TRUE(std::holds_alternative<CSSColor>(mixedSpacingValue));
133129
EXPECT_EQ(std::get<CSSColor>(mixedSpacingValue).r, 5);
134130
EXPECT_EQ(std::get<CSSColor>(mixedSpacingValue).g, 4);
@@ -155,10 +151,19 @@ TEST(CSSColor, rgb_rgba_values) {
155151
EXPECT_EQ(std::get<CSSColor>(percentageValue).g, 128);
156152
EXPECT_EQ(std::get<CSSColor>(percentageValue).b, 128);
157153

158-
auto mixedNumberPercentageValue =
154+
auto mixedLegacyNumberPercentageValue =
159155
parseCSSProperty<CSSColor>("rgb(50%, 0.5, 50%)");
160156
EXPECT_TRUE(
161-
std::holds_alternative<std::monostate>(mixedNumberPercentageValue));
157+
std::holds_alternative<std::monostate>(mixedLegacyNumberPercentageValue));
158+
159+
auto mixedModernNumberPercentageValue =
160+
parseCSSProperty<CSSColor>("rgb(50% 0.5 50%)");
161+
EXPECT_TRUE(
162+
std::holds_alternative<CSSColor>(mixedModernNumberPercentageValue));
163+
EXPECT_EQ(std::get<CSSColor>(mixedModernNumberPercentageValue).r, 128);
164+
EXPECT_EQ(std::get<CSSColor>(mixedModernNumberPercentageValue).g, 1);
165+
EXPECT_EQ(std::get<CSSColor>(mixedModernNumberPercentageValue).b, 128);
166+
EXPECT_EQ(std::get<CSSColor>(mixedModernNumberPercentageValue).a, 255);
162167

163168
auto rgbWithNumberAlphaValue =
164169
parseCSSProperty<CSSColor>("rgb(255 255 255 0.5)");
@@ -169,7 +174,7 @@ TEST(CSSColor, rgb_rgba_values) {
169174
EXPECT_EQ(std::get<CSSColor>(rgbWithNumberAlphaValue).a, 128);
170175

171176
auto rgbWithPercentageAlphaValue =
172-
parseCSSProperty<CSSColor>("rgb(255 255 255, 50%)");
177+
parseCSSProperty<CSSColor>("rgb(255 255 255 50%)");
173178
EXPECT_TRUE(std::holds_alternative<CSSColor>(rgbWithPercentageAlphaValue));
174179
EXPECT_EQ(std::get<CSSColor>(rgbWithPercentageAlphaValue).r, 255);
175180
EXPECT_EQ(std::get<CSSColor>(rgbWithPercentageAlphaValue).g, 255);
@@ -184,6 +189,11 @@ TEST(CSSColor, rgb_rgba_values) {
184189
EXPECT_EQ(std::get<CSSColor>(rgbWithSolidusAlphaValue).b, 255);
185190
EXPECT_EQ(std::get<CSSColor>(rgbWithSolidusAlphaValue).a, 128);
186191

192+
auto rgbLegacySyntaxWithSolidusAlphaValue =
193+
parseCSSProperty<CSSColor>("rgb(1, 4, 5 /0.5)");
194+
EXPECT_TRUE(std::holds_alternative<std::monostate>(
195+
rgbLegacySyntaxWithSolidusAlphaValue));
196+
187197
auto rgbaWithSolidusAlphaValue =
188198
parseCSSProperty<CSSColor>("rgba(255 255 255 / 0.5)");
189199
EXPECT_TRUE(std::holds_alternative<CSSColor>(rgbaWithSolidusAlphaValue));
@@ -236,7 +246,12 @@ TEST(CSSColor, rgb_rgba_values) {
236246
}
237247

238248
TEST(CSSColor, constexpr_values) {
239-
[[maybe_unused]] constexpr auto simpleValue =
249+
[[maybe_unused]] constexpr auto emptyValue = parseCSSProperty<CSSColor>("");
250+
251+
[[maybe_unused]] constexpr auto hexColorValue =
252+
parseCSSProperty<CSSColor>("#fff");
253+
254+
[[maybe_unused]] constexpr auto rgbFunctionValue =
240255
parseCSSProperty<CSSColor>("rgb(255, 255, 255)");
241256
}
242257

‎packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,8 @@ TEST(CSSSyntaxParser, required_whitespace_not_present) {
533533
EXPECT_EQ(delimValue2, "/");
534534
}
535535

536-
TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) {
537-
CSSSyntaxParser parser{"foo, bar / baz potato%"};
536+
TEST(CSSSyntaxParser, solidus_or_whitespace) {
537+
CSSSyntaxParser parser{"foo bar / baz potato, papaya"};
538538

539539
auto identValue1 = parser.consumeComponentValue<std::string_view>(
540540
CSSDelimiter::OptionalWhitespace, [](const CSSPreservedToken& token) {
@@ -546,8 +546,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) {
546546
EXPECT_EQ(identValue1, "foo");
547547

548548
auto identValue2 = parser.consumeComponentValue<std::string_view>(
549-
CSSDelimiter::CommaOrWhitespaceOrSolidus,
550-
[](const CSSPreservedToken& token) {
549+
CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) {
551550
EXPECT_EQ(token.type(), CSSTokenType::Ident);
552551
EXPECT_EQ(token.stringValue(), "bar");
553552
return token.stringValue();
@@ -556,8 +555,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) {
556555
EXPECT_EQ(identValue2, "bar");
557556

558557
auto identValue3 = parser.consumeComponentValue<std::string_view>(
559-
CSSDelimiter::CommaOrWhitespaceOrSolidus,
560-
[](const CSSPreservedToken& token) {
558+
CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) {
561559
EXPECT_EQ(token.type(), CSSTokenType::Ident);
562560
EXPECT_EQ(token.stringValue(), "baz");
563561
return token.stringValue();
@@ -566,8 +564,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) {
566564
EXPECT_EQ(identValue3, "baz");
567565

568566
auto identValue4 = parser.consumeComponentValue<std::string_view>(
569-
CSSDelimiter::CommaOrWhitespaceOrSolidus,
570-
[](const CSSPreservedToken& token) {
567+
CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) {
571568
EXPECT_EQ(token.type(), CSSTokenType::Ident);
572569
EXPECT_EQ(token.stringValue(), "potato");
573570
return token.stringValue();
@@ -576,7 +573,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) {
576573
EXPECT_EQ(identValue4, "potato");
577574

578575
auto delimValue1 = parser.consumeComponentValue<bool>(
579-
CSSDelimiter::CommaOrWhitespaceOrSolidus,
576+
CSSDelimiter::SolidusOrWhitespace,
580577
[](const CSSPreservedToken& token) { return true; });
581578

582579
EXPECT_FALSE(delimValue1);

0 commit comments

Comments
 (0)