REGRESSION(267072@main): round/mod/rem crashes when combining length …

…& non-percentage units https://bugs.webkit.org/show_bug.cgi?id=260941 rdar://114695951 Reviewed by Cameron McCormack. The previous code assumed that the second type always would be a percentage, which is not necessarily true. `calc(mod(0px, 0deg))` is an example where the current code fails. * LayoutTests/imported/w3c/web-platform-tests/css/css-values/mod-length-degrees-crash.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-values/rem-length-degrees-crash.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-length-degrees-crash.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid.html: * Source/WebCore/css/calc/CSSCalcOperationNode.cpp: (WebCore::resolvedTypeForStep): Canonical link: https://commits.webkit.org/267485@main
WebKit · Aug 31, 2023 · b9aa388 · b9aa388
1 parent b56ed87
commit b9aa388
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 10 deletions.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-values/mod-length-degrees-crash.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-values/mod-length-degrees-crash.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+<link rel="help" href="https://bugs.webkit.org/show_bug.cgi?id=260941">
+
+<p>PASS if no crash.</p>
+<div style="height: calc(mod(0px, 0deg))"></div>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-values/rem-length-degrees-crash.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-values/rem-length-degrees-crash.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+<link rel="help" href="https://bugs.webkit.org/show_bug.cgi?id=260941">
+
+<p>PASS if no crash.</p>
+<div style="height: calc(rem(0px, 0deg))"></div>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-length-degrees-crash.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-length-degrees-crash.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+<link rel="help" href="https://bugs.webkit.org/show_bug.cgi?id=260941">
+
+<p>PASS if no crash.</p>
+<div style="height: calc(round(0px, 0deg))"></div>
diff --git a/...utTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid-expected.txt b/...utTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid-expected.txt
@@ -76,4 +76,34 @@ PASS e.style['opacity'] = "rem(1, 0deg)" should not set the property value
 PASS e.style['opacity'] = "rem(1, 0Hz)" should not set the property value
 PASS e.style['opacity'] = "rem(1, 0dpi)" should not set the property value
 PASS e.style['opacity'] = "rem(1, 0fr)" should not set the property value
+PASS e.style['height'] = "round(0px, 0s)" should not set the property value
+PASS e.style['height'] = "calc(round(0px, 0s))" should not set the property value
+PASS e.style['height'] = "round(0px, 0deg)" should not set the property value
+PASS e.style['height'] = "calc(round(0px, 0deg))" should not set the property value
+PASS e.style['height'] = "round(0px, 0Hz)" should not set the property value
+PASS e.style['height'] = "calc(round(0px, 0Hz))" should not set the property value
+PASS e.style['height'] = "round(0px, 0dpi)" should not set the property value
+PASS e.style['height'] = "calc(round(0px, 0dpi))" should not set the property value
+PASS e.style['height'] = "round(0px, 0fr)" should not set the property value
+PASS e.style['height'] = "calc(round(0px, 0fr))" should not set the property value
+PASS e.style['height'] = "mod(0px, 0s)" should not set the property value
+PASS e.style['height'] = "calc(mod(0px, 0s))" should not set the property value
+PASS e.style['height'] = "mod(0px, 0deg)" should not set the property value
+PASS e.style['height'] = "calc(mod(0px, 0deg))" should not set the property value
+PASS e.style['height'] = "mod(0px, 0Hz)" should not set the property value
+PASS e.style['height'] = "calc(mod(0px, 0Hz))" should not set the property value
+PASS e.style['height'] = "mod(0px, 0dpi)" should not set the property value
+PASS e.style['height'] = "calc(mod(0px, 0dpi))" should not set the property value
+PASS e.style['height'] = "mod(0px, 0fr)" should not set the property value
+PASS e.style['height'] = "calc(mod(0px, 0fr))" should not set the property value
+PASS e.style['height'] = "rem(0px, 0s)" should not set the property value
+PASS e.style['height'] = "calc(rem(0px, 0s))" should not set the property value
+PASS e.style['height'] = "rem(0px, 0deg)" should not set the property value
+PASS e.style['height'] = "calc(rem(0px, 0deg))" should not set the property value
+PASS e.style['height'] = "rem(0px, 0Hz)" should not set the property value
+PASS e.style['height'] = "calc(rem(0px, 0Hz))" should not set the property value
+PASS e.style['height'] = "rem(0px, 0dpi)" should not set the property value
+PASS e.style['height'] = "calc(rem(0px, 0dpi))" should not set the property value
+PASS e.style['height'] = "rem(0px, 0fr)" should not set the property value
+PASS e.style['height'] = "calc(rem(0px, 0fr))" should not set the property value
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-invalid.html
@@ -7,6 +7,11 @@
 <script src="/resources/testharnessreport.js"></script>
 <script src="../support/parsing-testcommon.js"></script>
 <script>
