LibWeb/CSS: Backtrack the parser if a property does not accept a value

Author: https://github.com/tcl3
Commit: 028bcd3d67
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5088
Reviewed-by: https://github.com/gmta ✅

Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp

@@ -214,73 +214,110 @@ Optional<Parser::PropertyAndValue> Parser::parse_css_value_for_properties(Readon
     // <integer>/<number> come before <length>, so that 0 is not interpreted as a <length> in case both are allowed.
     if (auto property = any_property_accepts_type(property_ids, ValueType::Integer); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (auto value = parse_integer_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_integer() && property_accepts_integer(*property, value->as_integer().integer()))
+            }
+            if (value->is_integer() && property_accepts_integer(*property, value->as_integer().integer())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Number); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (auto value = parse_number_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_number() && property_accepts_number(*property, value->as_number().number()))
+            }
+            if (value->is_number() && property_accepts_number(*property, value->as_number().number())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Angle); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (property_accepts_type(*property, ValueType::Percentage)) {
             if (auto value = parse_angle_percentage_value(tokens)) {
-                if (value->is_calculated())
+                if (value->is_calculated()) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_angle() && property_accepts_angle(*property, value->as_angle().angle()))
+                }
+                if (value->is_angle() && property_accepts_angle(*property, value->as_angle().angle())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage()))
+                }
+                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
                 }
             }
+        }
         if (auto value = parse_angle_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_angle() && property_accepts_angle(*property, value->as_angle().angle()))
+            }
+            if (value->is_angle() && property_accepts_angle(*property, value->as_angle().angle())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Flex); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (auto value = parse_flex_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_flex() && property_accepts_flex(*property, value->as_flex().flex()))
+            }
+            if (value->is_flex() && property_accepts_flex(*property, value->as_flex().flex())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Frequency); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (property_accepts_type(*property, ValueType::Percentage)) {
             if (auto value = parse_frequency_percentage_value(tokens)) {
-                if (value->is_calculated())
+                if (value->is_calculated()) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_frequency() && property_accepts_frequency(*property, value->as_frequency().frequency()))
+                }
+                if (value->is_frequency() && property_accepts_frequency(*property, value->as_frequency().frequency())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage()))
+                }
+                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
                 }
             }
+        }
         if (auto value = parse_frequency_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_frequency() && property_accepts_frequency(*property, value->as_frequency().frequency()))
+            }
+            if (value->is_frequency() && property_accepts_frequency(*property, value->as_frequency().frequency())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::FitContent); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
@@ -290,64 +327,96 @@ Optional<Parser::PropertyAndValue> Parser::parse_css_value_for_properties(Readon
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Length); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (property_accepts_type(*property, ValueType::Percentage)) {
             if (auto value = parse_length_percentage_value(tokens)) {
-                if (value->is_calculated())
+                if (value->is_calculated()) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_length() && property_accepts_length(*property, value->as_length().length()))
+                }
+                if (value->is_length() && property_accepts_length(*property, value->as_length().length())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage()))
+                }
+                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
                 }
             }
+        }
         if (auto value = parse_length_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_length() && property_accepts_length(*property, value->as_length().length()))
+            }
+            if (value->is_length() && property_accepts_length(*property, value->as_length().length())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Resolution); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (auto value = parse_resolution_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_resolution() && property_accepts_resolution(*property, value->as_resolution().resolution()))
+            }
+            if (value->is_resolution() && property_accepts_resolution(*property, value->as_resolution().resolution())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Time); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (property_accepts_type(*property, ValueType::Percentage)) {
             if (auto value = parse_time_percentage_value(tokens)) {
-                if (value->is_calculated())
+                if (value->is_calculated()) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_time() && property_accepts_time(*property, value->as_time().time()))
+                }
+                if (value->is_time() && property_accepts_time(*property, value->as_time().time())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
-                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage()))
+                }
+                if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage())) {
+                    transaction.commit();
                     return PropertyAndValue { *property, value };
                 }
             }
+        }
         if (auto value = parse_time_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_time() && property_accepts_time(*property, value->as_time().time()))
