Skip to content

Commit

Permalink
LibWeb/CSS: Remove illegal <number-percentage> type
Browse files Browse the repository at this point in the history
Various places in the spec allow for `<number> | <percentage>`, but this
is either/or, and they are not allowed to be combined like dimensions
and percentages are. (For example, `calc(12 + 50%)` is never valid.)

User code generally doesn't need to care about this distinction, but it
does now need to check if a calculation resolves to a number, or to a
percentage, instead of a single call.

The existing parse_number_percentage[_value]() methods have been kept
for simplicity, but updated to check for number/percentage separately.
  • Loading branch information
AtkinsSJ committed Jan 7, 2025
1 parent e5c4b77 commit 41a55b3
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 43 deletions.
8 changes: 0 additions & 8 deletions Libraries/LibWeb/CSS/CSSNumericType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,6 @@ bool CSSNumericType::matches_number() const
return true;
}

// https://drafts.css-houdini.org/css-typed-om-1/#cssnumericvalue-match
bool CSSNumericType::matches_number_percentage() const
{
// FIXME: This is incorrect. <number-percentage> is not a legal type. Instead we should check separately if this
// matches a number, or if it matches a percentage.
return matches_number() || matches_percentage();
}

bool CSSNumericType::matches_dimension() const
{
// This isn't a spec algorithm.
Expand Down
1 change: 0 additions & 1 deletion Libraries/LibWeb/CSS/CSSNumericType.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class CSSNumericType {
bool matches_length() const { return matches_dimension(BaseType::Length); }
bool matches_length_percentage() const { return matches_dimension_percentage(BaseType::Length); }
bool matches_number() const;
bool matches_number_percentage() const;
bool matches_percentage() const;
bool matches_resolution() const { return matches_dimension(BaseType::Resolution); }
bool matches_resolution_percentage() const { return matches_dimension_percentage(BaseType::Resolution); }
Expand Down
45 changes: 12 additions & 33 deletions Libraries/LibWeb/CSS/Parser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2489,45 +2489,24 @@ RefPtr<CSSStyleValue> Parser::parse_number_value(TokenStream<ComponentValue>& to
RefPtr<CSSStyleValue> Parser::parse_number_percentage_value(TokenStream<ComponentValue>& tokens)
{
// Parses [<percentage> | <number>] (which is equivalent to [<alpha-value>])
auto const& peek_token = tokens.next_token();
if (peek_token.is(Token::Type::Number)) {
tokens.discard_a_token(); // number
return NumberStyleValue::create(peek_token.token().number().value());
}
if (peek_token.is(Token::Type::Percentage)) {
tokens.discard_a_token(); // percentage
return PercentageStyleValue::create(Percentage(peek_token.token().percentage()));
}
if (auto calc = parse_calculated_value(peek_token); calc && calc->resolves_to_number_percentage()) {
tokens.discard_a_token(); // calc
return calc;
}

if (auto value = parse_number_value(tokens))
return value;
if (auto value = parse_percentage_value(tokens))
return value;
return nullptr;
}

RefPtr<CSSStyleValue> Parser::parse_number_percentage_none_value(TokenStream<ComponentValue>& tokens)
{
// Parses [<percentage> | <number> | none] (which is equivalent to [<alpha-value> | none])
auto peek_token = tokens.next_token();
if (peek_token.is(Token::Type::Number)) {
tokens.discard_a_token(); // number
return NumberStyleValue::create(peek_token.token().number().value());
}
if (peek_token.is(Token::Type::Percentage)) {
tokens.discard_a_token(); // percentage
return PercentageStyleValue::create(Percentage(peek_token.token().percentage()));
}
if (auto calc = parse_calculated_value(peek_token); calc && calc->resolves_to_number_percentage()) {
tokens.discard_a_token(); // calc
return calc;
}
if (peek_token.is(Token::Type::Ident)) {
auto keyword = keyword_from_string(peek_token.token().ident());
if (keyword.has_value() && keyword.value() == Keyword::None) {
tokens.discard_a_token(); // keyword none
return CSSKeywordValue::create(keyword.value());
}
if (auto value = parse_number_value(tokens))
return value;
if (auto value = parse_percentage_value(tokens))
return value;

if (tokens.next_token().is_ident("none"sv)) {
tokens.discard_a_token(); // keyword none
return CSSKeywordValue::create(Keyword::None);
}

return nullptr;
Expand Down
1 change: 0 additions & 1 deletion Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ class CalculatedStyleValue : public CSSStyleValue {
Optional<Time> resolve_time_percentage(Time const& percentage_basis) const;

bool resolves_to_number() const { return m_resolved_type.matches_number(); }
bool resolves_to_number_percentage() const { return m_resolved_type.matches_number_percentage(); }
Optional<double> resolve_number() const;
Optional<double> resolve_number(Length::ResolutionContext const&) const;
Optional<double> resolve_number(Layout::Node const& layout_node) const;
Expand Down

0 comments on commit 41a55b3

Please sign in to comment.