LibWeb: Give equal computed values one canonical representation

Libraries/LibWeb/CSS/Properties.json

@@ -1213,7 +1213,7 @@
     "_comment": "Follows the newer definition from: https://drafts.csswg.org/css-tables-3/#border-spacing-property",
     "animation-type": "by-computed-value",
     "inherited": true,
-    "initial": "0",
+    "initial": "0px 0px",
     "max-values": 2,
     "requires-computation": "cascaded-value",
     "valid-types": [
@@ -4464,9 +4464,9 @@
     "initial": "0px",
     "requires-computation": "cascaded-value",
     "valid-types": [
-      "length [0,∞]",
-      "number [0,∞]",
-      "percentage [0,∞]"
+      "length [-∞,∞]",
+      "number [-∞,∞]",
+      "percentage [-∞,∞]"
     ],
     "percentages-resolve-to": "length"
   },

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -6232,6 +6232,13 @@ static NonnullRefPtr<StyleValue const> compute_style_value_list(NonnullRefPtr<St
     return StyleValueList::create(move(computed_entries), StyleValueList::Separator::Comma);
 }
 
+static NonnullRefPtr<StyleValue const> compute_svg_number_as_length(NonnullRefPtr<StyleValue const> const& style_value)
+{
+    if (!style_value->is_number())
+        return style_value;
+    return LengthStyleValue::create(Length::make_px(style_value->as_number().number()));
+}
+
 // https://drafts.csswg.org/css-contain-2/#contain-property
 static NonnullRefPtr<StyleValue const> collapse_containment_list(NonnullRefPtr<StyleValue const> const& style_value)
 {
@@ -6318,6 +6325,11 @@ NonnullRefPtr<StyleValue const> StyleComputer::compute_value_of_property(
     case PropertyID::BorderTopWidth:
     case PropertyID::OutlineWidth:
         return compute_border_or_outline_width(absolutized_value, device_pixels_per_css_pixel);
+    case PropertyID::BorderSpacing: {
+        if (absolutized_value->is_value_list())
+            return absolutized_value;
+        return StyleValueList::create(StyleValueVector { absolutized_value, absolutized_value }, StyleValueList::Separator::Space);
+    }
     case PropertyID::Contain:
         return collapse_containment_list(absolutized_value);
     case PropertyID::CornerBottomLeftShape:
@@ -6355,6 +6367,20 @@ NonnullRefPtr<StyleValue const> StyleComputer::compute_value_of_property(
         return compute_line_height(absolutized_value, computation_context.length_resolution_context.font_metrics.font_size);
     case PropertyID::MathDepth:
         return compute_math_depth(absolutized_value, inheritance_parent());
+    case PropertyID::StrokeDasharray:
+        // https://svgwg.org/svg2-draft/painting.html#StrokeDasharrayProperty
+        // as comma separated list of absolute lengths or percentages, numbers converted to
+        // absolute lengths first, or keyword specified
+        if (!absolutized_value->is_value_list())
+            return absolutized_value;
+        return compute_style_value_list(absolutized_value, [](NonnullRefPtr<StyleValue const> const& dash) {
+            return compute_svg_number_as_length(dash);
+        });
+    case PropertyID::StrokeDashoffset:
+    case PropertyID::StrokeWidth:
+        // https://svgwg.org/svg2-draft/painting.html#StrokeWidth
+        // an absolute length or percentage, numbers converted to absolute lengths first
+        return compute_svg_number_as_length(absolutized_value);
     case PropertyID::TransformOrigin:
         return compute_transform_origin(absolutized_value);
     default:

Libraries/LibWeb/Rust/src/css/absolutize.rs

@@ -223,6 +223,18 @@ fn rgb_color_function(r: f64, g: f64, b: f64, alpha: f64, color_syntax: u8) -> S
     }
 }
 
+/// The canonical computed form of a fully-resolved legacy sRGB color.
+fn canonical_legacy_rgb(value: &StyleValueData) -> Option<StyleValueData> {
+    let rgba = crate::css::color_resolution::to_color(value, &EMPTY_INPUT)?;
+    Some(rgb_color_function(
+        f64::from(rgba.r),
+        f64::from(rgba.g),
+        f64::from(rgba.b),
+        f64::from(rgba.a) / 255.0,
+        COLOR_SYNTAX_LEGACY,
+    ))
+}
+
 /// Port of hsl_to_absolutized_rgb() in ColorFunctionStyleValue.cpp.
 // https://drafts.csswg.org/css-color-4/#hsl-to-rgb
 fn hsl_to_absolutized_rgb(
@@ -392,13 +404,11 @@ fn absolutize_color_function(value: &StyleValueData, context: &AbsolutizationCon
         } else {
             hwb_to_absolutized_rgb(c1, c2, c3, alpha)
         };
-        return Some(Absolutized::Changed(retain_new(converted)));
+        let canonical = canonical_legacy_rgb(&converted).unwrap_or(converted);
+        return Some(Absolutized::Changed(retain_new(canonical)));
     }
 
-    if !changed {
-        return Some(Absolutized::Unchanged);
-    }
-    Some(Absolutized::Changed(retain_new(StyleValueData::ColorFunction {
+    let rebuilt = StyleValueData::ColorFunction {
         color_base: *color_base,
         channel_0: absolutized_c1,
         channel_1: absolutized_c2,
@@ -407,7 +417,21 @@ fn absolutize_color_function(value: &StyleValueData, context: &AbsolutizationCon
         has_name: *has_name,
         name: name.clone(),
         origin_color: retained_null(),
-    })))
+    };
+
+    if color_base.color_type == crate::css::color_conversion::RGB
+        && let Some(canonical) = canonical_legacy_rgb(&rebuilt)
+    {
+        if canonical == *value {
+            return Some(Absolutized::Unchanged);
+        }
+        return Some(Absolutized::Changed(retain_new(canonical)));
+    }
+
+    if !changed {
+        return Some(Absolutized::Unchanged);
+    }
+    Some(Absolutized::Changed(retain_new(rebuilt)))
 }
 
 /// Port of ColorMixStyleValue::absolutized: normalizes the mix percentages, resolves relative

Libraries/LibWeb/Rust/src/css/computed_value_types.rs

@@ -489,6 +489,7 @@ pub struct FontValues {
     pub math_shift_value: ComputedStyleValueHandle,
     pub math_style_value: ComputedStyleValueHandle,
     pub math_depth_value: ComputedStyleValueHandle,
+    pub font_size_value: ComputedStyleValueHandle,
 }
 
 pub const GRID_NO_INDEX: u32 = u32::MAX;

Libraries/LibWeb/Rust/src/css/computed_values.rs

@@ -625,6 +625,7 @@ impl_computed_payload_clone_and_eq!(FontValues {
     math_shift_value,
     math_style_value,
     math_depth_value,
+    font_size_value,
 });
 impl_computed_payload_clone_and_eq!(InheritedSVGValues {
     fill,
@@ -3016,6 +3017,7 @@ impl FontValues {
             math_shift_value: initial(property_id::MATH_SHIFT),
             math_style_value: initial(property_id::MATH_STYLE),
             math_depth_value: initial(property_id::MATH_DEPTH),
+            font_size_value: initial(property_id::FONT_SIZE),
         }
     }
 }
@@ -3606,9 +3608,9 @@ pub unsafe extern "C" fn rust_build_sizing_group(
 }
 
 /// Builds an inherited table group payload from the computed values, with the
-/// same sharing rules as the inherited box builder. Border-spacing must be an
-/// absolute pixel length; two-value spacings and anything else fall back to
-/// the C++ population path by returning null.
+/// same sharing rules as the inherited box builder. Border-spacing must be a
+/// pair of absolute pixel lengths; anything else falls back to the C++
+/// population path by returning null.
 ///
 /// # Safety
 /// The value pointers must be valid StyleValueData or null, and
@@ -3631,18 +3633,31 @@ pub unsafe extern "C" fn rust_build_inherited_table_group(
                 _ => None,
             }
         };