+function test_invalid_length_percentage(value) {
+  test_invalid_value('height', value);
+  test_invalid_value('height', `calc(${value})`);
+}
+
 function test_invalid_number(value) {
   test_invalid_value('opacity', value);
 }
@@ -91,4 +96,20 @@
 test_invalid_number('rem(1, 0Hz)');
 test_invalid_number('rem(1, 0dpi)');
 test_invalid_number('rem(1, 0fr)');
+
+test_invalid_length_percentage('round(0px, 0s)');
+test_invalid_length_percentage('round(0px, 0deg)');
+test_invalid_length_percentage('round(0px, 0Hz)');
+test_invalid_length_percentage('round(0px, 0dpi)');
+test_invalid_length_percentage('round(0px, 0fr)');
+test_invalid_length_percentage('mod(0px, 0s)');
+test_invalid_length_percentage('mod(0px, 0deg)');
+test_invalid_length_percentage('mod(0px, 0Hz)');
+test_invalid_length_percentage('mod(0px, 0dpi)');
+test_invalid_length_percentage('mod(0px, 0fr)');
+test_invalid_length_percentage('rem(0px, 0s)');
+test_invalid_length_percentage('rem(0px, 0deg)');
+test_invalid_length_percentage('rem(0px, 0Hz)');
+test_invalid_length_percentage('rem(0px, 0dpi)');
+test_invalid_length_percentage('rem(0px, 0fr)');
 </script>
diff --git a/Source/WebCore/css/calc/CSSCalcOperationNode.cpp b/Source/WebCore/css/calc/CSSCalcOperationNode.cpp
@@ -53,6 +53,11 @@ static const CalculationCategory addSubtractResult[static_cast<unsigned>(Calcula
     { CalculationCategory::Other,         CalculationCategory::PercentLength, CalculationCategory::PercentLength, CalculationCategory::Other,         CalculationCategory::PercentLength }, // CalculationCategory::PercentLength
 };
 
+static bool isSamePair(CalculationCategory a, CalculationCategory b, CalculationCategory x, CalculationCategory y)
+{
+    return (a == x && b == y) || (a == y && b == x);
+}
+
 static CalculationCategory determineCategory(const CSSCalcExpressionNode& leftSide, const CSSCalcExpressionNode& rightSide, CalcOperator op)
 {
     CalculationCategory leftCategory = leftSide.category();
@@ -185,23 +190,15 @@ static std::optional<CalculationCategory> resolvedTypeForStep(CalculationCategor
     if (a == b)
         return a;
 
-    if (a == CalculationCategory::Percent)
-        std::swap(a, b);
-
-    if (a == CalculationCategory::Length)
+    if (isSamePair(a, b, CalculationCategory::Length, CalculationCategory::Percent))
         return CalculationCategory::PercentLength;
 
-    if (a == CalculationCategory::Number)
+    if (isSamePair(a, b, CalculationCategory::Number, CalculationCategory::Percent))
         return CalculationCategory::PercentNumber;
 
     return { };
 }
 
-static bool isSamePair(CalculationCategory a, CalculationCategory b, CalculationCategory x, CalculationCategory y)
-{
-    return (a == x && b == y) || (a == y && b == x);
-}
-
 enum class SortingCategory {
     Number,
     Percent,