LibWeb: Distinguish between Integer and Number calc() values

Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -4207,7 +4207,11 @@ Optional<CalculatedStyleValue::CalcValue> Parser::parse_calc_value(TokenStream<S
     }
 
     if (current_token.is(Token::Type::Number))
-        return CalculatedStyleValue::CalcValue { static_cast<float>(current_token.token().number_value()) };
+        return CalculatedStyleValue::CalcValue {
+            CalculatedStyleValue::Number {
+                .is_integer = current_token.token().number_type() == Token::NumberType::Integer,
+                .value = static_cast<float>(current_token.token().number_value()) }
+        };
 
     if (current_token.is(Token::Type::Dimension) || current_token.is(Token::Type::Percentage)) {
         auto maybe_dimension = parse_dimension(current_token);
@@ -4230,7 +4234,7 @@ OwnPtr<CalculatedStyleValue::CalcProductPartWithOperator> Parser::parse_calc_pro
     // Note: The default value is not used or passed around.
     auto product_with_operator = make<CalculatedStyleValue::CalcProductPartWithOperator>(
         CalculatedStyleValue::ProductOperation::Multiply,
-        CalculatedStyleValue::CalcNumberValue { 0 });
+        CalculatedStyleValue::CalcNumberValue { CalculatedStyleValue::Number { false, 0 } });
 
     tokens.skip_whitespace();
 
@@ -4249,6 +4253,7 @@ OwnPtr<CalculatedStyleValue::CalcProductPartWithOperator> Parser::parse_calc_pro
         product_with_operator->value = { parsed_calc_value.release_value() };
 
     } else if (op == "/"sv) {
+        // FIXME: Detect divide-by-zero if possible
         tokens.next_token();
         tokens.skip_whitespace();
         product_with_operator->op = CalculatedStyleValue::ProductOperation::Divide;
@@ -4268,7 +4273,7 @@ OwnPtr<CalculatedStyleValue::CalcNumberProductPartWithOperator> Parser::parse_ca
     // Note: The default value is not used or passed around.
     auto number_product_with_operator = make<CalculatedStyleValue::CalcNumberProductPartWithOperator>(
         CalculatedStyleValue::ProductOperation::Multiply,
-        CalculatedStyleValue::CalcNumberValue { 0 });
+        CalculatedStyleValue::CalcNumberValue { CalculatedStyleValue::Number { false, 0 } });
 
     tokens.skip_whitespace();
 