-        let spacing = match unsafe { (border_spacing as *const StyleValueData).as_ref() } {
-            Some(StyleValueData::Length { value, unit }) if *unit == crate::css::style_compute::px_length_unit() => {
-                crate::css::css_pixels::CssPixels::nearest_value_for(*value).raw_value()
+        let spacing_component = |data: &StyleValueData| -> Option<i32> {
+            match data {
+                StyleValueData::Length { value, unit } if *unit == crate::css::style_compute::px_length_unit() => {
+                    Some(crate::css::css_pixels::CssPixels::nearest_value_for(*value).raw_value())
+                }
+                _ => None,
+            }
+        };
+        let (horizontal_spacing, vertical_spacing) = match unsafe { (border_spacing as *const StyleValueData).as_ref() }
+        {
+            Some(StyleValueData::ValueList { values, .. }) if values.as_slice().len() == 2 => {
+                let components = values.as_slice();
+                (
+                    spacing_component(components[0].data())?,
+                    spacing_component(components[1].data())?,
+                )
             }
             _ => return None,
         };
         let built = InheritedTableValues {
             border_collapse: keyword_code(border_collapse, crate::css::style_compute::keyword_to_border_collapse)?,
             caption_side: keyword_code(caption_side, crate::css::style_compute::keyword_to_caption_side)?,
             empty_cells: keyword_code(empty_cells, crate::css::style_compute::keyword_to_empty_cells)?,
-            border_spacing_horizontal: spacing,
-            border_spacing_vertical: spacing,
+            border_spacing_horizontal: horizontal_spacing,
+            border_spacing_vertical: vertical_spacing,
         };
 
         if !parent_payload.is_null() {

Libraries/LibWeb/Rust/src/css/style_compute.rs

@@ -2356,6 +2356,55 @@ fn compute_position_area(value: &StyleValueData) -> Option<Arc<StyleValueData>>
     }))
 }
 
