Skip to content

Commit

Permalink
Fix some CSSRatio Behavior (facebook#48984)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#48984

Noticed this in conjunction with another change, that I misinterpreted the ratio spec a bit. Ratios with a part less than zero are parse errors, while degenerate ratios are not (Chrome and Firefox both treat like this).

Removing usage of visitorless `consumeComponentValue()` here in preparation for next diff.

Changelog: [Internal]

Reviewed By: lenaic

Differential Revision: D68733519

fbshipit-source-id: 9afc7b7295b067a3e1469e2f80f5c9a6bea41fae
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 29, 2025
1 parent 2ed0ba0 commit 3b5dc56
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 21 deletions.
24 changes: 13 additions & 11 deletions packages/react-native/ReactCommon/react/renderer/css/CSSRatio.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ namespace facebook::react {
struct CSSRatio {
float numerator{};
float denominator{};

constexpr bool isDegenerate() const {
// If either number in the <ratio> is 0 or infinite, it represents a
// degenerate ratio (and, generally, won’t do anything).
// https://www.w3.org/TR/css-values-4/#ratios
return numerator == 0.0f ||
numerator == std::numeric_limits<float>::infinity() ||
denominator == 0.0f ||
denominator == std::numeric_limits<float>::infinity();
}
};

template <>
Expand All @@ -32,14 +42,14 @@ struct CSSDataTypeParser<CSSRatio> {
CSSSyntaxParser& parser) -> std::optional<CSSRatio> {
// <ratio> = <number [0,∞]> [ / <number [0,∞]> ]?
// https://www.w3.org/TR/css-values-4/#ratio
if (isValidRatioPart(token.numericValue())) {
if (token.numericValue() >= 0) {
float numerator = token.numericValue();

auto denominator =
peekNextCSSValue<CSSNumber>(parser, CSSDelimiter::Solidus);
if (std::holds_alternative<CSSNumber>(denominator) &&
isValidRatioPart(std::get<CSSNumber>(denominator).value)) {
parser.consumeComponentValue(CSSDelimiter::Solidus);
std::get<CSSNumber>(denominator).value >= 0) {
parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::Solidus);
return CSSRatio{numerator, std::get<CSSNumber>(denominator).value};
}

Expand All @@ -48,14 +58,6 @@ struct CSSDataTypeParser<CSSRatio> {

return {};
}

private:
static constexpr bool isValidRatioPart(float value) {
// If either number in the <ratio> is 0 or infinite, it represents a
// degenerate ratio (and, generally, won’t do anything).
// https://www.w3.org/TR/css-values-4/#ratios
return value > 0.0f && value != +std::numeric_limits<float>::infinity();
}
};

static_assert(CSSDataType<CSSRatio>);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ TEST(CSSRatio, ratio_values) {
EXPECT_EQ(std::get<CSSRatio>(fractionalNumber).numerator, 16.5f);
EXPECT_EQ(std::get<CSSRatio>(fractionalNumber).denominator, 1.0f);

auto fractionalNumerator = parseCSSProperty<CSSRatio>("16.5/9");
EXPECT_TRUE(std::holds_alternative<CSSRatio>(fractionalNumerator));
EXPECT_EQ(std::get<CSSRatio>(fractionalNumerator).numerator, 16.5f);
EXPECT_EQ(std::get<CSSRatio>(fractionalNumerator).denominator, 9.0f);

auto fractionalDenominator = parseCSSProperty<CSSRatio>("16/9.5");
EXPECT_TRUE(std::holds_alternative<CSSRatio>(fractionalDenominator));
EXPECT_EQ(std::get<CSSRatio>(fractionalDenominator).numerator, 16.0f);
}

TEST(CSSRatio, invalid_values) {
auto negativeNumber = parseCSSProperty<CSSRatio>("-16");
EXPECT_TRUE(std::holds_alternative<std::monostate>(negativeNumber));

Expand All @@ -46,18 +57,12 @@ TEST(CSSRatio, ratio_values) {

auto negativeDenominator = parseCSSProperty<CSSRatio>("16/-9");
EXPECT_TRUE(std::holds_alternative<std::monostate>(negativeDenominator));
}

auto fractionalNumerator = parseCSSProperty<CSSRatio>("16.5/9");
EXPECT_TRUE(std::holds_alternative<CSSRatio>(fractionalNumerator));
EXPECT_EQ(std::get<CSSRatio>(fractionalNumerator).numerator, 16.5f);
EXPECT_EQ(std::get<CSSRatio>(fractionalNumerator).denominator, 9.0f);

auto fractionalDenominator = parseCSSProperty<CSSRatio>("16/9.5");
EXPECT_TRUE(std::holds_alternative<CSSRatio>(fractionalDenominator));
EXPECT_EQ(std::get<CSSRatio>(fractionalDenominator).numerator, 16.0f);

TEST(CSSRatio, degenerate_values) {
auto degenerateRatio = parseCSSProperty<CSSRatio>("0");
EXPECT_TRUE(std::holds_alternative<std::monostate>(degenerateRatio));
EXPECT_TRUE(std::holds_alternative<CSSRatio>(degenerateRatio));
EXPECT_TRUE(std::get<CSSRatio>(degenerateRatio).isDegenerate());
}

} // namespace facebook::react

0 comments on commit 3b5dc56

Please sign in to comment.