@@ -4282,6 +4287,7 @@ OwnPtr<CalculatedStyleValue::CalcNumberProductPartWithOperator> Parser::parse_ca
         tokens.skip_whitespace();
         number_product_with_operator->op = CalculatedStyleValue::ProductOperation::Multiply;
     } else if (op == "/"sv) {
+        // FIXME: Detect divide-by-zero if possible
         tokens.next_token();
         tokens.skip_whitespace();
         number_product_with_operator->op = CalculatedStyleValue::ProductOperation::Divide;
@@ -4300,7 +4306,7 @@ OwnPtr<CalculatedStyleValue::CalcNumberProductPartWithOperator> Parser::parse_ca
 OwnPtr<CalculatedStyleValue::CalcNumberProduct> Parser::parse_calc_number_product(TokenStream<StyleComponentValueRule>& tokens)
 {
     auto calc_number_product = make<CalculatedStyleValue::CalcNumberProduct>(
-        CalculatedStyleValue::CalcNumberValue { 0 },
+        CalculatedStyleValue::CalcNumberValue { CalculatedStyleValue::Number { false, 0 } },
         NonnullOwnPtrVector<CalculatedStyleValue::CalcNumberProductPartWithOperator> {});
 
     auto first_calc_number_value_or_error = parse_calc_number_value(tokens);
@@ -4378,13 +4384,17 @@ Optional<CalculatedStyleValue::CalcNumberValue> Parser::parse_calc_number_value(
         return {};
     tokens.next_token();
 
-    return CalculatedStyleValue::CalcNumberValue { static_cast<float>(first.token().number_value()) };
+    return CalculatedStyleValue::CalcNumberValue {
+        CalculatedStyleValue::Number {
+            .is_integer = first.token().number_type() == Token::NumberType::Integer,
+            .value = static_cast<float>(first.token().number_value()) }
+    };
 }
 
 OwnPtr<CalculatedStyleValue::CalcProduct> Parser::parse_calc_product(TokenStream<StyleComponentValueRule>& tokens)
 {
     auto calc_product = make<CalculatedStyleValue::CalcProduct>(
-        CalculatedStyleValue::CalcValue { 0 },
+        CalculatedStyleValue::CalcValue { CalculatedStyleValue::Number { false, 0 } },
         NonnullOwnPtrVector<CalculatedStyleValue::CalcProductPartWithOperator> {});
 
     auto first_calc_value_or_error = parse_calc_value(tokens);

Userland/Libraries/LibWeb/CSS/StyleValue.cpp

@@ -291,11 +291,19 @@ void CalculatedStyleValue::CalculationResult::add_or_subtract_internal(SumOperat
     // Note: This is almost identical to ::add()
 
     m_value.visit(
-        [&](float f) {
-            if (op == SumOperation::Add)
-                m_value = f + other.m_value.get<float>();
-            else
-                m_value = f - other.m_value.get<float>();
+        [&](Number const& number) {
+            auto other_number = other.m_value.get<Number>();
+            if (op == SumOperation::Add) {
+                m_value = Number {
+                    .is_integer = number.is_integer && other_number.is_integer,
+                    .value = number.value + other_number.value
+                };
+            } else {
+                m_value = Number {
+                    .is_integer = number.is_integer && other_number.is_integer,
+                    .value = number.value - other_number.value
+                };
+            }
         },
         [&](Length const& length) {
             auto this_px = length.to_px(*layout_node);
@@ -339,13 +347,17 @@ void CalculatedStyleValue::CalculationResult::multiply_by(CalculationResult cons
 {
     // We know from validation when resolving the type, that at least one side must be a <number> or <integer>.
     // Both of these are represented as a float.
-    VERIFY(m_value.has<float>() || other.m_value.has<float>());
-    bool other_is_number = other.m_value.has<float>();
+    VERIFY(m_value.has<Number>() || other.m_value.has<Number>());
+    bool other_is_number = other.m_value.has<Number>();
 
     m_value.visit(
-        [&](float f) {
+        [&](Number const& number) {
             if (other_is_number) {
-                m_value = f * other.m_value.get<float>();
+                auto other_number = other.m_value.get<Number>();
+                m_value = Number {
+                    .is_integer = number.is_integer && other_number.is_integer,
+                    .value = number.value * other_number.value
+                };
             } else {
                 // Avoid duplicating all the logic by swapping `this` and `other`.
                 CalculationResult new_value = other;
@@ -355,24 +367,27 @@ void CalculatedStyleValue::CalculationResult::multiply_by(CalculationResult cons
         },
         [&](Length const& length) {
             VERIFY(layout_node);
-            m_value = Length::make_px(length.to_px(*layout_node) * other.m_value.get<float>());
+            m_value = Length::make_px(length.to_px(*layout_node) * other.m_value.get<Number>().value);
         },
         [&](Percentage const& percentage) {
-            m_value = Percentage { percentage.value() * other.m_value.get<float>() };
+            m_value = Percentage { percentage.value() * other.m_value.get<Number>().value };
         });
 }
 
 void CalculatedStyleValue::CalculationResult::divide_by(CalculationResult const& other, Layout::Node const* layout_node)
 {
     // We know from validation when resolving the type, that `other` must be a <number> or <integer>.
-    // Both of these are represented as a float.
-    float denominator = other.m_value.get<float>();
+    // Both of these are represented as a Number.
+    auto denominator = other.m_value.get<Number>().value;
     // FIXME: Dividing by 0 is invalid, and should be caught during parsing.
     VERIFY(denominator != 0.0f);
 
     m_value.visit(
-        [&](float f) {
-            m_value = f / denominator;
+        [&](Number const& number) {
+            m_value = Number {
+                .is_integer = false,
+                .value = number.value / denominator
+            };
         },
         [&](Length const& length) {
             VERIFY(layout_node);
@@ -388,7 +403,7 @@ Optional<Length> CalculatedStyleValue::resolve_length(Layout::Node const& layout
     auto result = m_expression->resolve(&layout_node, {});
 
     return result.value().visit(
-        [&](float) -> Optional<Length> {
+        [&](Number) -> Optional<Length> {
             return {};
         },
         [&](Length const& length) -> Optional<Length> {
@@ -405,7 +420,7 @@ Optional<LengthPercentage> CalculatedStyleValue::resolve_length_percentage(Layou
     auto result = m_expression->resolve(&layout_node, percentage_basis);
 
     return result.value().visit(
-        [&](float) -> Optional<LengthPercentage> {
+        [&](Number) -> Optional<LengthPercentage> {
             return {};
         },
         [&](Length const& length) -> Optional<LengthPercentage> {
@@ -427,16 +442,16 @@ Optional<Percentage> CalculatedStyleValue::resolve_percentage() const
 Optional<float> CalculatedStyleValue::resolve_number()
 {
     auto result = m_expression->resolve(nullptr, {});
-    if (result.value().has<float>())
-        return result.value().get<float>();
+    if (result.value().has<Number>())
+        return result.value().get<Number>().value;
     return {};
 }
 
 Optional<i64> CalculatedStyleValue::resolve_integer()
 {
     auto result = m_expression->resolve(nullptr, {});
-    if (result.value().has<float>())
-        return lroundf(result.value().get<float>());
+    if (result.value().has<Number>())
+        return lroundf(result.value().get<Number>().value);
     return {};
 }
 
@@ -596,7 +611,9 @@ Optional<CalculatedStyleValue::ResolvedType> CalculatedStyleValue::CalcProductPa
 Optional<CalculatedStyleValue::ResolvedType> CalculatedStyleValue::CalcValue::resolved_type() const
 {
     return value.visit(
-        [](float) -> Optional<CalculatedStyleValue::ResolvedType> { return { ResolvedType::Number }; },
+        [](Number const& number) -> Optional<CalculatedStyleValue::ResolvedType> {
+            return { number.is_integer ? ResolvedType::Integer : ResolvedType::Number };
+        },
         [](Length const&) -> Optional<CalculatedStyleValue::ResolvedType> { return { ResolvedType::Length }; },
         [](Percentage const&) -> Optional<CalculatedStyleValue::ResolvedType> { return { ResolvedType::Percentage }; },
         [](NonnullOwnPtr<CalcSum> const& sum) { return sum->resolved_type(); });
@@ -605,15 +622,17 @@ Optional<CalculatedStyleValue::ResolvedType> CalculatedStyleValue::CalcValue::re
 Optional<CalculatedStyleValue::ResolvedType> CalculatedStyleValue::CalcNumberValue::resolved_type() const
 {
     return value.visit(
-        [](float) -> Optional<CalculatedStyleValue::ResolvedType> { return { ResolvedType::Number }; },
+        [](Number const& number) -> Optional<CalculatedStyleValue::ResolvedType> {
+            return { number.is_integer ? ResolvedType::Integer : ResolvedType::Number };
+        },
         [](NonnullOwnPtr<CalcNumberSum> const& sum) { return sum->resolved_type(); });
 }
 
 CalculatedStyleValue::CalculationResult CalculatedStyleValue::CalcNumberValue::resolve(Layout::Node const* layout_node, Length const& percentage_basis) const
 {
     return value.visit(
-        [&](float f) -> CalculatedStyleValue::CalculationResult {
-            return CalculatedStyleValue::CalculationResult { f };
+        [&](Number const& number) -> CalculatedStyleValue::CalculationResult {
+            return CalculatedStyleValue::CalculationResult { number };
         },
         [&](NonnullOwnPtr<CalcNumberSum> const& sum) -> CalculatedStyleValue::CalculationResult {
             return sum->resolve(layout_node, percentage_basis);
@@ -623,8 +642,8 @@ CalculatedStyleValue::CalculationResult CalculatedStyleValue::CalcNumberValue::r
 CalculatedStyleValue::CalculationResult CalculatedStyleValue::CalcValue::resolve(Layout::Node const* layout_node, Length const& percentage_basis) const
 {
     return value.visit(
-        [&](float f) -> CalculatedStyleValue::CalculationResult {
-            return CalculatedStyleValue::CalculationResult { f };
+        [&](Number const& number) -> CalculatedStyleValue::CalculationResult {
+            return CalculatedStyleValue::CalculationResult { number };
         },
         [&](Length const& length) -> CalculatedStyleValue::CalculationResult {
             return CalculatedStyleValue::CalculationResult { length };
@@ -687,6 +706,8 @@ CalculatedStyleValue::CalculationResult CalculatedStyleValue::CalcProduct::resol
             [&](CalculatedStyleValue::CalcNumberValue const& calc_number_value) {
                 VERIFY(additional_value.op == CalculatedStyleValue::ProductOperation::Divide);
                 auto resolved_calc_number_value = calc_number_value.resolve(layout_node, percentage_basis);
+                // FIXME: Checking for division by 0 should happen during parsing.
+                VERIFY(resolved_calc_number_value.value().get<Number>().value != 0.0f);
                 value.divide_by(resolved_calc_number_value, layout_node);
             });
     }

Userland/Libraries/LibWeb/CSS/StyleValue.h

@@ -673,9 +673,14 @@ public:
         Divide,
     };
 
+    struct Number {
+        bool is_integer;
+        float value;
+    };
+
     class CalculationResult {
     public:
-        CalculationResult(Variant<float, Length, Percentage> value)
+        CalculationResult(Variant<Number, Length, Percentage> value)
             : m_value(move(value))
         {
         }
@@ -684,11 +689,11 @@ public:
         void multiply_by(CalculationResult const& other, Layout::Node const*);
         void divide_by(CalculationResult const& other, Layout::Node const*);
 
-        Variant<float, Length, Percentage> const& value() const { return m_value; }
+        Variant<Number, Length, Percentage> const& value() const { return m_value; }
 
     private:
         void add_or_subtract_internal(SumOperation op, CalculationResult const& other, Layout::Node const*, Length const& percentage_basis);
-        Variant<float, Length, Percentage> m_value;
+        Variant<Number, Length, Percentage> m_value;
     };
 
     struct CalcSum;
@@ -701,13 +706,13 @@ public:
     struct CalcNumberProductPartWithOperator;
 
     struct CalcNumberValue {
-        Variant<float, NonnullOwnPtr<CalcNumberSum>> value;
+        Variant<Number, NonnullOwnPtr<CalcNumberSum>> value;
         Optional<ResolvedType> resolved_type() const;
         CalculationResult resolve(Layout::Node const*, Length const& percentage_basis) const;
     };
 
     struct CalcValue {
-        Variant<float, Length, Percentage, NonnullOwnPtr<CalcSum>> value;
+        Variant<Number, Length, Percentage, NonnullOwnPtr<CalcSum>> value;
         Optional<ResolvedType> resolved_type() const;
         CalculationResult resolve(Layout::Node const*, Length const& percentage_basis) const;
     };