From 23d59a6caf1e3146d7684c7ae75f83ebdcf5f1d0 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 20 Sep 2023 12:50:48 +0100 Subject: [PATCH] LibWeb: Replace BorderStyleValue with ShorthandStyleValue And also expand builtin values to the longhands, which we weren't doing before. --- .../Libraries/LibWeb/CSS/StyleValues/BUILD.gn | 1 - Userland/Libraries/LibWeb/CMakeLists.txt | 1 - .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 66 ++++++++++----- Userland/Libraries/LibWeb/CSS/Parser/Parser.h | 2 +- .../CSS/ResolvedCSSStyleDeclaration.cpp | 21 +++-- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 80 +++++-------------- Userland/Libraries/LibWeb/CSS/StyleValue.cpp | 1 - Userland/Libraries/LibWeb/CSS/StyleValue.h | 1 - .../CSS/StyleValues/BorderStyleValue.cpp | 30 ------- .../LibWeb/CSS/StyleValues/BorderStyleValue.h | 49 ------------ Userland/Libraries/LibWeb/Forward.h | 1 - 11 files changed, 85 insertions(+), 168 deletions(-) delete mode 100644 Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.cpp delete mode 100644 Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.h diff --git a/Meta/gn/secondary/Userland/Libraries/LibWeb/CSS/StyleValues/BUILD.gn b/Meta/gn/secondary/Userland/Libraries/LibWeb/CSS/StyleValues/BUILD.gn index d464f239f8..9481525de0 100644 --- a/Meta/gn/secondary/Userland/Libraries/LibWeb/CSS/StyleValues/BUILD.gn +++ b/Meta/gn/secondary/Userland/Libraries/LibWeb/CSS/StyleValues/BUILD.gn @@ -6,7 +6,6 @@ source_set("StyleValues") { "BackgroundRepeatStyleValue.cpp", "BackgroundSizeStyleValue.cpp", "BorderRadiusStyleValue.cpp", - "BorderStyleValue.cpp", "CalculatedStyleValue.cpp", "ColorStyleValue.cpp", "ConicGradientStyleValue.cpp", diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index 798f5e42c3..be2862581e 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -83,7 +83,6 @@ set(SOURCES CSS/StyleValues/BackgroundRepeatStyleValue.cpp CSS/StyleValues/BackgroundSizeStyleValue.cpp CSS/StyleValues/BorderRadiusStyleValue.cpp - CSS/StyleValues/BorderStyleValue.cpp CSS/StyleValues/CalculatedStyleValue.cpp CSS/StyleValues/ColorStyleValue.cpp CSS/StyleValues/ConicGradientStyleValue.cpp diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index effcdc5b6d..b2bcc5075c 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -38,7 +38,6 @@ #include #include #include -#include #include #include #include @@ -3207,7 +3206,7 @@ RefPtr Parser::parse_single_background_size_value(TokenStream Parser::parse_border_value(Vector const& component_values) +RefPtr Parser::parse_border_value(PropertyID property_id, Vector const& component_values) { if (component_values.size() > 3) return nullptr; @@ -3216,7 +3215,41 @@ RefPtr Parser::parse_border_value(Vector const& comp RefPtr border_color; RefPtr border_style; - auto remaining_longhands = Vector { PropertyID::BorderWidth, PropertyID::BorderColor, PropertyID::BorderStyle }; + auto color_property = PropertyID::Invalid; + auto style_property = PropertyID::Invalid; + auto width_property = PropertyID::Invalid; + + switch (property_id) { + case PropertyID::Border: + color_property = PropertyID::BorderColor; + style_property = PropertyID::BorderStyle; + width_property = PropertyID::BorderWidth; + break; + case PropertyID::BorderBottom: + color_property = PropertyID::BorderBottomColor; + style_property = PropertyID::BorderBottomStyle; + width_property = PropertyID::BorderBottomWidth; + break; + case PropertyID::BorderLeft: + color_property = PropertyID::BorderLeftColor; + style_property = PropertyID::BorderLeftStyle; + width_property = PropertyID::BorderLeftWidth; + break; + case PropertyID::BorderRight: + color_property = PropertyID::BorderRightColor; + style_property = PropertyID::BorderRightStyle; + width_property = PropertyID::BorderRightWidth; + break; + case PropertyID::BorderTop: + color_property = PropertyID::BorderTopColor; + style_property = PropertyID::BorderTopStyle; + width_property = PropertyID::BorderTopWidth; + break; + default: + VERIFY_NOT_REACHED(); + } + + auto remaining_longhands = Vector { width_property, color_property, style_property }; auto tokens = TokenStream { component_values }; while (tokens.has_next_token()) { @@ -3226,35 +3259,30 @@ RefPtr Parser::parse_border_value(Vector const& comp auto& value = property_and_value->style_value; remove_property(remaining_longhands, property_and_value->property); - switch (property_and_value->property) { - case PropertyID::BorderWidth: { + if (property_and_value->property == width_property) { VERIFY(!border_width); border_width = value.release_nonnull(); - continue; - } - case PropertyID::BorderColor: { + } else if (property_and_value->property == color_property) { VERIFY(!border_color); border_color = value.release_nonnull(); - continue; - } - case PropertyID::BorderStyle: { + } else if (property_and_value->property == style_property) { VERIFY(!border_style); border_style = value.release_nonnull(); - continue; - } - default: + } else { VERIFY_NOT_REACHED(); } } if (!border_width) - border_width = property_initial_value(m_context.realm(), PropertyID::BorderWidth); + border_width = property_initial_value(m_context.realm(), width_property); if (!border_style) - border_style = property_initial_value(m_context.realm(), PropertyID::BorderStyle); + border_style = property_initial_value(m_context.realm(), style_property); if (!border_color) - border_color = property_initial_value(m_context.realm(), PropertyID::BorderColor); + border_color = property_initial_value(m_context.realm(), color_property); - return BorderStyleValue::create(border_width.release_nonnull(), border_style.release_nonnull(), border_color.release_nonnull()); + return ShorthandStyleValue::create(property_id, + { width_property, style_property, color_property }, + { border_width.release_nonnull(), border_style.release_nonnull(), border_color.release_nonnull() }); } RefPtr Parser::parse_border_radius_value(Vector const& component_values) @@ -5785,7 +5813,7 @@ Parser::ParseErrorOr> Parser::parse_css_value(Property case PropertyID::BorderLeft: case PropertyID::BorderRight: case PropertyID::BorderTop: - if (auto parsed_value = parse_border_value(component_values)) + if (auto parsed_value = parse_border_value(property_id, component_values)) return parsed_value.release_nonnull(); return ParseError::SyntaxError; case PropertyID::BorderTopLeftRadius: diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h index 2c6c866d4c..7771abf279 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h @@ -228,7 +228,7 @@ private: 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&); - RefPtr parse_border_value(Vector const&); + RefPtr parse_border_value(PropertyID, Vector const&); RefPtr parse_border_radius_value(Vector const&); RefPtr parse_border_radius_shorthand_value(Vector const&); RefPtr parse_content_value(Vector const&); diff --git a/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp b/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp index 2843a4644f..f83fe787c5 100644 --- a/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp +++ b/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -193,14 +192,18 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_property(L auto width = LengthStyleValue::create(Length::make_px(top.width)); auto style = IdentifierStyleValue::create(to_value_id(top.line_style)); auto color = ColorStyleValue::create(top.color); - return BorderStyleValue::create(width, style, color); + return ShorthandStyleValue::create(property_id, + { PropertyID::BorderWidth, PropertyID::BorderStyle, PropertyID::BorderColor }, + { width, style, color }); } case PropertyID::BorderBottom: { auto border = layout_node.computed_values().border_bottom(); auto width = LengthStyleValue::create(Length::make_px(border.width)); auto style = IdentifierStyleValue::create(to_value_id(border.line_style)); auto color = ColorStyleValue::create(border.color); - return BorderStyleValue::create(width, style, color); + return ShorthandStyleValue::create(property_id, + { PropertyID::BorderBottomWidth, PropertyID::BorderBottomStyle, PropertyID::BorderBottomColor }, + { width, style, color }); } case PropertyID::BorderBottomColor: return ColorStyleValue::create(layout_node.computed_values().border_bottom().color); @@ -216,7 +219,9 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_property(L auto width = LengthStyleValue::create(Length::make_px(border.width)); auto style = IdentifierStyleValue::create(to_value_id(border.line_style)); auto color = ColorStyleValue::create(border.color); - return BorderStyleValue::create(width, style, color); + return ShorthandStyleValue::create(property_id, + { PropertyID::BorderLeftWidth, PropertyID::BorderLeftStyle, PropertyID::BorderLeftColor }, + { width, style, color }); } case PropertyID::BorderLeftColor: return ColorStyleValue::create(layout_node.computed_values().border_left().color); @@ -252,7 +257,9 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_property(L auto width = LengthStyleValue::create(Length::make_px(border.width)); auto style = IdentifierStyleValue::create(to_value_id(border.line_style)); auto color = ColorStyleValue::create(border.color); - return BorderStyleValue::create(width, style, color); + return ShorthandStyleValue::create(property_id, + { PropertyID::BorderRightWidth, PropertyID::BorderRightStyle, PropertyID::BorderRightColor }, + { width, style, color }); } case PropertyID::BorderRightColor: return ColorStyleValue::create(layout_node.computed_values().border_right().color); @@ -268,7 +275,9 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_property(L auto width = LengthStyleValue::create(Length::make_px(border.width)); auto style = IdentifierStyleValue::create(to_value_id(border.line_style)); auto color = ColorStyleValue::create(border.color); - return BorderStyleValue::create(width, style, color); + return ShorthandStyleValue::create(property_id, + { PropertyID::BorderTopWidth, PropertyID::BorderTopStyle, PropertyID::BorderTopColor }, + { width, style, color }); } case PropertyID::BorderTopColor: return ColorStyleValue::create(layout_node.computed_values().border_top().color); diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 581b35fa18..a5d7cd04b1 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -554,63 +553,28 @@ static void set_property_expanding_shorthands(StyleProperties& style, CSS::Prope return; } - if (property_id == CSS::PropertyID::BorderTop - || property_id == CSS::PropertyID::BorderRight - || property_id == CSS::PropertyID::BorderBottom - || property_id == CSS::PropertyID::BorderLeft) { - - if (value.is_border()) { - auto const& border = value.as_border(); - if (property_id == CSS::PropertyID::BorderTop) { - set_longhand_property(PropertyID::BorderTopWidth, border.border_width()); - set_longhand_property(PropertyID::BorderTopStyle, border.border_style()); - set_longhand_property(PropertyID::BorderTopColor, border.border_color()); - return; - } - if (property_id == CSS::PropertyID::BorderRight) { - set_longhand_property(PropertyID::BorderRightWidth, border.border_width()); - set_longhand_property(PropertyID::BorderRightStyle, border.border_style()); - set_longhand_property(PropertyID::BorderRightColor, border.border_color()); - return; - } - if (property_id == CSS::PropertyID::BorderBottom) { - set_longhand_property(PropertyID::BorderBottomWidth, border.border_width()); - set_longhand_property(PropertyID::BorderBottomStyle, border.border_style()); - set_longhand_property(PropertyID::BorderBottomColor, border.border_color()); - return; - } - if (property_id == CSS::PropertyID::BorderLeft) { - set_longhand_property(PropertyID::BorderLeftWidth, border.border_width()); - set_longhand_property(PropertyID::BorderLeftStyle, border.border_style()); - set_longhand_property(PropertyID::BorderLeftColor, border.border_color()); - return; - } - return; - } - if (property_id == CSS::PropertyID::BorderTop) { - set_longhand_property(PropertyID::BorderTopWidth, value); - set_longhand_property(PropertyID::BorderTopStyle, value); - set_longhand_property(PropertyID::BorderTopColor, value); - return; - } - if (property_id == CSS::PropertyID::BorderRight) { - set_longhand_property(PropertyID::BorderRightWidth, value); - set_longhand_property(PropertyID::BorderRightStyle, value); - set_longhand_property(PropertyID::BorderRightColor, value); - return; - } - if (property_id == CSS::PropertyID::BorderBottom) { - set_longhand_property(PropertyID::BorderBottomWidth, value); - set_longhand_property(PropertyID::BorderBottomStyle, value); - set_longhand_property(PropertyID::BorderBottomColor, value); - return; - } - if (property_id == CSS::PropertyID::BorderLeft) { - set_longhand_property(PropertyID::BorderLeftWidth, value); - set_longhand_property(PropertyID::BorderLeftStyle, value); - set_longhand_property(PropertyID::BorderLeftColor, value); - return - } + if (property_id == CSS::PropertyID::BorderTop) { + set_longhand_property(PropertyID::BorderTopWidth, value); + set_longhand_property(PropertyID::BorderTopStyle, value); + set_longhand_property(PropertyID::BorderTopColor, value); + return; + } + if (property_id == CSS::PropertyID::BorderRight) { + set_longhand_property(PropertyID::BorderRightWidth, value); + set_longhand_property(PropertyID::BorderRightStyle, value); + set_longhand_property(PropertyID::BorderRightColor, value); + return; + } + if (property_id == CSS::PropertyID::BorderBottom) { + set_longhand_property(PropertyID::BorderBottomWidth, value); + set_longhand_property(PropertyID::BorderBottomStyle, value); + set_longhand_property(PropertyID::BorderBottomColor, value); + return; + } + if (property_id == CSS::PropertyID::BorderLeft) { + set_longhand_property(PropertyID::BorderLeftWidth, value); + set_longhand_property(PropertyID::BorderLeftStyle, value); + set_longhand_property(PropertyID::BorderLeftColor, value); return; } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp index 9ee5257fb8..57715f3f52 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValue.h index a698d1f914..888fd5361b 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.h @@ -86,7 +86,6 @@ using StyleValueVector = Vector>; __ENUMERATE_STYLE_VALUE_TYPE(Angle, angle) \ __ENUMERATE_STYLE_VALUE_TYPE(BackgroundRepeat, background_repeat) \ __ENUMERATE_STYLE_VALUE_TYPE(BackgroundSize, background_size) \ - __ENUMERATE_STYLE_VALUE_TYPE(Border, border) \ __ENUMERATE_STYLE_VALUE_TYPE(BorderRadius, border_radius) \ __ENUMERATE_STYLE_VALUE_TYPE(Calculated, calculated) \ __ENUMERATE_STYLE_VALUE_TYPE(Color, color) \ diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.cpp deleted file mode 100644 index d60e5e9523..0000000000 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.cpp +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (c) 2018-2020, Andreas Kling - * Copyright (c) 2021, Tobias Christiansen - * Copyright (c) 2021-2023, Sam Atkins - * Copyright (c) 2022-2023, MacDue - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include "BorderStyleValue.h" - -namespace Web::CSS { - -BorderStyleValue::BorderStyleValue( - ValueComparingNonnullRefPtr border_width, - ValueComparingNonnullRefPtr border_style, - ValueComparingNonnullRefPtr border_color) - : StyleValueWithDefaultOperators(Type::Border) - , m_properties { .border_width = move(border_width), .border_style = move(border_style), .border_color = move(border_color) } -{ -} - -BorderStyleValue::~BorderStyleValue() = default; - -String BorderStyleValue::to_string() const -{ - return MUST(String::formatted("{} {} {}", m_properties.border_width->to_string(), m_properties.border_style->to_string(), m_properties.border_color->to_string())); -} - -} diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.h deleted file mode 100644 index 51ca2f5681..0000000000 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/BorderStyleValue.h +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright (c) 2018-2020, Andreas Kling - * Copyright (c) 2021, Tobias Christiansen - * Copyright (c) 2021-2023, Sam Atkins - * Copyright (c) 2022-2023, MacDue - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include - -namespace Web::CSS { - -class BorderStyleValue final : public StyleValueWithDefaultOperators { -public: - static ValueComparingNonnullRefPtr create( - ValueComparingNonnullRefPtr border_width, - ValueComparingNonnullRefPtr border_style, - ValueComparingNonnullRefPtr border_color) - { - return adopt_ref(*new (nothrow) BorderStyleValue(move(border_width), move(border_style), move(border_color))); - } - virtual ~BorderStyleValue() override; - - ValueComparingNonnullRefPtr border_width() const { return m_properties.border_width; } - ValueComparingNonnullRefPtr border_style() const { return m_properties.border_style; } - ValueComparingNonnullRefPtr border_color() const { return m_properties.border_color; } - - virtual String to_string() const override; - - bool properties_equal(BorderStyleValue const& other) const { return m_properties == other.m_properties; } - -private: - BorderStyleValue( - ValueComparingNonnullRefPtr border_width, - ValueComparingNonnullRefPtr border_style, - ValueComparingNonnullRefPtr border_color); - - struct Properties { - ValueComparingNonnullRefPtr border_width; - ValueComparingNonnullRefPtr border_style; - ValueComparingNonnullRefPtr border_color; - bool operator==(Properties const&) const = default; - } m_properties; -}; - -} diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index b205bdbddb..7c2167a7eb 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -75,7 +75,6 @@ class AngleStyleValue; class BackgroundRepeatStyleValue; class BackgroundSizeStyleValue; class BorderRadiusStyleValue; -class BorderStyleValue; class CSSConditionRule; class CSSFontFaceRule; class CSSGroupingRule;