+/// The computed dash list, with each number converted to the length it measures in user units.
+/// None when the list holds no numbers, so an unchanged list keeps its identity.
+#[allow(clippy::arc_with_non_send_sync)]
+fn stroke_dasharray_numbers_as_lengths(value: &StyleValueData) -> Option<Arc<StyleValueData>> {
+    let StyleValueData::ValueList {
+        values,
+        separator,
+        collapsible,
+    } = value
+    else {
+        return None;
+    };
+    let dashes = values.as_slice();
+    if !dashes
+        .iter()
+        .any(|dash| matches!(dash.data(), StyleValueData::Number { .. }))
+    {
+        return None;
+    }
+    let computed_dashes = dashes
+        .iter()
+        .map(|dash| match dash.data() {
+            StyleValueData::Number { value } => unsafe {
+                RetainedStyleValueData::from_retained_pointer(Arc::into_raw(Arc::new(StyleValueData::Length {
+                    value: *value,
+                    unit: px_length_unit(),
+                })))
+            },
+            _ => dash.clone_retained(),
+        })
+        .collect();
+    Some(Arc::new(StyleValueData::ValueList {
+        values: RetainedStyleValueDataList::from_retained_values(computed_dashes),
+        separator: *separator,
+        collapsible: *collapsible,
+    }))
+}
+
+/// The computed two-value list for a `border-spacing` member used for both axes.
+#[allow(clippy::arc_with_non_send_sync)]
+fn border_spacing_pair(single: StyleValueData) -> Arc<StyleValueData> {
+    let single = unsafe { RetainedStyleValueData::from_retained_pointer(Arc::into_raw(Arc::new(single))) };
+    Arc::new(StyleValueData::ValueList {
+        values: RetainedStyleValueDataList::from_retained_values(vec![single.clone_retained(), single]),
+        separator: 0,
+        collapsible: true,
+    })
+}
+
 // https://drafts.csswg.org/css-contain-2/#contain-property
 #[allow(clippy::arc_with_non_send_sync)]
 fn collapse_containment_list(value: &StyleValueData) -> Option<Arc<StyleValueData>> {
@@ -4051,6 +4100,46 @@ pub unsafe extern "C" fn rust_drive_property_computation(
                         Some(value) => NativeValue::StyleValue(value),
                         None => NativeValue::Unchanged,
                     },
+                    (_, prop::STROKE_DASHOFFSET | prop::STROKE_WIDTH)
+                        if matches!(value_data, StyleValueData::Number { .. }) =>
+                    {
+                        let StyleValueData::Number { value } = value_data else {
+                            unreachable!("the guard accepted only numbers");
+                        };
+                        NativeValue::Px(*value)
+                    }
+                    (None, prop::STROKE_DASHARRAY) if matches!(value_data, StyleValueData::ValueList { .. }) => {
+                        let resolution_context =
+                            length_resolution_context.expect("a dash list must run with a resolution context");
+                        let absolutization_context = crate::css::absolutize::AbsolutizationContext {
+                            length: resolution_context,
+                            scheme: effective_color_scheme,
+                            resolved_viewport_relative_length: std::cell::Cell::new(false),
+                            tree_counting: tree_counting_context,
+                            random_base_values,
+                            document_base_url,
+                            style_sheet_resource_context,
+                        };
+                        let outcome = crate::css::absolutize::absolutize(value_data, &absolutization_context);
+                        if absolutization_context.resolved_viewport_relative_length.get() {
+                            results.depends_on_viewport_metrics = true;
+                        }
+                        match outcome {
+                            Some(crate::css::absolutize::Absolutized::Unchanged) => {
+                                match stroke_dasharray_numbers_as_lengths(value_data) {
+                                    Some(value) => NativeValue::StyleValue(value),
+                                    None => NativeValue::Unchanged,
+                                }
+                            }
+                            Some(crate::css::absolutize::Absolutized::Changed(value)) => {
+                                match stroke_dasharray_numbers_as_lengths(value.data()) {
+                                    Some(computed) => NativeValue::StyleValue(computed),
+                                    None => NativeValue::StyleValue(value.into_arc()),
+                                }
+                            }
+                            None => NativeValue::Unsupported,
+                        }
+                    }
                     (None, prop::TRANSFORM_ORIGIN) => {
                         let resolution_context =
                             length_resolution_context.expect("transform-origin requires a length resolution context");
@@ -4081,6 +4170,21 @@ pub unsafe extern "C" fn rust_drive_property_computation(
                             None => NativeValue::Unsupported,
                         }
                     }
+                    // https://drafts.csswg.org/css-tables-3/#border-spacing-property
+                    // two absolute lengths
+                    // A single specified length computes to the pair with both members equal, so
+                    // every computed border-spacing has the same two-value list shape; a specified
+                    // pair takes the generic arms below.
+                    (_, prop::BORDER_SPACING) if !matches!(value_data, StyleValueData::ValueList { .. }) => {
+                        let single = match absolutized {
+                            Some(Some(px)) => StyleValueData::Length {
+                                value: px,
+                                unit: px_length_unit(),
+                            },
+                            _ => value_data.clone(),
+                        };
+                        NativeValue::StyleValue(border_spacing_pair(single))
+                    }
                     (_, prop::CONTAIN) => match collapse_containment_list(value_data) {
                         Some(value) => NativeValue::StyleValue(value),
                         None => NativeValue::Unchanged,

Libraries/LibWeb/Rust/src/css/table_group_builder.rs

@@ -3090,6 +3090,7 @@ unsafe fn build_font_group(
         math_shift_value: retained(property_id::MATH_SHIFT),
         math_style_value: retained(property_id::MATH_STYLE),
         math_depth_value: retained(property_id::MATH_DEPTH),
+        font_size_value: retained(property_id::FONT_SIZE),
     };
     unsafe {
         crate::css::computed_values::build_group_payload_with_rust_fill(

Tests/LibWeb/Text/expected/MathML/presentational_hints.txt

@@ -1,12 +1,12 @@
 direction: ltr
 direction: ltr
 direction: rtl
-color: red
 color: rgb(255, 0, 0)
-color: orange
-background-color: blue
+color: rgb(255, 0, 0)
+color: rgb(255, 165, 0)
+background-color: rgb(0, 0, 255)
 background-color: rgb(0, 0, 255)
-background-color: transparent
+background-color: rgba(0, 0, 0, 0)
 font-size: 10px
 font-size: 24px
 font-size: 32px

Tests/LibWeb/Text/expected/css/computed-values-svg-geometry.txt

@@ -1,4 +1,4 @@
 rx: 12px
 ry: 34px
-stroke-dasharray: 1px, 2
+stroke-dasharray: 1px, 2px
 stroke-width: 25%

Tests/LibWeb/Text/expected/css/style-engine/computed-border-spacing-pair.txt

@@ -0,0 +1,3 @@
+pair: 10px
+single: 10px
+distinct pair: 10px 20px

Tests/LibWeb/Text/expected/css/style-engine/computed-color-canonical-form.txt

@@ -0,0 +1,6 @@
+named: rgb(255, 0, 0)
+functional: rgb(255, 0, 0)
+hex: rgb(119, 255, 255)
+functional: rgb(119, 255, 255)
+hsl: rgb(0, 255, 0)
+functional: rgb(0, 255, 0)

Tests/LibWeb/Text/expected/css/style-engine/computed-font-size-representation-change.txt

@@ -0,0 +1,2 @@
+inherited: 20.1px
+after: 20.09375px

Tests/LibWeb/Text/expected/css/style-engine/targeted-direct-read-uses-reaction-batch.txt

@@ -1,4 +1,4 @@
-color: green
+color: rgb(0, 128, 0)
 reaction batch runs: 1
 reaction elements: 1
 published reactions: 1

Tests/LibWeb/Text/expected/wpt-import/css/css-typed-om/set-var-reference-thcrash.txt

@@ -2,5 +2,5 @@ Harness status: OK
 
 Found 1 tests
 
-1 Fail
-Fail	Do not crash when referencing a variable with CSSVariableReferenceValue
+1 Pass
+Pass	Do not crash when referencing a variable with CSSVariableReferenceValue