LibWeb: Merge background-position parsing into position code

Implemented by adding the extra 3-value syntax as its own case and only
running it when parsing background-position. I'm sure it could be
implemented in a smarter way but this is still a bunch less code than
before. :^)

Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp

@@ -21,7 +21,7 @@ void generate_bounds_checking_function(JsonObject& properties, SourceGenerator&
 static bool type_name_is_enum(StringView type_name)
 {
     return !AK::first_is_one_of(type_name,
-        "angle"sv, "color"sv, "custom-ident"sv, "easing-function"sv, "flex"sv, "frequency"sv, "image"sv,
+        "angle"sv, "background-position"sv, "color"sv, "custom-ident"sv, "easing-function"sv, "flex"sv, "frequency"sv, "image"sv,
         "integer"sv, "length"sv, "number"sv, "paint"sv, "percentage"sv, "position"sv, "ratio"sv, "rect"sv,
         "resolution"sv, "string"sv, "time"sv, "url"sv);
 }
@@ -168,6 +168,7 @@ NonnullRefPtr<StyleValue> property_initial_value(JS::Realm&, PropertyID);
 
 enum class ValueType {
     Angle,
+    BackgroundPosition,
     Color,
     CustomIdent,
     EasingFunction,
@@ -616,6 +617,8 @@ bool property_accepts_type(PropertyID property_id, ValueType value_type)
 
                 if (type_name == "angle") {
                     property_generator.appendln("        case ValueType::Angle:");
+                } else if (type_name == "background-position") {
+                    property_generator.appendln("        case ValueType::BackgroundPosition:");
                 } else if (type_name == "color") {
                     property_generator.appendln("        case ValueType::Color:");
                 } else if (type_name == "custom-ident") {

Userland/Libraries/LibWeb/CSS/CSSNumericType.cpp

@@ -33,6 +33,7 @@ Optional<CSSNumericType::BaseType> CSSNumericType::base_type_from_value_type(Val
     case ValueType::Time:
         return BaseType::Time;
 
+    case ValueType::BackgroundPosition:
     case ValueType::Color:
     case ValueType::CustomIdent:
     case ValueType::EasingFunction:

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

@@ -2345,7 +2345,7 @@ RefPtr<StyleValue> Parser::parse_paint_value(TokenStream<ComponentValue>& tokens
 }
 
 // https://www.w3.org/TR/css-values-4/#position
-RefPtr<PositionStyleValue> Parser::parse_position_value(TokenStream<ComponentValue>& tokens)
+RefPtr<PositionStyleValue> Parser::parse_position_value(TokenStream<ComponentValue>& tokens, PositionParsingMode position_parsing_mode)
 {
     auto parse_position_edge = [](ComponentValue const& token) -> Optional<PositionEdge> {
         if (!token.is(Token::Type::Ident))
@@ -2542,9 +2542,83 @@ RefPtr<PositionStyleValue> Parser::parse_position_value(TokenStream<ComponentVal
         return nullptr;
     };
 
-    // Note: The alternatives must be attempted in this order since `alternative_2' can match a prefix of `alternative_3'
+    // The extra 3-value syntax that's allowed for background-position:
+    // [
+    //   [ [ left | right ] <length-percentage> ] &&
+    //   [ top | bottom ]
+    // |
+    //   [ left | right ] &&
+    //   [ [ top | bottom ] <length-percentage> ]
+    // ]
+    auto alternative_4_for_background_position = [&]() -> RefPtr<PositionStyleValue> {
+        auto transaction = tokens.begin_transaction();
+        tokens.skip_whitespace();
+        Optional<PositionEdge> horizontal_edge;
+        Optional<LengthPercentage> horizontal_offset;
+        Optional<PositionEdge> vertical_edge;
+        Optional<LengthPercentage> vertical_offset;
+
+        auto parse_horizontal = [&] {
+            // [ left | right ] <length-percentage> ]
+            auto transaction = tokens.begin_transaction();
+            tokens.skip_whitespace();
+            auto edge = parse_position_edge(tokens.next_token());
+            if (!edge.has_value() || !is_horizontal(*edge, false))
+                return false;
+            horizontal_edge = move(edge);
+
+            tokens.skip_whitespace();
+            auto length_percentage = parse_length_percentage(tokens.next_token());
+            if (length_percentage.has_value())
+                horizontal_offset = move(length_percentage);
+
+            transaction.commit();
+            return true;
+        };
+
+        auto parse_vertical = [&] {
+            // [ top | bottom ] <length-percentage> ]
+            auto transaction = tokens.begin_transaction();
+            tokens.skip_whitespace();
+            auto edge = parse_position_edge(tokens.next_token());
+            if (!edge.has_value() || !is_vertical(*edge, false))
+                return false;
+            vertical_edge = move(edge);
+
+            tokens.skip_whitespace();
+            auto length_percentage = parse_length_percentage(tokens.next_token());
+            if (length_percentage.has_value())
+                vertical_offset = move(length_percentage);
+
+            transaction.commit();
+            return true;
+        };
+
+        if (parse_horizontal() && parse_vertical()) {
+            transaction.commit();
+            return PositionStyleValue::create(EdgeStyleValue::create(*horizontal_edge, *horizontal_offset), EdgeStyleValue::create(*vertical_edge, *vertical_offset));
+        }
+
+        horizontal_edge.clear();
+        horizontal_offset.clear();
+        vertical_edge.clear();
+        vertical_offset.clear();
+
+        if (parse_vertical() && parse_horizontal()) {
+            transaction.commit();
+            return PositionStyleValue::create(EdgeStyleValue::create(*horizontal_edge, *horizontal_offset), EdgeStyleValue::create(*vertical_edge, *vertical_offset));
+        }
+
+        return nullptr;
+    };
+
+    // Note: The alternatives must be attempted in this order since shorter alternatives can match a prefix of longer ones.
     if (auto position = alternative_3())
         return position.release_nonnull();
+    if (position_parsing_mode == PositionParsingMode::BackgroundPosition) {
+        if (auto position = alternative_4_for_background_position())
+            return position.release_nonnull();
+    }
     if (auto position = alternative_2())
         return position;
     if (auto position = alternative_1())
@@ -2778,24 +2852,20 @@ RefPtr<StyleValue> Parser::parse_background_value(Vector<ComponentValue> const&
         }
         case PropertyID::BackgroundPosition: {
             VERIFY(!background_position);
-            tokens.reconsume_current_input_token();
+            background_position = value.release_nonnull();
-            if (auto maybe_background_position = parse_single_background_position_value(tokens)) {
-                background_position = maybe_background_position.release_nonnull();
 
-                // Attempt to parse `/ <background-size>`
+            // Attempt to parse `/ <background-size>`
-                auto transaction = tokens.begin_transaction();
+            auto transaction = tokens.begin_transaction();
-                auto& maybe_slash = tokens.next_token();
+            auto& maybe_slash = tokens.next_token();
-                if (maybe_slash.is_delim('/')) {
+            if (maybe_slash.is_delim('/')) {
-                    if (auto maybe_background_size = parse_single_background_size_value(tokens)) {
+                if (auto maybe_background_size = parse_single_background_size_value(tokens)) {
-                        transaction.commit();
+                    transaction.commit();
-                        background_size = maybe_background_size.release_nonnull();
+                    background_size = maybe_background_size.release_nonnull();
-                        continue;
+                    continue;
-                    }
-                    return nullptr;
                 }
-                continue;
+                return nullptr;
             }
-            return nullptr;
+            continue;
         }
         case PropertyID::BackgroundRepeat: {
             VERIFY(!background_repeat);
@@ -2876,149 +2946,6 @@ static Optional<LengthPercentage> style_value_to_length_percentage(auto value)
     return {};
 }
 
-RefPtr<StyleValue> Parser::parse_single_background_position_value(TokenStream<ComponentValue>& tokens)
-{
-    // NOTE: This *looks* like it parses a <position>, but it doesn't. From the spec:
-    //      "Note: The background-position property also accepts a three-value syntax.
-    //       This has been disallowed generically because it creates parsing ambiguities
-    //       when combined with other length or percentage components in a property value."
-    //           - https://www.w3.org/TR/css-values-4/#typedef-position
-    //       So, we'll need a separate function to parse <position> later.
-
-    auto transaction = tokens.begin_transaction();
-
-    auto is_horizontal = [](ValueID identifier) -> bool {
-        switch (identifier) {
-        case ValueID::Left:
-        case ValueID::Right:
-            return true;
-        default:
-            return false;
-        }
-    };
-    auto is_vertical = [](ValueID identifier) -> bool {
-        switch (identifier) {
-        case ValueID::Top:
-        case ValueID::Bottom:
-            return true;
-        default:
-            return false;
-        }
-    };
-
-    struct EdgeOffset {
-        PositionEdge edge;
-        LengthPercentage offset;
-        bool edge_provided;
-        bool offset_provided;
-    };
-
-    Optional<EdgeOffset> horizontal;
-    Optional<EdgeOffset> vertical;
-    bool found_center = false;
-
-    auto const center_offset = Percentage { 50 };
-    auto const zero_offset = Length::make_px(0);
-
-    while (tokens.has_next_token()) {
-        // Check if we're done
-        auto seen_items = (horizontal.has_value() ? 1 : 0) + (vertical.has_value() ? 1 : 0) + (found_center ? 1 : 0);
-        if (seen_items == 2)
-            break;
-
-        auto maybe_value = parse_css_value_for_property(PropertyID::BackgroundPosition, tokens);
-        if (!maybe_value)
-            break;
-        auto value = maybe_value.release_nonnull();
-
-        if (auto offset = style_value_to_length_percentage(value); offset.has_value()) {
-            if (!horizontal.has_value()) {
-                horizontal = EdgeOffset { PositionEdge::Left, *offset, false, true };
-            } else if (!vertical.has_value()) {
-                vertical = EdgeOffset { PositionEdge::Top, *offset, false, true };
-            } else {
-                return nullptr;
-            }
-            continue;
-        }
-
-        auto try_parse_offset = [&](bool& offset_provided) -> LengthPercentage {
-            auto transaction = tokens.begin_transaction();
-            if (tokens.has_next_token()) {
-                auto maybe_value = parse_css_value_for_property(PropertyID::BackgroundPosition, tokens);
-                if (!maybe_value)
-                    return zero_offset;
-                auto offset = style_value_to_length_percentage(maybe_value.release_nonnull());
-                if (offset.has_value()) {
-                    offset_provided = true;
-                    transaction.commit();
-                    return *offset;
-                }
-            }
-            return zero_offset;
-        };
-
-        if (value->is_identifier()) {
-            auto identifier = value->to_identifier();
-            if (is_horizontal(identifier)) {
-                bool offset_provided = false;
-                auto offset = try_parse_offset(offset_provided);
-                horizontal = EdgeOffset { *value_id_to_position_edge(identifier), offset, true, offset_provided };
-            } else if (is_vertical(identifier)) {
-                bool offset_provided = false;
-                auto offset = try_parse_offset(offset_provided);
-                vertical = EdgeOffset { *value_id_to_position_edge(identifier), offset, true, offset_provided };
-            } else if (identifier == ValueID::Center) {
-                found_center = true;
-            } else {
-                return nullptr;
-            }
-            continue;
-        }
-
-        tokens.reconsume_current_input_token();
-        break;
-    }
-
-    if (found_center) {
-        if (horizontal.has_value() && vertical.has_value())
-            return nullptr;
-        if (!horizontal.has_value())
-            horizontal = EdgeOffset { PositionEdge::Left, center_offset, true, false };
-        if (!vertical.has_value())
-            vertical = EdgeOffset { PositionEdge::Top, center_offset, true, false };
-    }
-
-    if (!horizontal.has_value() && !vertical.has_value())
-        return nullptr;
-
-    // Unpack `<edge> <length>`:
-    // The loop above reads this pattern as a single EdgeOffset, when actually, it should be treated
-    // as `x y` if the edge is horizontal, and `y` (with the second token reconsumed) otherwise.
-    if (!vertical.has_value() && horizontal->edge_provided && horizontal->offset_provided) {
-        // Split into `x y`
-        vertical = EdgeOffset { PositionEdge::Top, horizontal->offset, false, true };
-        horizontal->offset = zero_offset;
-        horizontal->offset_provided = false;
-    } else if (!horizontal.has_value() && vertical->edge_provided && vertical->offset_provided) {
-        // `y`, reconsume
-        vertical->offset = zero_offset;
-        vertical->offset_provided = false;
-        tokens.reconsume_current_input_token();
-    }
-
-    // If only one value is specified, the second value is assumed to be center.
-    if (!horizontal.has_value())
-        horizontal = EdgeOffset { PositionEdge::Left, center_offset, false, false };
-    if (!vertical.has_value())
-        vertical = EdgeOffset { PositionEdge::Top, center_offset, false, false };
-
-    transaction.commit();
-    return PositionStyleValue::create(
-        EdgeStyleValue::create(horizontal->edge, horizontal->offset),
-        EdgeStyleValue::create(vertical->edge, vertical->offset));
-}
-
 RefPtr<StyleValue> Parser::parse_single_background_position_x_or_y_value(TokenStream<ComponentValue>& tokens, PropertyID property)
 {
     PositionEdge relative_edge {};
@@ -5797,7 +5724,7 @@ Parser::ParseErrorOr<NonnullRefPtr<StyleValue>> Parser::parse_css_value(Property
             return parsed_value.release_nonnull();
         return ParseError::SyntaxError;
     case PropertyID::BackgroundPosition:
-        if (auto parsed_value = parse_comma_separated_value_list(component_values, [this](auto& tokens) { return parse_single_background_position_value(tokens); }))
+        if (auto parsed_value = parse_comma_separated_value_list(component_values, [this](auto& tokens) { return parse_position_value(tokens, PositionParsingMode::BackgroundPosition); }))
             return parsed_value.release_nonnull();
         return ParseError::SyntaxError;
     case PropertyID::BackgroundPositionX:
@@ -6129,6 +6056,11 @@ Optional<Parser::PropertyAndValue> Parser::parse_css_value_for_properties(Readon
             return PropertyAndValue { *property, maybe_position };
     }
 
+    if (auto property = any_property_accepts_type(property_ids, ValueType::BackgroundPosition); property.has_value()) {
+        if (auto maybe_position = parse_position_value(tokens, PositionParsingMode::BackgroundPosition))
+            return PropertyAndValue { *property, maybe_position };
+    }
+
     if (auto property = any_property_accepts_type(property_ids, ValueType::Ratio); property.has_value()) {
         if (auto maybe_ratio = parse_ratio_value(tokens))
             return PropertyAndValue { *property, maybe_ratio };

Userland/Libraries/LibWeb/CSS/Parser/Parser.h

@@ -215,7 +215,11 @@ private:
     RefPtr<StyleValue> parse_string_value(ComponentValue const&);
     RefPtr<StyleValue> parse_image_value(ComponentValue const&);
     RefPtr<StyleValue> parse_paint_value(TokenStream<ComponentValue>&);
-    RefPtr<PositionStyleValue> parse_position_value(TokenStream<ComponentValue>&);
+    enum class PositionParsingMode {
+        Normal,
+        BackgroundPosition,
+    };
+    RefPtr<PositionStyleValue> parse_position_value(TokenStream<ComponentValue>&, PositionParsingMode = PositionParsingMode::Normal);
     template<typename ParseFunction>
     RefPtr<StyleValue> parse_comma_separated_value_list(Vector<ComponentValue> const&, ParseFunction);
     RefPtr<StyleValue> parse_simple_comma_separated_value_list(PropertyID, Vector<ComponentValue> const&);
@@ -223,7 +227,6 @@ private:
     RefPtr<StyleValue> parse_filter_value_list_value(Vector<ComponentValue> const&);
     RefPtr<StyleValue> parse_aspect_ratio_value(Vector<ComponentValue> const&);
     RefPtr<StyleValue> parse_background_value(Vector<ComponentValue> const&);
-    RefPtr<StyleValue> parse_single_background_position_value(TokenStream<ComponentValue>&);
     RefPtr<StyleValue> parse_single_background_position_x_or_y_value(TokenStream<ComponentValue>&, PropertyID);
     RefPtr<StyleValue> parse_single_background_repeat_value(TokenStream<ComponentValue>&);
     RefPtr<StyleValue> parse_single_background_size_value(TokenStream<ComponentValue>&);

Userland/Libraries/LibWeb/CSS/Properties.json

@@ -221,17 +221,8 @@
     "initial": "0% 0%",
     "max-values": 4,
     "valid-types": [
-      "length [-∞,∞]",
+      "background-position"
-      "percentage [-∞,∞]"
     ],
-    "valid-identifiers": [
-      "bottom",
-      "center",
-      "left",
-      "right",
-      "top"
-    ],
-    "percentages-resolve-to": "length",
     "quirks": [
       "unitless-length"
     ],