From 148f873321c59a5251b8f8aaadd72f86d69c7df7 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 20 Nov 2023 15:33:21 +0000 Subject: [PATCH] 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. :^) --- .../LibWeb/GenerateCSSPropertyID.cpp | 5 +- .../Libraries/LibWeb/CSS/CSSNumericType.cpp | 1 + .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 254 +++++++----------- Userland/Libraries/LibWeb/CSS/Parser/Parser.h | 7 +- Userland/Libraries/LibWeb/CSS/Properties.json | 11 +- 5 files changed, 104 insertions(+), 174 deletions(-) diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp index f2c5a9abe8..33ee4bd96a 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp +++ b/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 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") { diff --git a/Userland/Libraries/LibWeb/CSS/CSSNumericType.cpp b/Userland/Libraries/LibWeb/CSS/CSSNumericType.cpp index 677627b162..7624525703 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSNumericType.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSNumericType.cpp @@ -33,6 +33,7 @@ Optional 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: diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 82e0c65030..f3bbcfbbe9 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -2345,7 +2345,7 @@ RefPtr Parser::parse_paint_value(TokenStream& tokens } // https://www.w3.org/TR/css-values-4/#position -RefPtr Parser::parse_position_value(TokenStream& tokens) +RefPtr Parser::parse_position_value(TokenStream& tokens, PositionParsingMode position_parsing_mode) { auto parse_position_edge = [](ComponentValue const& token) -> Optional { if (!token.is(Token::Type::Ident)) @@ -2542,9 +2542,83 @@ RefPtr Parser::parse_position_value(TokenStream ] && + // [ top | bottom ] + // | + // [ left | right ] && + // [ [ top | bottom ] ] + // ] + auto alternative_4_for_background_position = [&]() -> RefPtr { + auto transaction = tokens.begin_transaction(); + tokens.skip_whitespace(); + Optional horizontal_edge; + Optional horizontal_offset; + Optional vertical_edge; + Optional vertical_offset; + + auto parse_horizontal = [&] { + // [ left | right ] ] + 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 ] ] + 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 Parser::parse_background_value(Vector const& } case PropertyID::BackgroundPosition: { VERIFY(!background_position); - tokens.reconsume_current_input_token(); - if (auto maybe_background_position = parse_single_background_position_value(tokens)) { - background_position = maybe_background_position.release_nonnull(); + background_position = value.release_nonnull(); - // Attempt to parse `/ ` - auto transaction = tokens.begin_transaction(); - auto& maybe_slash = tokens.next_token(); - if (maybe_slash.is_delim('/')) { - if (auto maybe_background_size = parse_single_background_size_value(tokens)) { - transaction.commit(); - background_size = maybe_background_size.release_nonnull(); - continue; - } - return nullptr; + // Attempt to parse `/ ` + auto transaction = tokens.begin_transaction(); + auto& maybe_slash = tokens.next_token(); + if (maybe_slash.is_delim('/')) { + if (auto maybe_background_size = parse_single_background_size_value(tokens)) { + transaction.commit(); + background_size = maybe_background_size.release_nonnull(); + continue; } - continue; + return nullptr; } - return nullptr; + continue; } case PropertyID::BackgroundRepeat: { VERIFY(!background_repeat); @@ -2876,149 +2946,6 @@ static Optional style_value_to_length_percentage(auto value) return {}; } -RefPtr Parser::parse_single_background_position_value(TokenStream& tokens) -{ - // NOTE: This *looks* like it parses a , 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 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 horizontal; - Optional 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 ` `: - // 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 Parser::parse_single_background_position_x_or_y_value(TokenStream& tokens, PropertyID property) { PositionEdge relative_edge {}; @@ -5797,7 +5724,7 @@ Parser::ParseErrorOr> 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::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 }; diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h index f81fc619b1..a6e32e78c9 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h @@ -215,7 +215,11 @@ private: RefPtr parse_string_value(ComponentValue const&); RefPtr parse_image_value(ComponentValue const&); RefPtr parse_paint_value(TokenStream&); - RefPtr parse_position_value(TokenStream&); + enum class PositionParsingMode { + Normal, + BackgroundPosition, + }; + RefPtr parse_position_value(TokenStream&, PositionParsingMode = PositionParsingMode::Normal); template RefPtr parse_comma_separated_value_list(Vector const&, ParseFunction); RefPtr parse_simple_comma_separated_value_list(PropertyID, Vector const&); @@ -223,7 +227,6 @@ private: RefPtr parse_filter_value_list_value(Vector const&); RefPtr parse_aspect_ratio_value(Vector const&); RefPtr parse_background_value(Vector const&); - RefPtr parse_single_background_position_value(TokenStream&); RefPtr parse_single_background_position_x_or_y_value(TokenStream&, PropertyID); RefPtr parse_single_background_repeat_value(TokenStream&); RefPtr parse_single_background_size_value(TokenStream&); diff --git a/Userland/Libraries/LibWeb/CSS/Properties.json b/Userland/Libraries/LibWeb/CSS/Properties.json index 37d0e70ff2..1f24d8937c 100644 --- a/Userland/Libraries/LibWeb/CSS/Properties.json +++ b/Userland/Libraries/LibWeb/CSS/Properties.json @@ -221,17 +221,8 @@ "initial": "0% 0%", "max-values": 4, "valid-types": [ - "length [-∞,∞]", - "percentage [-∞,∞]" + "background-position" ], - "valid-identifiers": [ - "bottom", - "center", - "left", - "right", - "top" - ], - "percentages-resolve-to": "length", "quirks": [ "unitless-length" ],