From a7e5ded188f101e7f63a7886ffc7e6d8a9580644 Mon Sep 17 00:00:00 2001 From: Callum Law Date: Tue, 9 Sep 2025 20:47:11 +1200 Subject: [PATCH] LibWeb: Add generic logic for parsing "positional-value-list-shorthands" Continues the work started in #5386 to simplify handling of positional value list shorthand properties. Previously we would parse these as `StyleValueList`s and then rely on `StyleComputer::for_each_property_expanding_shorthands` to expand them into longhands. This required a bit of work to handle `ShorthandStyleValue`s for the `@page` `margin` descriptor. --- Libraries/LibWeb/CSS/CSSDescriptors.cpp | 36 +---- .../LibWeb/CSS/Parser/DescriptorParsing.cpp | 3 +- Libraries/LibWeb/CSS/Parser/Parser.h | 1 + .../LibWeb/CSS/Parser/PropertyParsing.cpp | 49 ++++++ Libraries/LibWeb/CSS/Properties.json | 20 --- Libraries/LibWeb/CSS/StyleComputer.cpp | 147 ------------------ 6 files changed, 57 insertions(+), 199 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSDescriptors.cpp b/Libraries/LibWeb/CSS/CSSDescriptors.cpp index 5f7407d82e7..2e5e70b08b7 100644 --- a/Libraries/LibWeb/CSS/CSSDescriptors.cpp +++ b/Libraries/LibWeb/CSS/CSSDescriptors.cpp @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include @@ -316,36 +316,12 @@ void for_each_expanded_longhand(AtRuleID at_rule, DescriptorID descriptor, RefPt return; } - if (value->is_value_list()) { - auto& values = value->as_value_list().values(); - if (values.size() == 4) { - callback(DescriptorID::MarginTop, values[0]); - callback(DescriptorID::MarginRight, values[1]); - callback(DescriptorID::MarginBottom, values[2]); - callback(DescriptorID::MarginLeft, values[3]); - } else if (values.size() == 3) { - callback(DescriptorID::MarginTop, values[0]); - callback(DescriptorID::MarginRight, values[1]); - callback(DescriptorID::MarginBottom, values[2]); - callback(DescriptorID::MarginLeft, values[1]); - } else if (values.size() == 2) { - callback(DescriptorID::MarginTop, values[0]); - callback(DescriptorID::MarginRight, values[1]); - callback(DescriptorID::MarginBottom, values[0]); - callback(DescriptorID::MarginLeft, values[1]); - } else if (values.size() == 1) { - callback(DescriptorID::MarginTop, values[0]); - callback(DescriptorID::MarginRight, values[0]); - callback(DescriptorID::MarginBottom, values[0]); - callback(DescriptorID::MarginLeft, values[0]); - } + auto const& shorthand_value = value->as_shorthand(); - } else { - callback(DescriptorID::MarginTop, *value); - callback(DescriptorID::MarginRight, *value); - callback(DescriptorID::MarginBottom, *value); - callback(DescriptorID::MarginLeft, *value); - } + callback(DescriptorID::MarginTop, shorthand_value.longhand(PropertyID::MarginTop)); + callback(DescriptorID::MarginRight, shorthand_value.longhand(PropertyID::MarginRight)); + callback(DescriptorID::MarginBottom, shorthand_value.longhand(PropertyID::MarginBottom)); + callback(DescriptorID::MarginLeft, shorthand_value.longhand(PropertyID::MarginLeft)); } } diff --git a/Libraries/LibWeb/CSS/Parser/DescriptorParsing.cpp b/Libraries/LibWeb/CSS/Parser/DescriptorParsing.cpp index 65834fa4daa..2e6decae810 100644 --- a/Libraries/LibWeb/CSS/Parser/DescriptorParsing.cpp +++ b/Libraries/LibWeb/CSS/Parser/DescriptorParsing.cpp @@ -53,9 +53,8 @@ Parser::ParseErrorOr> Parser::parse_descriptor_v auto value_for_property = value_or_error.release_value(); // Descriptors don't accept the following, which properties do: // - CSS-wide keywords - // - Shorthands // - Arbitrary substitution functions (so, UnresolvedStyleValue) - if (value_for_property->is_css_wide_keyword() || value_for_property->is_shorthand() || value_for_property->is_unresolved()) + if (value_for_property->is_css_wide_keyword() || value_for_property->is_unresolved()) return nullptr; return value_for_property; }, diff --git a/Libraries/LibWeb/CSS/Parser/Parser.h b/Libraries/LibWeb/CSS/Parser/Parser.h index 4f2148f7a0a..5541f8d71b2 100644 --- a/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Libraries/LibWeb/CSS/Parser/Parser.h @@ -345,6 +345,7 @@ private: ParseErrorOr> parse_css_value(PropertyID, TokenStream&, Optional original_source_text = {}); ParseErrorOr> parse_descriptor_value(AtRuleID, DescriptorID, TokenStream&); + RefPtr parse_positional_value_list_shorthand(PropertyID, TokenStream&); RefPtr parse_css_value_for_property(PropertyID, TokenStream&); struct PropertyAndValue { PropertyID property; diff --git a/Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp b/Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp index 09e2e1bd792..b13ed8d2891 100644 --- a/Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp +++ b/Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp @@ -810,6 +810,13 @@ Parser::ParseErrorOr> Parser::parse_css_value(Pr break; } + if (property_is_positional_value_list_shorthand(property_id)) { + if (auto parsed_value = parse_positional_value_list_shorthand(property_id, tokens); parsed_value && !tokens.has_next_token()) + return parsed_value.release_nonnull(); + + return ParseError::SyntaxError; + } + // If there's only 1 ComponentValue, we can only produce a single StyleValue. if (component_values.size() == 1) { auto stream = TokenStream { component_values }; @@ -891,6 +898,48 @@ Parser::ParseErrorOr> Parser::parse_css_value(Pr return { ShorthandStyleValue::create(property_id, move(longhand_properties), move(longhand_values)) }; } +RefPtr Parser::parse_positional_value_list_shorthand(PropertyID property_id, TokenStream& tokens) +{ + auto const& longhands = longhands_for_shorthand(property_id); + + Vector> parsed_values; + + while (auto parsed_value = parse_css_value_for_property(property_id, tokens)) + parsed_values.append(parsed_value.release_nonnull()); + + if (parsed_values.size() == 0 || parsed_values.size() > longhands.size()) + return nullptr; + + switch (longhands.size()) { + case 2: { + switch (parsed_values.size()) { + case 1: + return ShorthandStyleValue::create(property_id, longhands, { parsed_values[0], parsed_values[0] }); + case 2: + return ShorthandStyleValue::create(property_id, longhands, parsed_values); + default: + VERIFY_NOT_REACHED(); + } + } + case 4: { + switch (parsed_values.size()) { + case 1: + return ShorthandStyleValue::create(property_id, longhands, { parsed_values[0], parsed_values[0], parsed_values[0], parsed_values[0] }); + case 2: + return ShorthandStyleValue::create(property_id, longhands, { parsed_values[0], parsed_values[1], parsed_values[0], parsed_values[1] }); + case 3: + return ShorthandStyleValue::create(property_id, longhands, { parsed_values[0], parsed_values[1], parsed_values[2], parsed_values[1] }); + case 4: + return ShorthandStyleValue::create(property_id, longhands, parsed_values); + default: + VERIFY_NOT_REACHED(); + } + } + default: + TODO(); + } +} + RefPtr Parser::parse_color_scheme_value(TokenStream& tokens) { // normal | [ light | dark | ]+ && only? diff --git a/Libraries/LibWeb/CSS/Properties.json b/Libraries/LibWeb/CSS/Properties.json index abceadee2f0..9223c7778cf 100644 --- a/Libraries/LibWeb/CSS/Properties.json +++ b/Libraries/LibWeb/CSS/Properties.json @@ -517,7 +517,6 @@ "border-block-start-color", "border-block-end-color" ], - "max-values": 2, "valid-types": [ "color" ] @@ -590,7 +589,6 @@ "border-block-start-style", "border-block-end-style" ], - "max-values": 2, "valid-types": [ "line-style" ] @@ -603,7 +601,6 @@ "border-block-start-width", "border-block-end-width" ], - "max-values": 2, "valid-types": [ "length [0,∞]", "line-width" @@ -692,7 +689,6 @@ "border-bottom-color", "border-left-color" ], - "max-values": 4, "valid-types": [ "color" ], @@ -803,7 +799,6 @@ "border-inline-start-color", "border-inline-end-color" ], - "max-values": 2, "valid-types": [ "color" ] @@ -876,7 +871,6 @@ "border-inline-start-style", "border-inline-end-style" ], - "max-values": 2, "valid-types": [ "line-style" ] @@ -889,7 +883,6 @@ "border-inline-start-width", "border-inline-end-width" ], - "max-values": 2, "valid-types": [ "length [0,∞]", "line-width" @@ -1029,7 +1022,6 @@ "border-bottom-style", "border-left-style" ], - "max-values": 4, "valid-types": [ "line-style" ] @@ -1108,7 +1100,6 @@ "border-bottom-width", "border-left-width" ], - "max-values": 4, "valid-types": [ "length [0,∞]", "line-width" @@ -1800,7 +1791,6 @@ "length [0,∞]", "percentage [0,∞]" ], - "max-values": 2, "valid-identifiers": [ "normal" ], @@ -2064,7 +2054,6 @@ "bottom", "left" ], - "max-values": 4, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2081,7 +2070,6 @@ "inset-block-start", "inset-block-end" ], - "max-values": 2, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2112,7 +2100,6 @@ "inset-inline-start", "inset-inline-end" ], - "max-values": 2, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2262,7 +2249,6 @@ "margin-bottom", "margin-left" ], - "max-values": 4, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2282,7 +2268,6 @@ "margin-block-start", "margin-block-end" ], - "max-values": 2, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2327,7 +2312,6 @@ "margin-inline-start", "margin-inline-end" ], - "max-values": 2, "valid-types": [ "length [-∞,∞]", "percentage [-∞,∞]" @@ -2750,7 +2734,6 @@ "positional-value-list-shorthand": true, "inherited": false, "initial": "visible", - "max-values": 2, "valid-types": [ "overflow" ] @@ -2787,7 +2770,6 @@ "padding-bottom", "padding-left" ], - "max-values": 4, "valid-types": [ "length [0,∞]", "percentage [0,∞]" @@ -2804,7 +2786,6 @@ "padding-block-start", "padding-block-end" ], - "max-values": 2, "valid-types": [ "length [0,∞]", "percentage [0,∞]" @@ -2843,7 +2824,6 @@ "padding-inline-start", "padding-inline-end" ], - "max-values": 2, "valid-types": [ "length [0,∞]", "percentage [0,∞]" diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index 2fd558c01eb..94207111ad4 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -675,96 +675,6 @@ void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_i return; } - // FIXME: We should add logic in parse_css_value to parse "positional-value-list-shorthand"s as - // ShorthandStyleValues to avoid the need for this (and assign_start_and_end_values). - auto assign_edge_values = [&](PropertyID top_property, PropertyID right_property, PropertyID bottom_property, PropertyID left_property, StyleValue const& value) { - if (value.is_value_list()) { - auto values = value.as_value_list().values(); - - if (values.size() == 4) { - set_longhand_property(top_property, values[0]); - set_longhand_property(right_property, values[1]); - set_longhand_property(bottom_property, values[2]); - set_longhand_property(left_property, values[3]); - } else if (values.size() == 3) { - set_longhand_property(top_property, values[0]); - set_longhand_property(right_property, values[1]); - set_longhand_property(bottom_property, values[2]); - set_longhand_property(left_property, values[1]); - } else if (values.size() == 2) { - set_longhand_property(top_property, values[0]); - set_longhand_property(right_property, values[1]); - set_longhand_property(bottom_property, values[0]); - set_longhand_property(left_property, values[1]); - } else if (values.size() == 1) { - set_longhand_property(top_property, values[0]); - set_longhand_property(right_property, values[0]); - set_longhand_property(bottom_property, values[0]); - set_longhand_property(left_property, values[0]); - } - } else { - set_longhand_property(top_property, value); - set_longhand_property(right_property, value); - set_longhand_property(bottom_property, value); - set_longhand_property(left_property, value); - } - }; - - auto assign_start_and_end_values = [&](PropertyID start_property, PropertyID end_property, auto const& values) { - if (values.is_value_list()) { - set_longhand_property(start_property, value.as_value_list().values()[0]); - set_longhand_property(end_property, value.as_value_list().values()[1]); - } else { - set_longhand_property(start_property, value); - set_longhand_property(end_property, value); - } - }; - - if (property_id == CSS::PropertyID::BorderStyle) { - assign_edge_values(PropertyID::BorderTopStyle, PropertyID::BorderRightStyle, PropertyID::BorderBottomStyle, PropertyID::BorderLeftStyle, value); - return; - } - - if (property_id == CSS::PropertyID::BorderBlockStyle) { - assign_start_and_end_values(PropertyID::BorderBlockStartStyle, PropertyID::BorderBlockEndStyle, value); - return; - } - - if (property_id == CSS::PropertyID::BorderInlineStyle) { - assign_start_and_end_values(PropertyID::BorderInlineStartStyle, PropertyID::BorderInlineEndStyle, value); - return; - } - - if (property_id == CSS::PropertyID::BorderWidth) { - assign_edge_values(PropertyID::BorderTopWidth, PropertyID::BorderRightWidth, PropertyID::BorderBottomWidth, PropertyID::BorderLeftWidth, value); - return; - } - - if (property_id == CSS::PropertyID::BorderBlockWidth) { - assign_start_and_end_values(PropertyID::BorderBlockStartWidth, PropertyID::BorderBlockEndWidth, value); - return; - } - - if (property_id == CSS::PropertyID::BorderInlineWidth) { - assign_start_and_end_values(PropertyID::BorderInlineStartWidth, PropertyID::BorderInlineEndWidth, value); - return; - } - - if (property_id == CSS::PropertyID::BorderColor) { - assign_edge_values(PropertyID::BorderTopColor, PropertyID::BorderRightColor, PropertyID::BorderBottomColor, PropertyID::BorderLeftColor, value); - return; - } - - if (property_id == CSS::PropertyID::BorderBlockColor) { - assign_start_and_end_values(PropertyID::BorderBlockStartColor, PropertyID::BorderBlockEndColor, value); - return; - } - - if (property_id == CSS::PropertyID::BorderInlineColor) { - assign_start_and_end_values(PropertyID::BorderInlineStartColor, PropertyID::BorderInlineEndColor, value); - return; - } - if (property_id == CSS::PropertyID::BackgroundPosition) { if (value.is_position()) { auto const& position = value.as_position(); @@ -797,63 +707,6 @@ void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_i return; } - if (property_id == CSS::PropertyID::Inset) { - assign_edge_values(PropertyID::Top, PropertyID::Right, PropertyID::Bottom, PropertyID::Left, value); - return; - } - - if (property_id == CSS::PropertyID::InsetBlock) { - assign_start_and_end_values(PropertyID::InsetBlockStart, PropertyID::InsetBlockEnd, value); - return; - } - - if (property_id == CSS::PropertyID::InsetInline) { - assign_start_and_end_values(PropertyID::InsetInlineStart, PropertyID::InsetInlineEnd, value); - return; - } - - if (property_id == CSS::PropertyID::Margin) { - assign_edge_values(PropertyID::MarginTop, PropertyID::MarginRight, PropertyID::MarginBottom, PropertyID::MarginLeft, value); - return; - } - - if (property_id == CSS::PropertyID::MarginBlock) { - assign_start_and_end_values(PropertyID::MarginBlockStart, PropertyID::MarginBlockEnd, value); - return; - } - - if (property_id == CSS::PropertyID::MarginInline) { - assign_start_and_end_values(PropertyID::MarginInlineStart, PropertyID::MarginInlineEnd, value); - return; - } - - if (property_id == CSS::PropertyID::Padding) { - assign_edge_values(PropertyID::PaddingTop, PropertyID::PaddingRight, PropertyID::PaddingBottom, PropertyID::PaddingLeft, value); - return; - } - - if (property_id == CSS::PropertyID::PaddingBlock) { - assign_start_and_end_values(PropertyID::PaddingBlockStart, PropertyID::PaddingBlockEnd, value); - return; - } - - if (property_id == CSS::PropertyID::PaddingInline) { - assign_start_and_end_values(PropertyID::PaddingInlineStart, PropertyID::PaddingInlineEnd, value); - return; - } - - if (property_id == CSS::PropertyID::Gap) { - if (value.is_value_list()) { - auto const& values_list = value.as_value_list(); - set_longhand_property(CSS::PropertyID::RowGap, values_list.values()[0]); - set_longhand_property(CSS::PropertyID::ColumnGap, values_list.values()[1]); - return; - } - set_longhand_property(CSS::PropertyID::RowGap, value); - set_longhand_property(CSS::PropertyID::ColumnGap, value); - return; - } - if (property_id == CSS::PropertyID::Transition) { if (value.to_keyword() == Keyword::None) { // Handle `none` as a shorthand for `all 0s ease 0s`.