+            }
+            if (value->is_time() && property_accepts_time(*property, value->as_time().time())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     // <percentage> is checked after the <foo-percentage> types.
     if (auto property = any_property_accepts_type(property_ids, ValueType::Percentage); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);
+        auto transaction = tokens.begin_transaction();
         if (auto value = parse_percentage_value(tokens)) {
-            if (value->is_calculated())
+            if (value->is_calculated()) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
-            if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage()))
+            }
+            if (value->is_percentage() && property_accepts_percentage(*property, value->as_percentage().percentage())) {
+                transaction.commit();
                 return PropertyAndValue { *property, value };
             }
         }
+    }
 
     if (auto property = any_property_accepts_type(property_ids, ValueType::Paint); property.has_value()) {
         auto context_guard = push_temporary_value_parsing_context(*property);

Tests/LibWeb/Text/expected/wpt-import/css/css-align/gaps/gap-parsing-001.txt

@@ -2,8 +2,7 @@ Harness status: OK
 
 Found 26 tests
 
-25 Pass
+26 Pass
-1 Fail
 Pass	Default gap is 'normal'
 Pass	gap accepts pixels
 Pass	gap accepts pixels 2
@@ -29,4 +28,4 @@ Pass	Resolution gap is invalid
 Pass	Time gap is invalid
 Pass	gap with three values is invalid
 Pass	gap with slash is invalid
-Fail	gap with one wrong value is invalid
+Pass	gap with one wrong value is invalid

Tests/LibWeb/Text/expected/wpt-import/css/css-align/gaps/grid-gap-parsing-001.txt

@@ -2,8 +2,7 @@ Harness status: OK
 
 Found 26 tests
 
-25 Pass
+26 Pass
-1 Fail
 Pass	Default grid-gap is 'normal'
 Pass	grid-gap accepts pixels
 Pass	grid-gap accepts pixels 2
@@ -29,4 +28,4 @@ Pass	Resolution grid-gap is invalid
 Pass	Time grid-gap is invalid
 Pass	grid-gap with three values is invalid
 Pass	grid-gap with slash is invalid
-Fail	grid-gap with one wrong value is invalid
+Pass	grid-gap with one wrong value is invalid

Tests/LibWeb/Text/expected/wpt-import/css/css-align/parsing/grid-gap-invalid.txt

@@ -0,0 +1,9 @@
+Harness status: OK
+
+Found 4 tests
+
+4 Pass
+Pass	e.style['grid-gap'] = "auto" should not set the property value
+Pass	e.style['grid-gap'] = "-10px" should not set the property value
+Pass	e.style['grid-gap'] = "10px 20% 30px" should not set the property value
+Pass	e.style['grid-gap'] = "normal 10px normal" should not set the property value

Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/background-size-invalid.txt

@@ -0,0 +1,8 @@
+Harness status: OK
+
+Found 3 tests
+
+3 Pass
+Pass	e.style['background-size'] = "-1px" should not set the property value
+Pass	e.style['background-size'] = "2% -3%" should not set the property value
+Pass	e.style['background-size'] = "1px 2px 3px" should not set the property value

Tests/LibWeb/Text/expected/wpt-import/css/css-multicol/parsing/columns-invalid.txt

@@ -0,0 +1,14 @@
+Harness status: OK
+
+Found 9 tests
+
+9 Pass
+Pass	e.style['columns'] = "none" should not set the property value
+Pass	e.style['columns'] = "10px 20px" should not set the property value
+Pass	e.style['columns'] = "10 20" should not set the property value
+Pass	e.style['columns'] = "0 0" should not set the property value
+Pass	e.style['columns'] = "0 7px" should not set the property value
+Pass	e.style['columns'] = "auto auto auto" should not set the property value
+Pass	e.style['columns'] = "10em auto auto" should not set the property value
+Pass	e.style['columns'] = "initial initial" should not set the property value
+Pass	e.style['columns'] = "inherit inherit" should not set the property value

Tests/LibWeb/Text/expected/wpt-import/css/css-multicol/parsing/columns-valid.txt

@@ -0,0 +1,20 @@
+Harness status: OK
+
+Found 15 tests
+
+15 Pass
+Pass	e.style['columns'] = "auto 3" should set the property value
+Pass	e.style['columns'] = "auto 10em" should set the property value
+Pass	e.style['columns'] = "3 auto" should set the property value
+Pass	e.style['columns'] = "10em auto" should set the property value
+Pass	e.style['columns'] = "2 10px" should set the property value
+Pass	e.style['columns'] = "10px 2" should set the property value
+Pass	e.style['columns'] = "auto" should set the property value
+Pass	e.style['columns'] = "auto auto" should set the property value
+Pass	e.style['columns'] = "7" should set the property value
+Pass	e.style['columns'] = "7em" should set the property value
+Pass	e.style['columns'] = "0 1" should set the property value
+Pass	e.style['columns'] = "1 0" should set the property value
+Pass	e.style['columns'] = "0px 1" should set the property value
+Pass	e.style['columns'] = "initial" should set the property value
+Pass	e.style['columns'] = "inherit" should set the property value

Tests/LibWeb/Text/input/wpt-import/css/css-align/parsing/grid-gap-invalid.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Box Alignment Level 3: parsing grid-gap with invalid values</title>
+<link rel="help" href="https://drafts.csswg.org/css-align/#grid-gap-legacy">
+<link rel="author" title="Takuya Kurimoto" href="mailto:takuya004869@gmail.com">
+<meta name="assert" content="grid-gap supports only the grammar '<grid-row-gap> <grid-column-gap>?'.">
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../../../css/support/parsing-testcommon.js"></script>
+</head>
+<body>
+<script>
+test_invalid_value("grid-gap", "auto");
+test_invalid_value("grid-gap", "-10px");
+
+test_invalid_value("grid-gap", "10px 20% 30px");
+test_invalid_value("grid-gap", "normal 10px normal");
+</script>
+</body>
+</html>

Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/background-size-invalid.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Backgrounds and Borders Module Level 3: parsing background-size with invalid values</title>
+<link rel="author" title="Eric Willigers" href="mailto:ericwilligers@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#background-size">
+<meta name="assert" content="background-size supports only the grammar '<bg-size>#'.">
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../../../css/support/parsing-testcommon.js"></script>
+</head>
+<body>
+<script>
+// Blink and WebKit fail these by accepting negative values.
+test_invalid_value("background-size", "-1px");
+test_invalid_value("background-size", "2% -3%");
+
+test_invalid_value("background-size", "1px 2px 3px");
+</script>
+</body>
+</html>

Tests/LibWeb/Text/input/wpt-import/css/css-multicol/parsing/columns-invalid.html

@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Multi-column Layout: parsing columns with invalid values</title>
+<link rel="help" href="https://drafts.csswg.org/css-multicol/#propdef-columns">
+<meta name="assert" content="columns supports only the grammar '<column-width> || <column-count>'.">
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../../../css/support/parsing-testcommon.js"></script>
+</head>
+<body>
+<script>
+// Invalid keyword.
+test_invalid_value("columns", "none");
+
+// Only column-count can be unitless.
+test_invalid_value("columns", "10px 20px");
+test_invalid_value("columns", "10 20");
+
+// column-count needs to be 1 or more.
+test_invalid_value("columns", "0 0");
+test_invalid_value("columns", "0 7px");
+
+// Excess keywords.
+test_invalid_value("columns", "auto auto auto");
+test_invalid_value("columns", "10em auto auto");
+test_invalid_value("columns", "initial initial");
+test_invalid_value("columns", "inherit inherit");
+</script>
+</body>
+</html>

Tests/LibWeb/Text/input/wpt-import/css/css-multicol/parsing/columns-valid.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Multi-column Layout: parsing columns with valid values</title>
+<link rel="help" href="https://drafts.csswg.org/css-multicol/#propdef-columns">
+<meta name="assert" content="columns supports the full grammar '<column-width> || <column-count>'.">
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../../../css/support/parsing-testcommon.js"></script>
+</head>
+<body>
+<script>
+test_valid_value("columns", "auto 3", "3");
+test_valid_value("columns", "auto 10em", "10em");
+test_valid_value("columns", "3 auto", "3");
+test_valid_value("columns", "10em auto", "10em");
+test_valid_value("columns", "2 10px", "10px 2");
+test_valid_value("columns", "10px 2");
+test_valid_value("columns", "auto");
+test_valid_value("columns", "auto auto", "auto");
+test_valid_value("columns", "7");
+test_valid_value("columns", "7em");
+
+// Unitless zero is allowed for column-width.
+test_valid_value("columns", "0 1", "0px 1");
+test_valid_value("columns", "1 0", "0px 1");
+test_valid_value("columns", "0px 1");
+
+// CSS-wide keywords.
+test_valid_value("columns", "initial");
+test_valid_value("columns", "inherit");
+</script>
+</body>
+</html>