From 85728b297f5fd92aed1443526722a406b88ecbae Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Tue, 18 Mar 2025 12:49:16 +0000 Subject: [PATCH] LibWeb: Ensure the shortest serialization is used for `border-radius` This implementation also fixes an issue where the individual components of the `border-radius` shorthand were always assumed to be of type `BorderRadiusStyleValue`, which could lead to a crash when CSS-wide keywords were used. --- .../CSS/StyleValues/ShorthandStyleValue.cpp | 56 ++++++++++++++----- ...upported-properties-and-default-values.txt | 10 ++-- .../parsing/border-radius-valid.txt | 29 ++++++++++ .../parsing/border-radius-valid.html | 42 ++++++++++++++ 4 files changed, 119 insertions(+), 18 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/border-radius-valid.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/border-radius-valid.html diff --git a/Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp b/Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp index e4efcb47ce8..90363c03366 100644 --- a/Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp +++ b/Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp @@ -134,20 +134,50 @@ String ShorthandStyleValue::to_string(SerializationMode mode) const return MUST(builder.to_string()); } case PropertyID::BorderRadius: { - auto& top_left = longhand(PropertyID::BorderTopLeftRadius)->as_border_radius(); - auto& top_right = longhand(PropertyID::BorderTopRightRadius)->as_border_radius(); - auto& bottom_right = longhand(PropertyID::BorderBottomRightRadius)->as_border_radius(); - auto& bottom_left = longhand(PropertyID::BorderBottomLeftRadius)->as_border_radius(); + auto top_left = longhand(PropertyID::BorderTopLeftRadius); + auto top_right = longhand(PropertyID::BorderTopRightRadius); + auto bottom_right = longhand(PropertyID::BorderBottomRightRadius); + auto bottom_left = longhand(PropertyID::BorderBottomLeftRadius); - return MUST(String::formatted("{} {} {} {} / {} {} {} {}", - top_left.horizontal_radius().to_string(), - top_right.horizontal_radius().to_string(), - bottom_right.horizontal_radius().to_string(), - bottom_left.horizontal_radius().to_string(), - top_left.vertical_radius().to_string(), - top_right.vertical_radius().to_string(), - bottom_right.vertical_radius().to_string(), - bottom_left.vertical_radius().to_string())); + auto horizontal_radius = [&](auto& style_value) -> String { + if (style_value->is_border_radius()) + return style_value->as_border_radius().horizontal_radius().to_string(); + return style_value->to_string(mode); + }; + + auto top_left_horizontal_string = horizontal_radius(top_left); + auto top_right_horizontal_string = horizontal_radius(top_right); + auto bottom_right_horizontal_string = horizontal_radius(bottom_right); + auto bottom_left_horizontal_string = horizontal_radius(bottom_left); + + auto vertical_radius = [&](auto& style_value) -> String { + if (style_value->is_border_radius()) + return style_value->as_border_radius().vertical_radius().to_string(); + return style_value->to_string(mode); + }; + + auto top_left_vertical_string = vertical_radius(top_left); + auto top_right_vertical_string = vertical_radius(top_right); + auto bottom_right_vertical_string = vertical_radius(bottom_right); + auto bottom_left_vertical_string = vertical_radius(bottom_left); + + auto serialize_radius = [](auto top_left, auto const& top_right, auto const& bottom_right, auto const& bottom_left) -> String { + if (first_is_equal_to_all_of(top_left, top_right, bottom_right, bottom_left)) + return top_left; + if (top_left == bottom_right && top_right == bottom_left) + return MUST(String::formatted("{} {}", top_left, top_right)); + if (top_right == bottom_left) + return MUST(String::formatted("{} {} {}", top_left, top_right, bottom_right)); + + return MUST(String::formatted("{} {} {} {}", top_left, top_right, bottom_right, bottom_left)); + }; + + auto first_radius_serialization = serialize_radius(move(top_left_horizontal_string), top_right_horizontal_string, bottom_right_horizontal_string, bottom_left_horizontal_string); + auto second_radius_serialization = serialize_radius(move(top_left_vertical_string), top_right_vertical_string, bottom_right_vertical_string, bottom_left_vertical_string); + if (first_radius_serialization == second_radius_serialization) + return first_radius_serialization; + + return MUST(String::formatted("{} / {}", first_radius_serialization, second_radius_serialization)); } case PropertyID::Columns: { auto column_width = longhand(PropertyID::ColumnWidth)->to_string(mode); diff --git a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-all-supported-properties-and-default-values.txt b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-all-supported-properties-and-default-values.txt index 7d06a3f642d..aea43775afe 100644 --- a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-all-supported-properties-and-default-values.txt +++ b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-all-supported-properties-and-default-values.txt @@ -57,9 +57,9 @@ All supported properties and their default values exposed from CSSStyleDeclarati 'WebkitBorderBottomRightRadius': '0px' 'webkitBorderBottomRightRadius': '0px' '-webkit-border-bottom-right-radius': '0px' -'WebkitBorderRadius': '0px 0px 0px 0px / 0px 0px 0px 0px' -'webkitBorderRadius': '0px 0px 0px 0px / 0px 0px 0px 0px' -'-webkit-border-radius': '0px 0px 0px 0px / 0px 0px 0px 0px' +'WebkitBorderRadius': '0px' +'webkitBorderRadius': '0px' +'-webkit-border-radius': '0px' 'WebkitBorderTopLeftRadius': '0px' 'webkitBorderTopLeftRadius': '0px' '-webkit-border-top-left-radius': '0px' @@ -242,8 +242,8 @@ All supported properties and their default values exposed from CSSStyleDeclarati 'border-left-style': 'none' 'borderLeftWidth': 'medium' 'border-left-width': 'medium' -'borderRadius': '0px 0px 0px 0px / 0px 0px 0px 0px' -'border-radius': '0px 0px 0px 0px / 0px 0px 0px 0px' +'borderRadius': '0px' +'border-radius': '0px' 'borderRight': 'medium none rgb(0, 0, 0)' 'border-right': 'medium none rgb(0, 0, 0)' 'borderRightColor': 'rgb(0, 0, 0)' diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/border-radius-valid.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/border-radius-valid.txt new file mode 100644 index 00000000000..32491ba978b --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/border-radius-valid.txt @@ -0,0 +1,29 @@ +Harness status: OK + +Found 23 tests + +21 Pass +2 Fail +Pass e.style['border-radius'] = "initial" should set the property value +Pass e.style['border-radius'] = "inherit" should set the property value +Pass e.style['border-radius'] = "unset" should set the property value +Pass e.style['border-radius'] = "revert" should set the property value +Pass e.style['border-radius'] = "1px" should set the property value +Pass e.style['border-radius'] = "1px 5%" should set the property value +Pass e.style['border-radius'] = "1px 2% 3px" should set the property value +Pass e.style['border-radius'] = "1px 2% 3px 4%" should set the property value +Pass e.style['border-radius'] = "1px / 2px" should set the property value +Pass e.style['border-radius'] = "5em / 1px 2% 3px 4%" should set the property value +Pass e.style['border-radius'] = "1px 2% / 3px 4px" should set the property value +Pass e.style['border-radius'] = "1px 2px 3em / 1px 2px 3%" should set the property value +Pass e.style['border-radius'] = "1px 2% / 2px 3em 4px 5em" should set the property value +Pass e.style['border-radius'] = "1px 2% 3px 4% / 5em" should set the property value +Pass e.style['border-radius'] = "1px 1px 1px 2% / 1px 2% 1px 2%" should set the property value +Pass e.style['border-radius'] = "1px 1px 1px 1px / 1px 1px 2% 1px" should set the property value +Pass e.style['border-radius'] = "1px 1px 2% 2%" should set the property value +Pass e.style['border-radius'] = "1px 2% 1px 1px" should set the property value +Pass e.style['border-radius'] = "1px 2% 2% 2% / 1px 2% 3px 2%" should set the property value +Pass e.style['border-top-left-radius'] = "10px" should set the property value +Pass e.style['border-top-right-radius'] = "20%" should set the property value +Fail e.style['border-bottom-right-radius'] = "30px 40%" should set the property value +Fail e.style['border-bottom-left-radius'] = "50% 60px" should set the property value \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/border-radius-valid.html b/Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/border-radius-valid.html new file mode 100644 index 00000000000..f2d5ffae59d --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/border-radius-valid.html @@ -0,0 +1,42 @@ + + + + +CSS Backgrounds and Borders Module Level 3: parsing border-radius with valid values + + + + + + + + + +