From 41a55b3a57a2405b9836dcefee810e999c7a8d65 Mon Sep 17 00:00:00 2001
From: Sam Atkins <sam@ladybird.org>
Date: Tue, 7 Jan 2025 11:33:29 +0000
Subject: [PATCH] LibWeb/CSS: Remove illegal `<number-percentage>` type

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.
---
 Libraries/LibWeb/CSS/CSSNumericType.cpp       |  8 ----
 Libraries/LibWeb/CSS/CSSNumericType.h         |  1 -
 Libraries/LibWeb/CSS/Parser/Parser.cpp        | 45 +++++--------------
 .../CSS/StyleValues/CalculatedStyleValue.h    |  1 -
 4 files changed, 12 insertions(+), 43 deletions(-)

diff --git a/Libraries/LibWeb/CSS/CSSNumericType.cpp b/Libraries/LibWeb/CSS/CSSNumericType.cpp
index 9080b3d39488..40780b5cefdd 100644
--- a/Libraries/LibWeb/CSS/CSSNumericType.cpp
+++ b/Libraries/LibWeb/CSS/CSSNumericType.cpp
@@ -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.
diff --git a/Libraries/LibWeb/CSS/CSSNumericType.h b/Libraries/LibWeb/CSS/CSSNumericType.h
index 3e37fbe7a493..f29cadc3843e 100644
--- a/Libraries/LibWeb/CSS/CSSNumericType.h
+++ b/Libraries/LibWeb/CSS/CSSNumericType.h
@@ -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); }
diff --git a/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Libraries/LibWeb/CSS/Parser/Parser.cpp
index 89739ef393ed..9bccc1dbe692 100644
--- a/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -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;
diff --git a/Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
index 27a7e7a62464..b97b5bacca9d 100644
--- a/Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
+++ b/Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
@@ -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;