Skip to content

Commit

Permalink
Add CSSComponentValueDelimiter::CommaOrWhitespace (#48769)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #48769

Must function notations do not require delimiting commas, with color function "legacy" syntax being an exception.

E.g.

```
rgb() = [ <legacy-rgb-syntax> | <modern-rgb-syntax> ]
rgba() = [ <legacy-rgba-syntax> | <modern-rgba-syntax> ]
<legacy-rgb-syntax> =   rgb( <percentage>#{3} , <alpha-value>? ) |
                  rgb( <number>#{3} , <alpha-value>? )
<legacy-rgba-syntax> = rgba( <percentage>#{3} , <alpha-value>? ) |
                  rgba( <number>#{3} , <alpha-value>? )
<modern-rgb-syntax> = rgb(
  [ <number> | <percentage> | none]{3}
  [ / [<alpha-value> | none] ]?  )
<modern-rgba-syntax> = rgba(
  [ <number> | <percentage> | none]{3}
  [ / [<alpha-value> | none] ]?  )
```

Theoretically, this should mean an expression like `rgb(1, 2 0)`, which mixes both comma and whitespace delimeters would be malformed, but both Chrome, and RN's existing `normalize-color` package support this, so to make this pattern easier, we add the ability to scan based on using either whitespace or comma as component value delimiter.

Interestingly, the current spec revision also allows `rgb` function with alpha, or `rgba` function without alpha, which Chrome correctly supports, but `normalize-color` does not.

Changelog: [Internal]

Reviewed By: joevilches

Differential Revision: D68349385

fbshipit-source-id: 05cd756ee20227af4d9ec20edc9e7cfc8cc2c071
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 17, 2025
1 parent c33dd62 commit 743de70
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ concept CSSUniqueComponentValueVisitors =
enum class CSSComponentValueDelimiter {
Comma,
Whitespace,
CommaOrWhitespace,
None,
};

Expand Down Expand Up @@ -212,6 +213,13 @@ struct CSSComponentValueVisitorDispatcher {
case CSSComponentValueDelimiter::Whitespace:
parser.consumeWhitespace();
break;
case CSSComponentValueDelimiter::CommaOrWhitespace:
parser.consumeWhitespace();
if (parser.peek().type() == CSSTokenType::Comma) {
parser.consumeToken();
}
parser.consumeWhitespace();
break;
case CSSComponentValueDelimiter::None:
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,52 @@ TEST(CSSSyntaxParser, single_function_with_comma_delimited_args) {
EXPECT_EQ(funcArgs, expectedArgs);
}

TEST(CSSSyntaxParser, single_function_with_mixed_delimited_args) {
CSSSyntaxParser parser{"rgb(100, 200 50 )"};

auto funcArgs = parser.consumeComponentValue<std::array<uint8_t, 3>>(
[&](const CSSFunctionBlock& function, CSSSyntaxParser& blockParser) {
EXPECT_EQ(function.name, "rgb");

std::array<uint8_t, 3> rgb{};

rgb[0] = blockParser.consumeComponentValue<uint8_t>(
CSSComponentValueDelimiter::None,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Number);
EXPECT_EQ(token.numericValue(), 100);
return static_cast<uint8_t>(token.numericValue());
});

rgb[1] = blockParser.consumeComponentValue<uint8_t>(
CSSComponentValueDelimiter::CommaOrWhitespace,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Number);
EXPECT_EQ(token.numericValue(), 200);
return static_cast<uint8_t>(token.numericValue());
});

rgb[2] = blockParser.consumeComponentValue<uint8_t>(
CSSComponentValueDelimiter::CommaOrWhitespace,
[](const CSSPreservedToken& token) {
EXPECT_EQ(token.type(), CSSTokenType::Number);
EXPECT_EQ(token.numericValue(), 50);
return static_cast<uint8_t>(token.numericValue());
});

auto hasMoreTokens = blockParser.consumeComponentValue<bool>(
CSSComponentValueDelimiter::Whitespace,
[](const CSSPreservedToken& /*token*/) { return true; });

EXPECT_FALSE(hasMoreTokens);

return rgb;
});

std::array<uint8_t, 3> expectedArgs{{100, 200, 50}};
EXPECT_EQ(funcArgs, expectedArgs);
}

TEST(CSSSyntaxParser, complex) {
CSSSyntaxParser parser{"foo(a bar())baz() 12px"};

Expand Down

0 comments on commit 743de70

Please sign in to comment.