LibWeb: Move CSS value evaluation onto the Rust value graph - #10861
Merged
Conversation
📝 WalkthroughWalkthroughThis PR migrates LibWeb's CSS style-value ownership model from C++ shell pointers to Rust-owned retained data ( ChangesRust Style Engine Migration
Estimated code review effort: 5 (Critical) | ~180 minutes Sequence Diagram(s)sequenceDiagram
participant KeyframeEffect
participant StyleComputer
participant AnimationRs as "animation.rs"
participant ComputedProperties
KeyframeEffect->>StyleComputer: collect_animations_into(effects)
StyleComputer->>AnimationRs: rust_evaluate_animations(batch)
AnimationRs-->>StyleComputer: FfiAnimationValueResult[]
StyleComputer->>ComputedProperties: apply evaluated values
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
Represent style values with immutable, reference-counted Rust data handles. Teach C++ facades to adopt transferred handles and lazily wrap nested data. Generate animation property metadata and establish scalar interpolation, composition, and FFI crossing measurements on the shared value graph.
Store transform, value-list, and tuple children as shared Rust handles. Materialize typed C++ wrappers only when callers request child values. Cover child handles and wrappers surviving their original parent shells.
Implement transform primitive matching, identity extension, matrix conversion, decomposition, interpolation, and recomposition in Rust. Snapshot reference-box geometry before entering Rust and preserve singular matrix behavior without querying layout through callbacks.
Collect active animation effects per element and pass them to Rust as one batch. Add scalar composition, remaining dimension interpolation, opacity composition, and matching list combination to the Rust evaluator.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (24)
Tests/LibWeb/TestStyleValueEquality.cpp (1)
1308-1316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a helper for the repeated
FfiAnimationContextliteral.The same 7-field initializer is duplicated at Lines 1333-1341, 1379-1387, 1428-1436, 2090-2098, 2389-2397 and 2441-2449. Since this FFI struct is actively changing in this migration, every added field means touching seven test sites.
♻️ Suggested helper
static StyleValueFFI::FfiAnimationContext make_animation_context(bool allow_discrete) { return StyleValueFFI::FfiAnimationContext { .allow_discrete = allow_discrete, .current_color = nullptr, .has_length_resolution_context = false, .length_resolution_context = {}, .has_transform_reference_box = false, .transform_reference_box_width = 0, .transform_reference_box_height = 0, }; }Call sites then become
auto context = make_animation_context(false);, with the reference-box case overriding the two fields it needs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/TestStyleValueEquality.cpp` around lines 1308 - 1316, Extract a shared make_animation_context(bool allow_discrete) helper for the repeated StyleValueFFI::FfiAnimationContext initialization, preserving the existing default field values and passing allow_discrete through. Replace all duplicated literals in the affected tests with this helper, while retaining any reference-box-specific field overrides at their call sites.Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp (1)
132-182: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider named constants for the FFI piece/node kind codes.
piece.kind,piece.numeric_kind, andnode.kindare matched as bare integers here (and again inCalcNodeRef::numeric), so the Rust discriminants are only pinned by convention. Mirroring them as smallenum classvalues next to the existingstatic_assertblock would make drift a compile error instead of aVERIFY_NOT_REACHED()at runtime.Also applies to: 499-579
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp` around lines 132 - 182, Define named enum class values for the FFI piece, numeric, and node kind discriminants beside the existing static_assert block, preserving the Rust numeric values. Update the switches in calculated style serialization and CalcNodeRef::numeric to use these named constants instead of bare integer literals, including piece.kind and piece.numeric_kind, so mismatches are caught at compile time.Libraries/LibWeb/CSS/Rust/src/calc.rs (2)
3526-3535: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that the returned pieces borrow the calculation's style values.
CalcSerializer::style_valuestores the rawStyleValueDatabacking pointer without retaining it, so everyFfiCalcSerializationpiece is only valid whilecalculated(and hence its calculation tree) stays alive. Worth stating in the safety comment next to the existingcalculatedrequirement so a future caller doesn't release the value before draining the batch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/calc.rs` around lines 3526 - 3535, Update the safety documentation for rust_calc_serialize to state that returned FfiCalcSerialization pieces borrow style values from calculated and remain valid only while calculated and its calculation tree stay alive; retain the existing requirement that calculated points to valid Calculated style value data.
3842-3892: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBail out of the reification walk as soon as a child fails.
for_each_childkeeps recursing afterfailedis set, so an unsupported node deep in a large tree still walks and pushes entries for every remaining sibling subtree before the caller discards everything. A short-circuit keeps the common failure path cheap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/calc.rs` around lines 3842 - 3892, Update the child traversal in append_reification_node so its for_each_child callback returns immediately once failed is true, avoiding recursion into remaining sibling subtrees after append_reification_node returns None. Preserve the existing failed state and final None result for unsupported descendants.Libraries/LibWeb/CSS/StyleComputer.cpp (1)
1765-1793: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert the values each action kind requires before dereferencing them.
Start/RemoveAndStartdereferencebefore_change_value/after_change_value, which are only populated whenhas_matching_transition == Yes, and the reversing/interrupted kinds dereferencecurrent_value, only populated whenhas_running_transition. If the Rust decision ever emits a kind outside those preconditions this is a null deref rather than a clear failure. AVERIFYper branch documents and enforces the FFI contract cheaply.🛡️ Proposed guards
case StyleValueFFI::FfiTransitionActionKind::Start: + VERIFY(prepared_transition.before_change_value && prepared_transition.after_change_value); start_a_transition(*prepared_transition.before_change_value, *prepared_transition.after_change_value, *prepared_transition.before_change_value); break; case StyleValueFFI::FfiTransitionActionKind::RemoveAndStart: + VERIFY(prepared_transition.before_change_value && prepared_transition.after_change_value); remove_existing_transition(); start_a_transition(*prepared_transition.before_change_value, *prepared_transition.after_change_value, *prepared_transition.before_change_value); break; case StyleValueFFI::FfiTransitionActionKind::CancelRemoveAndStartReversing: { VERIFY(existing_transition); + VERIFY(prepared_transition.current_value && prepared_transition.after_change_value); auto reversing_adjusted_start_value = existing_transition->transition_end_value(); cancel_and_remove_existing_transition(); start_a_transition(*prepared_transition.current_value, *prepared_transition.after_change_value, *reversing_adjusted_start_value); break; } case StyleValueFFI::FfiTransitionActionKind::CancelRemoveAndStartInterrupted: + VERIFY(prepared_transition.current_value && prepared_transition.after_change_value); cancel_and_remove_existing_transition();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleComputer.cpp` around lines 1765 - 1793, Assert each transition action’s required prepared values before dereferencing them in the switch. Add VERIFY checks for before_change_value and after_change_value in Start and RemoveAndStart, and for current_value in CancelRemoveAndStartReversing and CancelRemoveAndStartInterrupted; preserve the existing transition operations and reversing_adjusted_start_value handling.Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h (1)
49-51: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReturn the stored components by const reference.
first_component()/second_component()copy aColorMixComponent(two refcount bumps) per call, andserializealone calls them four times (Lines 312-316 ofColorMixStyleValue.cpp). Now that they are plain members,ColorMixComponent const&is free.♻️ Proposed refactor
- ColorMixComponent first_component() const { return m_first_component; } - ColorMixComponent second_component() const { return m_second_component; } + ColorMixComponent const& first_component() const { return m_first_component; } + ColorMixComponent const& second_component() const { return m_second_component; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h` around lines 49 - 51, Update ColorMixStyleValue::first_component() and second_component() to return ColorMixComponent const& instead of values, preserving their existing access to m_first_component and m_second_component; leave color_interpolation_method_value() unchanged.Libraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.h (1)
38-41: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider returning
channels()by const reference.
channels()copies threeValueComparingNonnullRefPtr(three atomic increments/decrements) per call, and it's called repeatedly per invocation into_color,absolutized, andserialize(e.g. Lines 506-510 and 521-528 ofLibraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.cpp). Since the array is now a stable member, a const reference is free.♻️ Proposed refactor
- Array<ValueComparingNonnullRefPtr<StyleValue const>, 3> channels() const + Array<ValueComparingNonnullRefPtr<StyleValue const>, 3> const& channels() const { return m_channels; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.h` around lines 38 - 41, Update ColorFunctionStyleValue::channels() to return a const reference to the stable m_channels member instead of returning the array by value, preserving its const access and existing callers.Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp (1)
391-401: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnconditional default-method allocation on every
to_color()call.
ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab)allocates a C++ shell plus a RustStyleValueDataon every call, even whencolor_interpolation_method_value()is present. Consider a function-local static (the value is immutable and shareable) or constructing it only in the null branch. Same pattern at Lines 422-425.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp` around lines 391 - 401, Update the default interpolation-method setup in to_color() so ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab) runs only when color_interpolation_method_value() is absent, avoiding allocation when an explicit method exists. Apply the same lazy-default change to the corresponding setup around the second occurrence at lines 422-425, while preserving the existing Oklab fallback behavior.Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h (1)
36-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared helpers for the repeated retain/adopt/create idiom.
Every migrated subclass repeats the same two one-liners verbatim: "retain-then-create" (
StyleValueFFI::rust_style_value_retain(x->rust_style_value_data())fed into arust_style_value_create_*call) and "retain-then-adopt" (StyleValue::adopt_rust_style_value_data(StyleValueFFI::rust_style_value_retain(ptr))).BorderImageSliceStyleValue.halone repeats this 8 times; the same shape recurs across the whole cohort.
Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h#L36-L52: replace the 4 retain+adopt calls in the FFI-data ctor and 4 retain+create calls in the value ctor with two smallStyleValuestatic helpers (e.g.StyleValue::adopt_retained_child(ptr)andStyleValue::retained_data(value)).Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.cpp#L15-L17: use the "retain-then-create" helper for size_x/size_y.Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.h#L38-L48: use the "retain-then-adopt" helper for size_x/size_y.Libraries/LibWeb/CSS/StyleValues/BorderRadiusRectStyleValue.h#L48-L64: use both helpers for the 4 corners.Libraries/LibWeb/CSS/StyleValues/BorderRadiusStyleValue.h#L45-L57: use both helpers for horizontal/vertical radius.Libraries/LibWeb/CSS/StyleValues/CounterStyleSystemStyleValue.h#L76-L98: use both helpers for the optionalfirst_symbol.Libraries/LibWeb/CSS/StyleValues/PendingSubstitutionStyleValue.h#L44-L52: use both helpers fororiginal_shorthand_value.Libraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.h#L53-L62: use the "retain-then-create" helper per-entry in the loop.A single point of truth for this retain-count bump reduces the risk of a future subclass forgetting the retain (leading to a use-after-free) or double-retaining (a leak).
♻️ Example helper shape
// In StyleValue.h (private/protected static helpers) static ValueComparingNonnullRefPtr<StyleValue const> adopt_retained_child(StyleValueFFI::StyleValueData const* pointer) { return adopt_rust_style_value_data(StyleValueFFI::rust_style_value_retain(pointer)); } static StyleValueFFI::StyleValueData const* retained_data(StyleValue const& value) { return StyleValueFFI::rust_style_value_retain(value.rust_style_value_data()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h` around lines 36 - 52, The repeated retain/adopt and retain/create expressions should be centralized in two StyleValue static helpers that each perform exactly one retain-count bump. Add helpers such as adopt_retained_child and retained_data, then update Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h lines 36-52 to use them for all four children in both constructors; apply the corresponding helper at Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.cpp lines 15-17, BackgroundSizeStyleValue.h lines 38-48, BorderRadiusRectStyleValue.h lines 48-64, BorderRadiusStyleValue.h lines 45-57, CounterStyleSystemStyleValue.h lines 76-98, PendingSubstitutionStyleValue.h lines 44-52, and CounterDefinitionsStyleValue.h lines 53-62, including each loop entry and optional first_symbol without changing ownership semantics.Libraries/LibWeb/CSS/StyleValues/TransformationStyleValue.h (1)
29-32: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider returning
m_valuesby const reference.
values()now copies the cachedStyleValueVector(aVectorof ref-counted pointers) on every call, incurring a refcount bump per element. Sincem_valuesis already a stable member, returningStyleValueVector const&would avoid the copy for callers (e.g.ShorthandStyleValue::serialize) that only read from it. This is already a big improvement over the previous per-call Rust rematerialization, so the remaining copy cost is low.Optional tweak
- StyleValueVector values() const + StyleValueVector const& values() const { return m_values; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/TransformationStyleValue.h` around lines 29 - 32, Update the values() accessor to return a const reference to the existing m_values member instead of returning StyleValueVector by value, preserving read-only access while avoiding per-call vector and refcount copies.Libraries/LibWeb/CSS/Rust/src/animation.rs (6)
6346-6352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParenthesize the mixed
||/&&fallback condition.
!result.handled || result.value.is_null() && context.is_some_and(...)relies on&&binding tighter than||. The behavior is correct (handled-with-null andallow_discretefalse must return the null result), but this is the file's most consequential branch and reads as if it were(!handled || null) && allow_discrete. Explicit parentheses cost nothing.♻️ Proposed change
- if !result.handled || result.value.is_null() && context.is_some_and(|context| context.allow_discrete) { + if !result.handled || (result.value.is_null() && context.is_some_and(|context| context.allow_discrete)) { return discrete_value(context, from, to, delta); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 6346 - 6352, Parenthesize the `result.value.is_null() && context.is_some_and(|context| context.allow_discrete)` portion of the fallback condition in the interpolation flow, preserving the existing `!result.handled || (...)` behavior and return paths.
693-699: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winInteger interpolation loses precision above 2^24.
interpolate_i32does the arithmetic inf32, so large operands (e.g.z-index: 16777217) round to the wrong integer even atdelta0 or 1. The surrounding code already works inf64; using it here removes the artifact at no cost.♻️ Proposed change
- let value = (from as f32 + (to as f32 - from as f32) * delta).round(); - clamp_to_range(f64::from(value), range) as i32 + let value = (f64::from(from) + (f64::from(to) - f64::from(from)) * f64::from(delta)).round(); + clamp_to_range(value, range) as i32🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 693 - 699, Update interpolate_i32 to perform interpolation arithmetic in f64 rather than f32, including the conversion of from, to, and delta before rounding. Preserve the existing clamp_to_range behavior and i32 return conversion so integer endpoints such as values above 2^24 remain exact at delta 0 and 1.
4089-4108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated quaternion slerp.
The inline slerp inside
interpolate_rotate_3dis a line-for-line copy ofslerp_quaternions(Line 4693), including thef32::EPSILONdegeneracy checks. Two copies of numerically delicate code will drift; call the helper instead.Also applies to: 4693-4713
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 4089 - 4108, The inline quaternion slerp in interpolate_rotate_3d duplicates the existing slerp_quaternions implementation. Replace the local product, angle, weight, and degeneracy-handling logic with a call to slerp_quaternions, passing the same from_quaternion, to_quaternion, and delta inputs, while preserving the current interpolation result.
6567-6570: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit coverage for the pure-math transform paths.
The module's most intricate logic —
decompose_matrix/recompose_matrix,interpolate_matrices,slerp_quaternions,interpolate_rotate_3d, and the grid track expansion — has no unit tests here and is only exercised indirectly through the WPT text expectations. A round-trip assertion (recompose_matrix(decompose_matrix(m)) ≈ m) and a degenerate-axis rotate3d case would have caught the NaN path flagged at Line 4064 directly, with far tighter feedback than a WPT diff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 6567 - 6570, Add unit tests in the existing tests module covering the pure-math paths decompose_matrix/recompose_matrix, interpolate_matrices, slerp_quaternions, interpolate_rotate_3d, and grid track expansion. Include a matrix round-trip assertion with approximate equality and a degenerate-axis rotate3d case verifying finite, expected behavior, so the NaN path is caught directly.
6079-6110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffCollapse the repeated
ANIMATION_TYPE_CUSTOM &&chain.
animation_type == ANIMATION_TYPE_CUSTOMis re-tested about a dozen times, each paired with a property-id comparison, before the final catch-all discrete fallback. A singleif animation_type == ANIMATION_TYPE_CUSTOM { match property_id { ... } }would express "custom algorithms, dispatched by property" directly and make it obvious that every custom property either returns or falls through to discrete.Also applies to: 6184-6186, 6245-6292, 6306-6344
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 6079 - 6110, Refactor the custom-animation handling in the surrounding interpolation function to check animation_type == ANIMATION_TYPE_CUSTOM once, then dispatch property-specific algorithms through a single match on property_id, including the existing filter, shadow, stroke-dasharray, and other custom-property branches referenced by the comment. Preserve each branch’s current return and fall-through behavior so unhandled custom properties still reach the existing discrete fallback.
26-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the generated enum constants instead of raw discriminants.
These values already come from the shared CSS enum source, so copying them intoanimation.rsadds a second source of truth.
Libraries/LibWeb/CSS/Rust/src/animation.rs#L26-L86: import the generatedcss_enumsconstants instead of duplicatingVALUE_TYPE_*,TRANSFORM_FUNCTION_*,COLOR_TYPE_*,STEP_POSITION_*, andBASIC_SHAPE_*.Libraries/LibWeb/CSS/Rust/src/animation.rs#L5593-L5597,#L5684-L5697,#L5749-L5757: replace the bareColorFilterTypeordinals with named constants as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/animation.rs` around lines 26 - 86, In Libraries/LibWeb/CSS/Rust/src/animation.rs at lines 26-86, remove the duplicated VALUE_TYPE_*, TRANSFORM_FUNCTION_*, COLOR_TYPE_*, STEP_POSITION_*, and BASIC_SHAPE_* definitions and import the corresponding generated css_enums constants. At lines 5593-5597, 5684-5697, and 5749-5757, replace bare ColorFilterType ordinals with the generated named constants; update all references to preserve the existing values and behavior.Tests/LibWeb/Text/input/css/repeatable-list-animation-rust.html (1)
15-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCounters are reset but never asserted.
resetStyleFfiCounters()has no effect on this test's output, so the-rustsuffix isn't actually guarded — the test would still pass if evaluation fell back to a non-Rust path. Either drop the reset, or printinternals.styleFfiCounters().animationEvaluationEntriesthe waytransition-effect-batch-rust.htmldoes (remember to update the expected.txt). The same unused reset appears inTests/LibWeb/Text/input/css/shadow-animation-rust.htmlLine 12.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/Text/input/css/repeatable-list-animation-rust.html` around lines 15 - 17, Update the Rust animation tests around resetStyleFfiCounters(), including repeatable-list and shadow-animation cases, so the reset is either removed or followed by output of internals.styleFfiCounters().animationEvaluationEntries to verify Rust evaluation; update the corresponding expected output files if counters are printed.Libraries/LibWeb/CSS/Rust/src/color_conversion.rs (4)
284-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated hue derivation between
srgb_to_hwbandsrgb_to_hsl.Lines 291-301 are a copy of Lines 264-278 minus the negative-saturation fixup. Extracting a shared
fn srgb_hue(red, green, blue, chroma) -> f32would remove the drift risk between the two copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs` around lines 284 - 304, Extract the duplicated hue calculation from srgb_to_hwb and srgb_to_hsl into a shared srgb_hue(red, green, blue, chroma) helper returning f32. Update both conversion functions to call this helper while preserving their existing saturation-specific behavior and output values.
231-231: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent alpha handling: only
hsl_to_srgbclamps alpha.Every other conversion in this module passes
color[3]through unchanged; this one clamps to[0, 1]. That makesconvert()'s alpha behavior depend on the source space, which is surprising for a pure conversion table (and clamping is already the caller's job incolor_interpolation.rsLine 324).♻️ Proposed consistency fix
- [convert(0.0), convert(8.0), convert(4.0), color[3].clamp(0.0, 1.0)] + [convert(0.0), convert(8.0), convert(4.0), color[3]]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs` at line 231, Update the hsl_to_srgb conversion entry to pass color[3] through unchanged instead of clamping it, matching the alpha handling of the other conversion functions and leaving range clamping to the caller.
493-513: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing the reverse
SRGB→RGBfast path.Line 494 short-circuits
RGB→SRGBbut not the reverse, soSRGB→RGBfalls through to Line 512 and round-trips through linear-light XYZ D65 — two transfer-function applications and two matrix multiplies that should be identity, leaving float drift in the result. Also worth parenthesizing the mixed||/&&for readability.♻️ Proposed fix
- if color_type == target_type || color_type == RGB && target_type == SRGB { + if color_type == target_type + || (color_type == RGB && target_type == SRGB) + || (color_type == SRGB && target_type == RGB) + { return Some(color); }Note this changes
RGB's gamut-clamp behavior only for theSRGB→RGBdirection, whichto_xyz65(Line 445) currently clamps andfrom_xyz65(Line 472) does not — so confirm which side is meant to own clamping before applying.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs` around lines 493 - 513, Update convert to short-circuit both RGB↔SRGB conversions, including the missing SRGB-to-RGB path, while preserving the intended gamut-clamping behavior for that direction; confirm whether clamping belongs on the source or destination side before implementing. Parenthesize the mixed conditions in convert for readability, especially the RGB/SRGB and HSL/HWB checks.
523-561: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRound-trip test never exercises the piecewise linear segments.
All three components of
source(0.25/0.5/0.75) sit above the knee of every transfer function, so the<= 16/512,< BETA, and<= 0.04045branches — the parts most prone to a transposed constant — are never executed. Adding a near-zero sample would cover them, andRGB(legacy, the gamut-clamping arm) isn't in the list at all.Also, Line 525 asserts float equality; the neighboring white assertion uses a tolerance, so applying one consistently would be less brittle.
💚 Proposed test extension
- let source = [0.25, 0.5, 0.75, 0.8]; - for color_type in [ + for source in [[0.25, 0.5, 0.75, 0.8], [0.001, 0.01, 0.02, 1.0]] { + for color_type in [ SRGB,(then close the extra scope, and thread
sourcethrough the existing assertions)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs` around lines 523 - 561, Extend round_trips_supported_color_spaces to test a near-zero source sample that exercises the low-end piecewise transfer-function branches, and include the legacy RGB color space so its gamut-clamping path is covered. Thread each sample through the existing conversion and component assertions, preserving the current sample, and update converts_srgb_endpoints_to_oklab to compare the black endpoint with the same tolerance-based approach used for white.Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs (3)
364-364: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueClamp alpha after applying
alpha_multiplier.
interpolated_alphais clamped at Line 324, but the multiplier is applied afterwards without re-clamping, so a multiplier above1.0yields an out-of-range alpha in the constructed color.♻️ Proposed clamp
- result[3] *= alpha_multiplier; + result[3] = (result[3] * alpha_multiplier).clamp(0.0, 1.0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs` at line 364, Update the alpha handling in the color interpolation routine containing interpolated_alpha and result[3] so the value is clamped again after applying alpha_multiplier. Preserve the existing multiplier operation, then constrain the resulting alpha to the valid [0.0, 1.0] range before constructing the color.
54-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the spec tables in this module.
carry_forward_missing_components,fixup_hues, andsubstitute_missing_componentsare pure functions over fixed-size arrays and encode the trickiest parts of css-color-4, yet this module has no#[cfg(test)]block while its siblingcolor_conversion.rsdoes. Table-driven tests over the analogous-component and hue-fixup cases would pin the behavior cheaply.Want me to draft the test module?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs` around lines 54 - 62, Add a #[cfg(test)] module in color_interpolation.rs with table-driven unit tests covering the pure functions carry_forward_missing_components, fixup_hues, and substitute_missing_components. Include cases for analogous-component handling, all-missing component groups, and the CSS Color 4 hue-fixup scenarios, asserting each fixed-size array result against the specification tables.
217-219: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAvoid
.unwrap()in an abort-on-panic FFI path.The
unwrap()is currently safe because everypowerless-true arm has a hue index, but the invariant lives in a separatematch. A future polar arm would turn this into a process abort viaabort_on_panic.♻️ Proposed defensive rewrite
- if powerless { - missing[hue_index(target_type).unwrap()] = true; - } + if powerless { + if let Some(index) = hue_index(target_type) { + missing[index] = true; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs` around lines 217 - 219, Update the powerless handling around target_type and hue_index to avoid calling unwrap in this abort-on-panic FFI path. Validate or pattern-match the hue index before assigning missing, and handle the impossible/no-hue case defensively without panicking while preserving the existing behavior for hue-bearing polar types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/CSS/ComputedValues.cpp`:
- Around line 950-978: Guard the nullable result from
ComputedValuesFFI::rust_build_alignment_group in ComputedValues::create() before
adopting it, using the function’s established VERIFY/null-check pattern, and
ensure multi-keyword align-* and justify-* computed values cannot reach the
builder. Apply the same guard to the rust_build_svg_reset_group result at
Libraries/LibWeb/CSS/ComputedValues.cpp lines 1279-1295 for vector-effect and
shape-rendering mappings.
In `@Libraries/LibWeb/CSS/PercentageOr.h`:
- Around line 112-121: Update LengthPercentage::operator== to compare Calculated
operands structurally using calculated()->equals(*other.calculated()), rather
than relying only on m_value pointer identity. Preserve the existing length,
percentage, and false-result branches for other operand types.
In `@Libraries/LibWeb/CSS/PseudoElementPropertyGroups.txt`:
- Around line 38-86: Update the [border-properties] group to include the
border-image shorthand alongside the existing border-image-* longhands, ensuring
pseudo-elements using this property group accept the complete border image
property set.
In `@Libraries/LibWeb/CSS/Rust/src/animation.rs`:
- Around line 497-500: Update property_is_important to use checked subtraction
when computing the index from FIRST_LONGHAND_PROPERTY_ID, returning false
immediately when property_id is below that base; preserve the existing bitmap
lookup for valid longhand IDs.
- Around line 4064-4108: Use the already-computed from_axis_normalized and
to_axis_normalized values when constructing from_quaternion and to_quaternion in
the quaternion interpolation branch, rather than passing the raw from_axis and
to_axis values. Preserve the existing degenerate-axis fallback behavior and
quaternion interpolation logic.
- Around line 3432-3446: The Length interpolation arm in the animation value
matching logic must reject or fall through when the source and target units
differ. Compare the target Length unit with from_unit before calling
interpolate_f64, preserving the existing raw interpolation only for matching
units and allowing mismatches to use the appropriate fallback path.
- Around line 612-636: Update the shorthand expansion callback around
expand_shorthands_with and AnimationPropertyConflictCandidate so synthesized
pending-substitution values remain retained after the callback returns. Ensure
candidates store a retained reference or otherwise preserve the pending value’s
lifetime, while leaving regular borrowed sub-value handling unchanged.
In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs`:
- Around line 11-26: Replace the duplicated color-space,
polar/rectangular-space, and hue-method ordinal constants in
Libraries/LibWeb/CSS/Rust/src/color_conversion.rs (lines 11-26) and
Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs (lines 112-137) with shared
bindings to the C++ enum definitions, or add compile-time assertions covering
every ordinal at both sites. Ensure Rust cannot silently diverge when the
corresponding C++ enums change.
In `@Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs`:
- Around line 280-308: Update resolve_color_for_rust_interpolation() so
mark_powerless_hue_after_conversion() runs before the source_type == target_type
early return, including same-space polar interpolation. Ensure achromatic polar
hues become missing before endpoint missing-component propagation, while
preserving the existing conversion behavior for differing color spaces.
In `@Libraries/LibWeb/CSS/Rust/src/style_compute.rs`:
- Around line 1180-1192: Update value_is_computationally_independent and the
rust_style_value_is_computationally_independent call path to handle every
computed StyleValueData variant, including Url, Image, ValueList, Tuple,
ColorScheme, and CounterDefinitions, without returning None for supported
values. Preserve the existing behavior for genuinely unsupported values while
ensuring rust_drive_property_computation can query all computed longhands
without unwrapping None.
- Around line 1670-1681: Update the position-area value handling in
parse_position_area to accept the valid single-keyword form returned by parsing
instead of treating it as unreachable. Special-case one keyword and return the
appropriate result (or None), while preserving the existing two-keyword
block/inline handling.
In `@Libraries/LibWeb/CSS/Rust/src/transition.rs`:
- Around line 268-277: Update rust_decide_transitions to check
input.property_count before calling std::slice::from_raw_parts; when the count
is zero, return from the abort_on_panic closure without dereferencing the null
ffi_properties.data() pointer, while preserving the existing transition loop for
non-empty lists.
---
Nitpick comments:
In `@Libraries/LibWeb/CSS/Rust/src/animation.rs`:
- Around line 6346-6352: Parenthesize the `result.value.is_null() &&
context.is_some_and(|context| context.allow_discrete)` portion of the fallback
condition in the interpolation flow, preserving the existing `!result.handled ||
(...)` behavior and return paths.
- Around line 693-699: Update interpolate_i32 to perform interpolation
arithmetic in f64 rather than f32, including the conversion of from, to, and
delta before rounding. Preserve the existing clamp_to_range behavior and i32
return conversion so integer endpoints such as values above 2^24 remain exact at
delta 0 and 1.
- Around line 4089-4108: The inline quaternion slerp in interpolate_rotate_3d
duplicates the existing slerp_quaternions implementation. Replace the local
product, angle, weight, and degeneracy-handling logic with a call to
slerp_quaternions, passing the same from_quaternion, to_quaternion, and delta
inputs, while preserving the current interpolation result.
- Around line 6567-6570: Add unit tests in the existing tests module covering
the pure-math paths decompose_matrix/recompose_matrix, interpolate_matrices,
slerp_quaternions, interpolate_rotate_3d, and grid track expansion. Include a
matrix round-trip assertion with approximate equality and a degenerate-axis
rotate3d case verifying finite, expected behavior, so the NaN path is caught
directly.
- Around line 6079-6110: Refactor the custom-animation handling in the
surrounding interpolation function to check animation_type ==
ANIMATION_TYPE_CUSTOM once, then dispatch property-specific algorithms through a
single match on property_id, including the existing filter, shadow,
stroke-dasharray, and other custom-property branches referenced by the comment.
Preserve each branch’s current return and fall-through behavior so unhandled
custom properties still reach the existing discrete fallback.
- Around line 26-86: In Libraries/LibWeb/CSS/Rust/src/animation.rs at lines
26-86, remove the duplicated VALUE_TYPE_*, TRANSFORM_FUNCTION_*, COLOR_TYPE_*,
STEP_POSITION_*, and BASIC_SHAPE_* definitions and import the corresponding
generated css_enums constants. At lines 5593-5597, 5684-5697, and 5749-5757,
replace bare ColorFilterType ordinals with the generated named constants; update
all references to preserve the existing values and behavior.
In `@Libraries/LibWeb/CSS/Rust/src/calc.rs`:
- Around line 3526-3535: Update the safety documentation for rust_calc_serialize
to state that returned FfiCalcSerialization pieces borrow style values from
calculated and remain valid only while calculated and its calculation tree stay
alive; retain the existing requirement that calculated points to valid
Calculated style value data.
- Around line 3842-3892: Update the child traversal in append_reification_node
so its for_each_child callback returns immediately once failed is true, avoiding
recursion into remaining sibling subtrees after append_reification_node returns
None. Preserve the existing failed state and final None result for unsupported
descendants.
In `@Libraries/LibWeb/CSS/Rust/src/color_conversion.rs`:
- Around line 284-304: Extract the duplicated hue calculation from srgb_to_hwb
and srgb_to_hsl into a shared srgb_hue(red, green, blue, chroma) helper
returning f32. Update both conversion functions to call this helper while
preserving their existing saturation-specific behavior and output values.
- Line 231: Update the hsl_to_srgb conversion entry to pass color[3] through
unchanged instead of clamping it, matching the alpha handling of the other
conversion functions and leaving range clamping to the caller.
- Around line 493-513: Update convert to short-circuit both RGB↔SRGB
conversions, including the missing SRGB-to-RGB path, while preserving the
intended gamut-clamping behavior for that direction; confirm whether clamping
belongs on the source or destination side before implementing. Parenthesize the
mixed conditions in convert for readability, especially the RGB/SRGB and HSL/HWB
checks.
- Around line 523-561: Extend round_trips_supported_color_spaces to test a
near-zero source sample that exercises the low-end piecewise transfer-function
branches, and include the legacy RGB color space so its gamut-clamping path is
covered. Thread each sample through the existing conversion and component
assertions, preserving the current sample, and update
converts_srgb_endpoints_to_oklab to compare the black endpoint with the same
tolerance-based approach used for white.
In `@Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs`:
- Line 364: Update the alpha handling in the color interpolation routine
containing interpolated_alpha and result[3] so the value is clamped again after
applying alpha_multiplier. Preserve the existing multiplier operation, then
constrain the resulting alpha to the valid [0.0, 1.0] range before constructing
the color.
- Around line 54-62: Add a #[cfg(test)] module in color_interpolation.rs with
table-driven unit tests covering the pure functions
carry_forward_missing_components, fixup_hues, and substitute_missing_components.
Include cases for analogous-component handling, all-missing component groups,
and the CSS Color 4 hue-fixup scenarios, asserting each fixed-size array result
against the specification tables.
- Around line 217-219: Update the powerless handling around target_type and
hue_index to avoid calling unwrap in this abort-on-panic FFI path. Validate or
pattern-match the hue index before assigning missing, and handle the
impossible/no-hue case defensively without panicking while preserving the
existing behavior for hue-bearing polar types.
In `@Libraries/LibWeb/CSS/StyleComputer.cpp`:
- Around line 1765-1793: Assert each transition action’s required prepared
values before dereferencing them in the switch. Add VERIFY checks for
before_change_value and after_change_value in Start and RemoveAndStart, and for
current_value in CancelRemoveAndStartReversing and
CancelRemoveAndStartInterrupted; preserve the existing transition operations and
reversing_adjusted_start_value handling.
In `@Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h`:
- Around line 36-52: The repeated retain/adopt and retain/create expressions
should be centralized in two StyleValue static helpers that each perform exactly
one retain-count bump. Add helpers such as adopt_retained_child and
retained_data, then update
Libraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.h lines 36-52 to use
them for all four children in both constructors; apply the corresponding helper
at Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.cpp lines 15-17,
BackgroundSizeStyleValue.h lines 38-48, BorderRadiusRectStyleValue.h lines
48-64, BorderRadiusStyleValue.h lines 45-57, CounterStyleSystemStyleValue.h
lines 76-98, PendingSubstitutionStyleValue.h lines 44-52, and
CounterDefinitionsStyleValue.h lines 53-62, including each loop entry and
optional first_symbol without changing ownership semantics.
In `@Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp`:
- Around line 132-182: Define named enum class values for the FFI piece,
numeric, and node kind discriminants beside the existing static_assert block,
preserving the Rust numeric values. Update the switches in calculated style
serialization and CalcNodeRef::numeric to use these named constants instead of
bare integer literals, including piece.kind and piece.numeric_kind, so
mismatches are caught at compile time.
In `@Libraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.h`:
- Around line 38-41: Update ColorFunctionStyleValue::channels() to return a
const reference to the stable m_channels member instead of returning the array
by value, preserving its const access and existing callers.
In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp`:
- Around line 391-401: Update the default interpolation-method setup in
to_color() so
ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab) runs
only when color_interpolation_method_value() is absent, avoiding allocation when
an explicit method exists. Apply the same lazy-default change to the
corresponding setup around the second occurrence at lines 422-425, while
preserving the existing Oklab fallback behavior.
In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h`:
- Around line 49-51: Update ColorMixStyleValue::first_component() and
second_component() to return ColorMixComponent const& instead of values,
preserving their existing access to m_first_component and m_second_component;
leave color_interpolation_method_value() unchanged.
In `@Libraries/LibWeb/CSS/StyleValues/TransformationStyleValue.h`:
- Around line 29-32: Update the values() accessor to return a const reference to
the existing m_values member instead of returning StyleValueVector by value,
preserving read-only access while avoiding per-call vector and refcount copies.
In `@Tests/LibWeb/TestStyleValueEquality.cpp`:
- Around line 1308-1316: Extract a shared make_animation_context(bool
allow_discrete) helper for the repeated StyleValueFFI::FfiAnimationContext
initialization, preserving the existing default field values and passing
allow_discrete through. Replace all duplicated literals in the affected tests
with this helper, while retaining any reference-box-specific field overrides at
their call sites.
In `@Tests/LibWeb/Text/input/css/repeatable-list-animation-rust.html`:
- Around line 15-17: Update the Rust animation tests around
resetStyleFfiCounters(), including repeatable-list and shadow-animation cases,
so the reset is either removed or followed by output of
internals.styleFfiCounters().animationEvaluationEntries to verify Rust
evaluation; update the corresponding expected output files if counters are
printed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Retain and combine superellipses, text indents, background sizes, edges, positions, rectangles, border radii, and border image slices in Rust. Keep nested values on the shared Rust graph and lazily expose typed facades.
Retain and combine OpenType settings and function arguments in Rust. Move font-style angle ownership and interpolation onto the same value graph. Cover nested handles and C++ wrappers outliving their parent facades.
Move identifiers, counters, colors, anchors, generated content, local font names, pending shorthands, and related nested values to Rust handles. Preserve lazy typed C++ access without retaining child shells from Rust.
There was a problem hiding this comment.
🧹 Nitpick comments (5)
Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp (1)
429-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the default Oklab interpolation-method construction. Both call sites build the same
ColorInterpolationMethodStyleValueinline on every invocation (each one also allocating Rust FFI data), and both repeat the samestatic_cast<StyleValue const&>ternary dance.
Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp#L429-L432: replace the inline default with the shared helper and pass the resulting ref tointerpolate_color_in_rust.Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp#L460-L463: use the same helper for the absolutized path.♻️ Proposed helper + call-site updates
Add near the other file-local helpers:
static ValueComparingNonnullRefPtr<StyleValue const> default_color_interpolation_method() { return ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab); }
to_color():- auto default_color_interpolation_method = ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab); - auto const& color_interpolation_method = color_interpolation_method_value() - ? *color_interpolation_method_value() - : static_cast<StyleValue const&>(*default_color_interpolation_method); + auto interpolation_method = color_interpolation_method_value() + ? ValueComparingNonnullRefPtr<StyleValue const> { *color_interpolation_method_value() } + : default_color_interpolation_method(); auto style_value = interpolate_color_in_rust( *first_component().color, *second_component().color, normalized.second_percentage.as_fraction(), normalized.alpha_multiplier, - color_interpolation_method, + *interpolation_method, color_resolution_context);
absolutized():- auto default_color_interpolation_method = ColorInterpolationMethodStyleValue::create(RectangularColorSpace::Oklab); - auto const& color_interpolation_method = absolutized_color_interpolation_method - ? *absolutized_color_interpolation_method - : static_cast<StyleValue const&>(*default_color_interpolation_method); + auto interpolation_method = absolutized_color_interpolation_method + ? ValueComparingNonnullRefPtr<StyleValue const> { *absolutized_color_interpolation_method } + : default_color_interpolation_method();(and pass
*interpolation_methodat theinterpolate_color_in_rustcall on L486.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp` around lines 429 - 432, Deduplicate default Oklab interpolation-method creation in ColorMixStyleValue.cpp by adding a file-local default_color_interpolation_method() helper returning the shared StyleValue reference. Update both to_color() at lines 429-432 and absolutized() at lines 460-463 to use this helper and pass the resulting reference to interpolate_color_in_rust, including the call at line 486.Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp (1)
221-304: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake
CalcResolutionSnapshotnon-copyable/non-movable.It owns FFI storage released in the destructor and stores
&length_resolution_context.value()intoffi_context, so any copy or move silently produces a dangling interior pointer plus a double release. All current uses are local named objects, but the type should enforce that.♻️ Proposed guard
struct CalcResolutionSnapshot { + AK_MAKE_NONCOPYABLE(CalcResolutionSnapshot); + AK_MAKE_NONMOVABLE(CalcResolutionSnapshot); + CalcResolutionSnapshot(StyleValueFFI::CalcNode const* root, CalculationContext const& calculation_context, CalculationResolutionContext const& resolution_context)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp` around lines 221 - 304, Make CalcResolutionSnapshot explicitly non-copyable and non-movable by deleting its copy constructor, copy assignment operator, move constructor, and move assignment operator. Keep the existing destructor and constructor behavior unchanged, ensuring local named instances remain usable while preventing unsafe ownership or interior-pointer duplication.Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp (1)
44-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hoisting the
adopt/adopt_optionalhelpers into a shared header.The same two lambdas are duplicated verbatim in
Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cpp(lines 59-68) and presumably in other FFI-data constructors. A shared inline helper (e.g. next toRustStyleValueHandle) would keep the retain/adopt contract in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp` around lines 44 - 53, Hoist the duplicated adopt and adopt_optional lambdas from the current constructor and BasicShapeStyleValue into a shared inline helper near RustStyleValueHandle. Update both call sites to use the shared helpers while preserving their retain/adopt behavior and nullable handling, and reuse the helpers in other matching FFI-data constructors where applicable.Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h (1)
32-51: 📐 Maintainability & Code Quality | 🔵 TrivialCorrect, but duplicates a retain-or-null helper seen elsewhere.
Parameter order into
rust_style_value_create_color_mixmatches the Rust signature. The localretainlambda here duplicates the same nullable-retain pattern used inAbstractImageStyleValue.cpp; see consolidated comment.Also applies to: 66-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h` around lines 32 - 51, Replace the local retain lambda in make_color_mix_data with the existing shared nullable-retain helper used by AbstractImageStyleValue.cpp, preserving null handling and argument order for rust_style_value_create_color_mix. Apply the same deduplication to the related code at the constructor area around the additionally noted lines.Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp (1)
33-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract a shared helper for the "retain + adopt" FFI child-value idiom.
The same three-step sequence —
static_cast<StyleValueFFI::StyleValueData const*>(ptr)→StyleValueFFI::rust_style_value_retain(...)→StyleValue::adopt_rust_style_value_data(...)— is copy-pasted (with and without a null-check wrapper) across every subclass reconstructing a child value from FFI data. Centralizing this in one or two static helpers onStyleValue(nullable + non-null variants) would remove the duplication and reduce the risk of a call site omitting the retain call (dangling Rust data) or the null check (crash on an optional field) as this pattern keeps getting copied to new subclasses.
Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp#L33-L46: replace both inline lambdas with calls to a sharedStyleValue::adopt_rust_optional_child(void const*)helper.Libraries/LibWeb/CSS/StyleValues/OpacityValueStyleValue.h#L38-L38: replace with a sharedStyleValue::adopt_rust_child(void const*)(non-null) helper call.Libraries/LibWeb/CSS/StyleValues/FunctionStyleValue.h#L34-L34: replace with the same non-null helper.Libraries/LibWeb/CSS/StyleValues/SuperellipseStyleValue.h#L40-L40: replace with the same non-null helper.Libraries/LibWeb/CSS/StyleValues/RadialSizeStyleValue.h#L67-L74: replace bothm_value_0/m_value_1initializations with the nullable helper.Libraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.cpp#L20-L21: replace the loop body's inline null-check/retain/adopt with the nullable helper.Libraries/LibWeb/CSS/StyleValues/StyleValueList.h#L74-L75: replace the loop body's retain/adopt with the non-null helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp` around lines 33 - 46, Extract shared StyleValue helpers named adopt_rust_child(void const*) for required FFI children and adopt_rust_optional_child(void const*) for nullable children, preserving retain-before-adopt and null handling. Update Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp lines 33-46, OpacityValueStyleValue.h line 38, FunctionStyleValue.h line 34, SuperellipseStyleValue.h line 40, RadialSizeStyleValue.h lines 67-74, CounterDefinitionsStyleValue.cpp lines 20-21, and StyleValueList.h lines 74-75 to use the appropriate helper; both ConicGradient lambdas use the nullable helper, and each listed non-null or nullable site follows its indicated variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp`:
- Around line 221-304: Make CalcResolutionSnapshot explicitly non-copyable and
non-movable by deleting its copy constructor, copy assignment operator, move
constructor, and move assignment operator. Keep the existing destructor and
constructor behavior unchanged, ensuring local named instances remain usable
while preventing unsafe ownership or interior-pointer duplication.
In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cpp`:
- Around line 429-432: Deduplicate default Oklab interpolation-method creation
in ColorMixStyleValue.cpp by adding a file-local
default_color_interpolation_method() helper returning the shared StyleValue
reference. Update both to_color() at lines 429-432 and absolutized() at lines
460-463 to use this helper and pass the resulting reference to
interpolate_color_in_rust, including the call at line 486.
In `@Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h`:
- Around line 32-51: Replace the local retain lambda in make_color_mix_data with
the existing shared nullable-retain helper used by AbstractImageStyleValue.cpp,
preserving null handling and argument order for
rust_style_value_create_color_mix. Apply the same deduplication to the related
code at the constructor area around the additionally noted lines.
In `@Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp`:
- Around line 33-46: Extract shared StyleValue helpers named
adopt_rust_child(void const*) for required FFI children and
adopt_rust_optional_child(void const*) for nullable children, preserving
retain-before-adopt and null handling. Update
Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp lines 33-46,
OpacityValueStyleValue.h line 38, FunctionStyleValue.h line 34,
SuperellipseStyleValue.h line 40, RadialSizeStyleValue.h lines 67-74,
CounterDefinitionsStyleValue.cpp lines 20-21, and StyleValueList.h lines 74-75
to use the appropriate helper; both ConicGradient lambdas use the nullable
helper, and each listed non-null or nullable site follows its indicated variant.
In `@Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp`:
- Around line 44-53: Hoist the duplicated adopt and adopt_optional lambdas from
the current constructor and BasicShapeStyleValue into a shared inline helper
near RustStyleValueHandle. Update both call sites to use the shared helpers
while preserving their retain/adopt behavior and nullable handling, and reuse
the helpers in other matching FFI-data constructors where applicable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1eb2c02a-0ef5-430a-b545-79860c9f3af1
📒 Files selected for processing (216)
Documentation/CSSGeneratedFiles.mdLibraries/LibWeb/Animations/Animation.cppLibraries/LibWeb/Animations/Animation.hLibraries/LibWeb/Animations/AnimationEffect.cppLibraries/LibWeb/Animations/AnimationEffect.hLibraries/LibWeb/Animations/KeyframeEffect.cppLibraries/LibWeb/Animations/KeyframeEffect.hLibraries/LibWeb/CMakeLists.txtLibraries/LibWeb/CSS/CSSCounterStyleRule.hLibraries/LibWeb/CSS/CSSImportRule.cppLibraries/LibWeb/CSS/CSSImportRule.hLibraries/LibWeb/CSS/CSSNamespaceRule.hLibraries/LibWeb/CSS/CSSPropertyRule.hLibraries/LibWeb/CSS/CSSScopeRule.cppLibraries/LibWeb/CSS/CSSScopeRule.hLibraries/LibWeb/CSS/CSSStyleProperties.cppLibraries/LibWeb/CSS/CSSStyleProperties.hLibraries/LibWeb/CSS/CSSTransition.cppLibraries/LibWeb/CSS/CSSTransition.hLibraries/LibWeb/CSS/CascadedProperties.cppLibraries/LibWeb/CSS/CascadedProperties.hLibraries/LibWeb/CSS/ColorInterpolation.cppLibraries/LibWeb/CSS/ColorInterpolation.hLibraries/LibWeb/CSS/ComputedProperties.cppLibraries/LibWeb/CSS/ComputedProperties.hLibraries/LibWeb/CSS/ComputedValues.cppLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/ContainerQuery.hLibraries/LibWeb/CSS/CustomPropertyData.cppLibraries/LibWeb/CSS/EasingFunction.cppLibraries/LibWeb/CSS/GridTrackPlacement.hLibraries/LibWeb/CSS/GridTrackSize.cppLibraries/LibWeb/CSS/GridTrackSize.hLibraries/LibWeb/CSS/Interpolation.cppLibraries/LibWeb/CSS/Interpolation.hLibraries/LibWeb/CSS/InvalidationSet.hLibraries/LibWeb/CSS/Length.hLibraries/LibWeb/CSS/Parser/Parser.cppLibraries/LibWeb/CSS/Parser/Parser.hLibraries/LibWeb/CSS/Parser/Types.cppLibraries/LibWeb/CSS/Parser/Types.hLibraries/LibWeb/CSS/Parser/ValueParsing.cppLibraries/LibWeb/CSS/PercentageOr.hLibraries/LibWeb/CSS/PreferredContrast.cppLibraries/LibWeb/CSS/PreferredContrast.hLibraries/LibWeb/CSS/PreferredMotion.cppLibraries/LibWeb/CSS/PreferredMotion.hLibraries/LibWeb/CSS/PseudoElementPropertyGroups.txtLibraries/LibWeb/CSS/Rust/build.rsLibraries/LibWeb/CSS/Rust/src/animation.rsLibraries/LibWeb/CSS/Rust/src/calc.rsLibraries/LibWeb/CSS/Rust/src/cascaded_properties.rsLibraries/LibWeb/CSS/Rust/src/color_conversion.rsLibraries/LibWeb/CSS/Rust/src/color_interpolation.rsLibraries/LibWeb/CSS/Rust/src/computed_values.rsLibraries/LibWeb/CSS/Rust/src/custom_properties.rsLibraries/LibWeb/CSS/Rust/src/ffi_stats.rsLibraries/LibWeb/CSS/Rust/src/lib.rsLibraries/LibWeb/CSS/Rust/src/property_metadata.rsLibraries/LibWeb/CSS/Rust/src/style_compute.rsLibraries/LibWeb/CSS/Rust/src/style_value.rsLibraries/LibWeb/CSS/Rust/src/transition.rsLibraries/LibWeb/CSS/RustStyleBridge.cppLibraries/LibWeb/CSS/RustStyleBridge.hLibraries/LibWeb/CSS/Serialize.cppLibraries/LibWeb/CSS/Serialize.hLibraries/LibWeb/CSS/Size.cppLibraries/LibWeb/CSS/Size.hLibraries/LibWeb/CSS/StyleComputer.cppLibraries/LibWeb/CSS/StyleComputer.hLibraries/LibWeb/CSS/StyleScope.cppLibraries/LibWeb/CSS/StyleScope.hLibraries/LibWeb/CSS/StyleStructRef.hLibraries/LibWeb/CSS/StyleValues/AbstractImageStyleValue.cppLibraries/LibWeb/CSS/StyleValues/AnchorSizeStyleValue.cppLibraries/LibWeb/CSS/StyleValues/AnchorSizeStyleValue.hLibraries/LibWeb/CSS/StyleValues/AnchorStyleValue.cppLibraries/LibWeb/CSS/StyleValues/AnchorStyleValue.hLibraries/LibWeb/CSS/StyleValues/AngleStyleValue.hLibraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.cppLibraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.hLibraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cppLibraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.hLibraries/LibWeb/CSS/StyleValues/BorderImageSliceStyleValue.hLibraries/LibWeb/CSS/StyleValues/BorderRadiusRectStyleValue.hLibraries/LibWeb/CSS/StyleValues/BorderRadiusStyleValue.hLibraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorInterpolationMethodStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorSchemeStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorStyleValue.hLibraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/ContentStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ContentStyleValue.hLibraries/LibWeb/CSS/StyleValues/ContrastColorStyleValue.hLibraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.hLibraries/LibWeb/CSS/StyleValues/CounterStyleStyleValue.hLibraries/LibWeb/CSS/StyleValues/CounterStyleSystemStyleValue.hLibraries/LibWeb/CSS/StyleValues/CounterStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CounterStyleValue.hLibraries/LibWeb/CSS/StyleValues/CursorStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CursorStyleValue.hLibraries/LibWeb/CSS/StyleValues/CustomIdentStyleValue.hLibraries/LibWeb/CSS/StyleValues/DimensionStyleValue.hLibraries/LibWeb/CSS/StyleValues/DisplayStyleValue.hLibraries/LibWeb/CSS/StyleValues/EasingStyleValue.cppLibraries/LibWeb/CSS/StyleValues/EasingStyleValue.hLibraries/LibWeb/CSS/StyleValues/EdgeStyleValue.hLibraries/LibWeb/CSS/StyleValues/EmptyOptionalStyleValue.hLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.hLibraries/LibWeb/CSS/StyleValues/FlexStyleValue.hLibraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.hLibraries/LibWeb/CSS/StyleValues/FontStyleStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FontStyleStyleValue.hLibraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.hLibraries/LibWeb/CSS/StyleValues/FunctionStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridAutoFlowStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTemplateAreaStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTrackPlacementStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cppLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.hLibraries/LibWeb/CSS/StyleValues/GuaranteedInvalidStyleValue.hLibraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.hLibraries/LibWeb/CSS/StyleValues/ImageStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ImageStyleValue.hLibraries/LibWeb/CSS/StyleValues/IntegerStyleValue.hLibraries/LibWeb/CSS/StyleValues/KeywordStyleValue.hLibraries/LibWeb/CSS/StyleValues/LengthStyleValue.hLibraries/LibWeb/CSS/StyleValues/LightDarkStyleValue.hLibraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/NumberStyleValue.hLibraries/LibWeb/CSS/StyleValues/OpacityValueStyleValue.hLibraries/LibWeb/CSS/StyleValues/OpenTypeTaggedStyleValue.hLibraries/LibWeb/CSS/StyleValues/OverflowClipMarginStyleValue.cppLibraries/LibWeb/CSS/StyleValues/OverflowClipMarginStyleValue.hLibraries/LibWeb/CSS/StyleValues/PendingSubstitutionStyleValue.hLibraries/LibWeb/CSS/StyleValues/PercentageStyleValue.hLibraries/LibWeb/CSS/StyleValues/PositionStyleValue.hLibraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/RadialSizeStyleValue.hLibraries/LibWeb/CSS/StyleValues/RandomValueSharingStyleValue.hLibraries/LibWeb/CSS/StyleValues/RatioStyleValue.hLibraries/LibWeb/CSS/StyleValues/RectStyleValue.hLibraries/LibWeb/CSS/StyleValues/RepeatStyleStyleValue.hLibraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.hLibraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.hLibraries/LibWeb/CSS/StyleValues/ScrollbarColorStyleValue.hLibraries/LibWeb/CSS/StyleValues/ScrollbarGutterStyleValue.hLibraries/LibWeb/CSS/StyleValues/ShadowStyleValue.hLibraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.hLibraries/LibWeb/CSS/StyleValues/StringStyleValue.hLibraries/LibWeb/CSS/StyleValues/StyleValue.cppLibraries/LibWeb/CSS/StyleValues/StyleValue.hLibraries/LibWeb/CSS/StyleValues/StyleValueList.hLibraries/LibWeb/CSS/StyleValues/SuperellipseStyleValue.hLibraries/LibWeb/CSS/StyleValues/TextIndentStyleValue.cppLibraries/LibWeb/CSS/StyleValues/TextIndentStyleValue.hLibraries/LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.hLibraries/LibWeb/CSS/StyleValues/TimeStyleValue.hLibraries/LibWeb/CSS/StyleValues/TransformationStyleValue.hLibraries/LibWeb/CSS/StyleValues/TreeCountingFunctionStyleValue.hLibraries/LibWeb/CSS/StyleValues/TupleStyleValue.hLibraries/LibWeb/CSS/StyleValues/URLStyleValue.hLibraries/LibWeb/CSS/StyleValues/UnicodeRangeStyleValue.hLibraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.cppLibraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.hLibraries/LibWeb/Layout/FlexFormattingContext.cppLibraries/LibWeb/Layout/GridFormattingContext.hMeta/Generators/generate_libweb_css_property_id.pyMeta/Generators/generate_libweb_css_pseudo_element.pyMeta/StyleFfiBaseline/animation-transition.htmlTests/LibWeb/TestStylePropertyMetadataParity.cppTests/LibWeb/TestStyleValueEquality.cppTests/LibWeb/Text/expected/css/animation-css-wide-keywords.txtTests/LibWeb/Text/expected/css/animation-effect-batch-rust.txtTests/LibWeb/Text/expected/css/animation-important-suppression.txtTests/LibWeb/Text/expected/css/animation-keyframe-conflict-preference.txtTests/LibWeb/Text/expected/css/calculated-animation-rust.txtTests/LibWeb/Text/expected/css/discrete-animation-rust.txtTests/LibWeb/Text/expected/css/filter-animation-rust.txtTests/LibWeb/Text/expected/css/modern-color-animation-rust.txtTests/LibWeb/Text/expected/css/repeatable-list-animation-rust.txtTests/LibWeb/Text/expected/css/shadow-animation-rust.txtTests/LibWeb/Text/expected/css/transition-effect-batch-rust.txtTests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/animations/box-shadow-interpolation.txtTests/LibWeb/Text/expected/wpt-import/css/css-masking/animations/clip-path-composition.txtTests/LibWeb/Text/expected/wpt-import/css/css-shapes/animation/shape-outside-composition.txtTests/LibWeb/Text/expected/wpt-import/css/css-transforms/animation/transform-interpolation-computed-value.txtTests/LibWeb/Text/expected/wpt-import/css/css-transitions/animations/text-shadow-interpolation.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-composition-001.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-interpolation-004.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/filter-interpolation-004.txtTests/LibWeb/Text/input/css-placeholder-transition.htmlTests/LibWeb/Text/input/css/animation-css-wide-keywords.htmlTests/LibWeb/Text/input/css/animation-effect-batch-rust.htmlTests/LibWeb/Text/input/css/animation-important-suppression.htmlTests/LibWeb/Text/input/css/animation-keyframe-conflict-preference.htmlTests/LibWeb/Text/input/css/calculated-animation-rust.htmlTests/LibWeb/Text/input/css/discrete-animation-rust.htmlTests/LibWeb/Text/input/css/filter-animation-rust.htmlTests/LibWeb/Text/input/css/modern-color-animation-rust.htmlTests/LibWeb/Text/input/css/repeatable-list-animation-rust.htmlTests/LibWeb/Text/input/css/shadow-animation-rust.htmlTests/LibWeb/Text/input/css/transition-effect-batch-rust.html
💤 Files with no reviewable changes (36)
- Libraries/LibWeb/CSS/CSSNamespaceRule.h
- Libraries/LibWeb/CSS/PreferredContrast.h
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.cpp
- Libraries/LibWeb/CSS/GridTrackSize.cpp
- Libraries/LibWeb/CSS/Serialize.h
- Libraries/LibWeb/CSS/PreferredContrast.cpp
- Libraries/LibWeb/CSS/InvalidationSet.h
- Libraries/LibWeb/CSS/CSSScopeRule.h
- Libraries/LibWeb/CSS/CSSTransition.h
- Libraries/LibWeb/CSS/GridTrackPlacement.h
- Libraries/LibWeb/CSS/Parser/Types.h
- Libraries/LibWeb/CSS/ColorInterpolation.h
- Libraries/LibWeb/CSS/StyleScope.cpp
- Libraries/LibWeb/CSS/Serialize.cpp
- Libraries/LibWeb/CSS/PreferredMotion.cpp
- Libraries/LibWeb/CSS/Parser/Parser.cpp
- Libraries/LibWeb/CSS/CSSImportRule.h
- Libraries/LibWeb/CSS/CSSStyleProperties.h
- Libraries/LibWeb/CSS/ContainerQuery.h
- Libraries/LibWeb/CSS/ColorInterpolation.cpp
- Libraries/LibWeb/CSS/Interpolation.h
- Libraries/LibWeb/CSS/StyleScope.h
- Libraries/LibWeb/CSS/CSSImportRule.cpp
- Libraries/LibWeb/CSS/Length.h
- Libraries/LibWeb/CSS/Size.cpp
- Libraries/LibWeb/CSS/ComputedProperties.h
- Libraries/LibWeb/CSS/GridTrackSize.h
- Libraries/LibWeb/CSS/PreferredMotion.h
- Libraries/LibWeb/CSS/CSSCounterStyleRule.h
- Libraries/LibWeb/CSS/Parser/ValueParsing.cpp
- Libraries/LibWeb/CSS/CSSPropertyRule.h
- Libraries/LibWeb/CSS/Parser/Types.cpp
- Libraries/LibWeb/CMakeLists.txt
- Meta/Generators/generate_libweb_css_pseudo_element.py
- Libraries/LibWeb/CSS/Parser/Parser.h
- Libraries/LibWeb/CSS/CSSScopeRule.cpp
🚧 Files skipped from review as they are similar to previous changes (147)
- Tests/LibWeb/Text/expected/css/repeatable-list-animation-rust.txt
- Tests/LibWeb/Text/input/css/filter-animation-rust.html
- Libraries/LibWeb/CSS/PseudoElementPropertyGroups.txt
- Tests/LibWeb/Text/input/css/calculated-animation-rust.html
- Tests/LibWeb/Text/input/css/animation-css-wide-keywords.html
- Tests/LibWeb/Text/expected/css/transition-effect-batch-rust.txt
- Tests/LibWeb/Text/expected/css/modern-color-animation-rust.txt
- Libraries/LibWeb/CSS/StyleValues/FontStyleStyleValue.cpp
- Tests/LibWeb/Text/expected/css/animation-effect-batch-rust.txt
- Tests/LibWeb/Text/expected/css/discrete-animation-rust.txt
- Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp
- Tests/LibWeb/Text/input/css/repeatable-list-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/RepeatStyleStyleValue.h
- Meta/StyleFfiBaseline/animation-transition.html
- Tests/LibWeb/Text/input/css/animation-keyframe-conflict-preference.html
- Tests/LibWeb/Text/input/css/discrete-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/UnicodeRangeStyleValue.h
- Tests/LibWeb/Text/expected/css/shadow-animation-rust.txt
- Documentation/CSSGeneratedFiles.md
- Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ColorSchemeStyleValue.h
- Tests/LibWeb/Text/input/css/animation-important-suppression.html
- Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/DimensionStyleValue.h
- Tests/LibWeb/Text/expected/css/filter-animation-rust.txt
- Libraries/LibWeb/CSS/StyleValues/AnchorSizeStyleValue.cpp
- Tests/LibWeb/Text/expected/css/animation-keyframe-conflict-preference.txt
- Tests/LibWeb/Text/expected/css/animation-css-wide-keywords.txt
- Tests/LibWeb/Text/input/css/shadow-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h
- Tests/LibWeb/Text/expected/css/calculated-animation-rust.txt
- Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.cpp
- Tests/LibWeb/Text/input/css/transition-effect-batch-rust.html
- Tests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-interpolation-004.txt
- Libraries/LibWeb/CSS/StyleValues/DisplayStyleValue.h
- Tests/LibWeb/Text/expected/css/animation-important-suppression.txt
- Libraries/LibWeb/CSS/StyleValues/GuaranteedInvalidStyleValue.h
- Tests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/filter-interpolation-004.txt
- Libraries/LibWeb/CSS/StyleValues/RectStyleValue.h
- Libraries/LibWeb/Animations/AnimationEffect.cpp
- Libraries/LibWeb/CSS/RustStyleBridge.h
- Libraries/LibWeb/CSS/StyleValues/KeywordStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/BackgroundSizeStyleValue.h
- Libraries/LibWeb/CSS/StyleStructRef.h
- Libraries/LibWeb/CSS/StyleValues/NumberStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.h
- Tests/LibWeb/Text/input/css/animation-effect-batch-rust.html
- Libraries/LibWeb/CSS/StyleValues/AnchorStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/OverflowClipMarginStyleValue.cpp
- Tests/LibWeb/Text/input/css/modern-color-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/EmptyOptionalStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/FontStyleStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/PositionStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/GridAutoFlowStyleValue.h
- Libraries/LibWeb/Animations/KeyframeEffect.h
- Libraries/LibWeb/CSS/StyleValues/TextIndentStyleValue.cpp
- Libraries/LibWeb/Layout/GridFormattingContext.h
- Libraries/LibWeb/CSS/StyleValues/AnchorSizeStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/CounterStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/EdgeStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/BorderRadiusRectStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/RandomValueSharingStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ScrollbarGutterStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/IntegerStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ContentStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/GridTemplateAreaStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/StringStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.h
- Tests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-composition-001.txt
- Libraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.h
- Libraries/LibWeb/CSS/StyleValues/TupleStyleValue.h
- Libraries/LibWeb/Animations/AnimationEffect.h
- Libraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ContentStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/CounterStyleStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/PercentageStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/RatioStyleValue.h
- Tests/LibWeb/Text/expected/wpt-import/css/css-shapes/animation/shape-outside-composition.txt
- Libraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ColorInterpolationMethodStyleValue.h
- Tests/LibWeb/TestStylePropertyMetadataParity.cpp
- Libraries/LibWeb/CSS/StyleValues/BorderRadiusStyleValue.h
- Libraries/LibWeb/CSS/CustomPropertyData.cpp
- Libraries/LibWeb/Layout/FlexFormattingContext.cpp
- Libraries/LibWeb/CSS/StyleValues/OpenTypeTaggedStyleValue.h
- Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/animations/box-shadow-interpolation.txt
- Libraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/CounterStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/OverflowClipMarginStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/LightDarkStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.cpp
- Libraries/LibWeb/CSS/Rust/src/transition.rs
- Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/TreeCountingFunctionStyleValue.h
- Libraries/LibWeb/CSS/CSSStyleProperties.cpp
- Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/animations/text-shadow-interpolation.txt
- Libraries/LibWeb/CSS/StyleValues/CursorStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ContrastColorStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/StyleValue.h
- Libraries/LibWeb/Animations/Animation.cpp
- Libraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/TextIndentStyleValue.h
- Libraries/LibWeb/Animations/KeyframeEffect.cpp
- Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs
- Libraries/LibWeb/CSS/StyleValues/GridTrackPlacementStyleValue.h
- Libraries/LibWeb/Animations/Animation.h
- Libraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/CounterStyleSystemStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/PendingSubstitutionStyleValue.h
- Libraries/LibWeb/CSS/StyleComputer.h
- Libraries/LibWeb/CSS/StyleValues/CursorStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/property_metadata.rs
- Libraries/LibWeb/CSS/RustStyleBridge.cpp
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/ffi_stats.rs
- Libraries/LibWeb/CSS/StyleValues/ShadowStyleValue.h
- Libraries/LibWeb/CSS/PercentageOr.h
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cpp
- Libraries/LibWeb/CSS/Rust/src/custom_properties.rs
- Libraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ScrollbarColorStyleValue.h
- Libraries/LibWeb/CSS/Size.h
- Libraries/LibWeb/CSS/ComputedProperties.cpp
- Libraries/LibWeb/CSS/Rust/build.rs
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/animation.rs
- Libraries/LibWeb/CSS/StyleValues/StyleValue.cpp
- Libraries/LibWeb/CSS/Rust/src/color_conversion.rs
- Libraries/LibWeb/CSS/Rust/src/style_value.rs
- Libraries/LibWeb/CSS/ComputedValues.cpp
- Libraries/LibWeb/CSS/ComputedValues.h
- Libraries/LibWeb/CSS/StyleComputer.cpp
- Libraries/LibWeb/CSS/Rust/src/computed_values.rs
- Libraries/LibWeb/CSS/Rust/src/cascaded_properties.rs
- Libraries/LibWeb/CSS/Rust/src/calc.rs
- Libraries/LibWeb/CSS/Rust/src/style_compute.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/LibWeb/TestStyleValueEquality.cpp (1)
1308-1316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a helper for the repeated
FfiAnimationContextliteral.The full
FfiAnimationContextaggregate is duplicated verbatim across 7 test cases, differing only inallow_discrete(and, once, the transform reference box). Since this struct just gained new fields (current_color,has_length_resolution_context,length_resolution_context) in this PR, duplicating the literal means a future field addition can be silently zero/default-initialized at any site that isn't manually updated, since designated-initializer omissions don't fail to compile.A small factory reduces that risk and the boilerplate:
♻️ Suggested helper
static StyleValueFFI::FfiAnimationContext make_animation_context(bool allow_discrete, bool has_transform_reference_box = false, double reference_box_width = 0, double reference_box_height = 0) { return { .allow_discrete = allow_discrete, .current_color = nullptr, .has_length_resolution_context = false, .length_resolution_context = {}, .has_transform_reference_box = has_transform_reference_box, .transform_reference_box_width = reference_box_width, .transform_reference_box_height = reference_box_height, }; }Also applies to: 1333-1341, 1379-1387, 1428-1436, 2090-2098, 2389-2397, 2441-2449
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/TestStyleValueEquality.cpp` around lines 1308 - 1316, Extract a shared make_animation_context helper in the test file that initializes every FfiAnimationContext field, accepting allow_discrete and optional transform reference-box settings. Replace all seven duplicated FfiAnimationContext literals with calls to this helper, preserving each test’s existing values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/LibWeb/TestStyleValueEquality.cpp`:
- Around line 1308-1316: Extract a shared make_animation_context helper in the
test file that initializes every FfiAnimationContext field, accepting
allow_discrete and optional transform reference-box settings. Replace all seven
duplicated FfiAnimationContext literals with calls to this helper, preserving
each test’s existing values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ac2e0f6-bdac-4771-94fe-882491caf3c6
📒 Files selected for processing (74)
Documentation/CSSGeneratedFiles.mdLibraries/LibWeb/CMakeLists.txtLibraries/LibWeb/CSS/CSSCounterStyleRule.hLibraries/LibWeb/CSS/CSSImportRule.cppLibraries/LibWeb/CSS/CSSImportRule.hLibraries/LibWeb/CSS/CSSNamespaceRule.hLibraries/LibWeb/CSS/CSSPropertyRule.hLibraries/LibWeb/CSS/CSSScopeRule.cppLibraries/LibWeb/CSS/CSSScopeRule.hLibraries/LibWeb/CSS/CSSStyleProperties.cppLibraries/LibWeb/CSS/CSSStyleProperties.hLibraries/LibWeb/CSS/CSSTransition.hLibraries/LibWeb/CSS/ComputedProperties.cppLibraries/LibWeb/CSS/ComputedProperties.hLibraries/LibWeb/CSS/ComputedValues.cppLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/ContainerQuery.hLibraries/LibWeb/CSS/GridTrackPlacement.hLibraries/LibWeb/CSS/GridTrackSize.cppLibraries/LibWeb/CSS/GridTrackSize.hLibraries/LibWeb/CSS/InvalidationSet.hLibraries/LibWeb/CSS/Length.hLibraries/LibWeb/CSS/Parser/Parser.cppLibraries/LibWeb/CSS/Parser/Parser.hLibraries/LibWeb/CSS/Parser/Types.cppLibraries/LibWeb/CSS/Parser/Types.hLibraries/LibWeb/CSS/Parser/ValueParsing.cppLibraries/LibWeb/CSS/PercentageOr.hLibraries/LibWeb/CSS/PreferredContrast.cppLibraries/LibWeb/CSS/PreferredContrast.hLibraries/LibWeb/CSS/PreferredMotion.cppLibraries/LibWeb/CSS/PreferredMotion.hLibraries/LibWeb/CSS/PseudoElementPropertyGroups.txtLibraries/LibWeb/CSS/Rust/build.rsLibraries/LibWeb/CSS/Rust/src/animation.rsLibraries/LibWeb/CSS/Rust/src/calc.rsLibraries/LibWeb/CSS/Rust/src/cascaded_properties.rsLibraries/LibWeb/CSS/Rust/src/computed_values.rsLibraries/LibWeb/CSS/Rust/src/ffi_stats.rsLibraries/LibWeb/CSS/Rust/src/property_metadata.rsLibraries/LibWeb/CSS/Rust/src/style_compute.rsLibraries/LibWeb/CSS/Rust/src/style_value.rsLibraries/LibWeb/CSS/Rust/src/transition.rsLibraries/LibWeb/CSS/Serialize.cppLibraries/LibWeb/CSS/Serialize.hLibraries/LibWeb/CSS/Size.cppLibraries/LibWeb/CSS/Size.hLibraries/LibWeb/CSS/StyleComputer.cppLibraries/LibWeb/CSS/StyleComputer.hLibraries/LibWeb/CSS/StyleScope.cppLibraries/LibWeb/CSS/StyleScope.hLibraries/LibWeb/CSS/StyleStructRef.hLibraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cppLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.hLibraries/LibWeb/CSS/StyleValues/OpenTypeTaggedStyleValue.hLibraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.hLibraries/LibWeb/CSS/StyleValues/StyleValue.cppLibraries/LibWeb/CSS/StyleValues/StyleValue.hLibraries/LibWeb/Layout/FlexFormattingContext.cppLibraries/LibWeb/Layout/GridFormattingContext.hMeta/Generators/generate_libweb_css_pseudo_element.pyTests/LibWeb/TestStyleValueEquality.cppTests/LibWeb/Text/expected/css/animation-css-wide-keywords.txtTests/LibWeb/Text/expected/css/animation-effect-batch-rust.txtTests/LibWeb/Text/expected/css/animation-important-suppression.txtTests/LibWeb/Text/expected/css/transition-effect-batch-rust.txtTests/LibWeb/Text/input/css-placeholder-transition.htmlTests/LibWeb/Text/input/css/animation-css-wide-keywords.htmlTests/LibWeb/Text/input/css/animation-effect-batch-rust.htmlTests/LibWeb/Text/input/css/animation-important-suppression.htmlTests/LibWeb/Text/input/css/transition-effect-batch-rust.html
💤 Files with no reviewable changes (42)
- Tests/LibWeb/Text/expected/css/transition-effect-batch-rust.txt
- Libraries/LibWeb/CSS/PreferredContrast.cpp
- Libraries/LibWeb/CSS/Length.h
- Libraries/LibWeb/CSS/ContainerQuery.h
- Libraries/LibWeb/CSS/PreferredMotion.h
- Tests/LibWeb/Text/input/css/transition-effect-batch-rust.html
- Libraries/LibWeb/CSS/ComputedProperties.h
- Libraries/LibWeb/CSS/CSSScopeRule.cpp
- Tests/LibWeb/Text/input/css/animation-effect-batch-rust.html
- Tests/LibWeb/Text/expected/css/animation-effect-batch-rust.txt
- Libraries/LibWeb/CSS/StyleScope.cpp
- Libraries/LibWeb/CSS/CSSScopeRule.h
- Libraries/LibWeb/CSS/CSSPropertyRule.h
- Libraries/LibWeb/CSS/Serialize.h
- Libraries/LibWeb/CSS/PreferredContrast.h
- Libraries/LibWeb/CSS/CSSImportRule.cpp
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.cpp
- Libraries/LibWeb/CSS/InvalidationSet.h
- Libraries/LibWeb/CSS/CSSTransition.h
- Libraries/LibWeb/CSS/Parser/ValueParsing.cpp
- Libraries/LibWeb/CSS/GridTrackSize.cpp
- Libraries/LibWeb/CSS/Size.cpp
- Libraries/LibWeb/CSS/CSSStyleProperties.h
- Libraries/LibWeb/CSS/Parser/Parser.h
- Libraries/LibWeb/CSS/CSSCounterStyleRule.h
- Libraries/LibWeb/CSS/PreferredMotion.cpp
- Libraries/LibWeb/CSS/CSSNamespaceRule.h
- Libraries/LibWeb/CSS/GridTrackSize.h
- Libraries/LibWeb/CSS/Parser/Types.cpp
- Libraries/LibWeb/CSS/Serialize.cpp
- Libraries/LibWeb/CSS/Parser/Types.h
- Libraries/LibWeb/CSS/CSSImportRule.h
- Libraries/LibWeb/CMakeLists.txt
- Libraries/LibWeb/CSS/StyleScope.h
- Libraries/LibWeb/CSS/Parser/Parser.cpp
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.h
- Libraries/LibWeb/CSS/GridTrackPlacement.h
- Libraries/LibWeb/CSS/StyleValues/StyleValue.h
- Libraries/LibWeb/CSS/StyleComputer.h
- Meta/Generators/generate_libweb_css_pseudo_element.py
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.h
- Libraries/LibWeb/CSS/ComputedProperties.cpp
🚧 Files skipped from review as they are similar to previous changes (29)
- Tests/LibWeb/Text/input/css/animation-important-suppression.html
- Tests/LibWeb/Text/input/css/animation-css-wide-keywords.html
- Tests/LibWeb/Text/expected/css/animation-css-wide-keywords.txt
- Tests/LibWeb/Text/expected/css/animation-important-suppression.txt
- Tests/LibWeb/Text/input/css-placeholder-transition.html
- Libraries/LibWeb/CSS/PseudoElementPropertyGroups.txt
- Libraries/LibWeb/CSS/StyleValues/OpenTypeTaggedStyleValue.h
- Libraries/LibWeb/Layout/FlexFormattingContext.cpp
- Libraries/LibWeb/CSS/Rust/src/transition.rs
- Libraries/LibWeb/CSS/StyleStructRef.h
- Libraries/LibWeb/CSS/Rust/src/ffi_stats.rs
- Libraries/LibWeb/CSS/CSSStyleProperties.cpp
- Documentation/CSSGeneratedFiles.md
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/StyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.h
- Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp
- Libraries/LibWeb/CSS/PercentageOr.h
- Libraries/LibWeb/CSS/Rust/build.rs
- Libraries/LibWeb/CSS/Size.h
- Libraries/LibWeb/CSS/Rust/src/animation.rs
- Libraries/LibWeb/CSS/Rust/src/computed_values.rs
- Libraries/LibWeb/CSS/StyleComputer.cpp
- Libraries/LibWeb/CSS/ComputedValues.cpp
- Libraries/LibWeb/CSS/Rust/src/style_value.rs
- Libraries/LibWeb/CSS/Rust/src/cascaded_properties.rs
- Libraries/LibWeb/CSS/Rust/src/style_compute.rs
- Libraries/LibWeb/CSS/ComputedValues.h
- Libraries/LibWeb/CSS/Rust/src/calc.rs
Move shapes, shadows, filters, easing values, color functions, gradients, grid tracks, images, calculations, custom properties, cascaded values, and computed values onto the shared Rust value graph.
Implement visibility, display, font variation, stroke dash array, and individual transform interpolation in Rust. Store calculated numeric types explicitly and remove the corresponding C++ transform and ratio fallbacks.
Evaluate easing descriptors, keyframe-local easing, property intervals, and resolved animation values in batched Rust evaluation. Move the CSS Transitions decision algorithm to Rust while C++ executes returned actions.
Implement grid tracks, basic shapes, legacy and modern colors, shadows, and filters in Rust. Preserve missing color components and composition behavior for compound filter lists.
Send all effects for an element through one animation batch. Keep discrete and unsupported custom decisions in Rust and normalize repeatable lists without crossings proportional to properties or list children.
Interpolate and compose length-percentage and general calculated values in Rust. Own discrete and unsupported composition decisions, missing keyframe values, transitionability, and newly started transition evaluation in Rust. Remove the C++ interpolation, composition, and animation-value fallbacks.
Represent initial values, shorthand expansion, declarations, custom properties, and longhand inputs with Rust handles instead of shell and data pairs. Remove obsolete C++ style-value shell transfer interfaces.
Move color interpolation and transition value comparison to Rust. Generate deterministic conflict metadata and resolve logical, physical, shorthand, and longhand keyframe declarations inside Rust animation preparation.
Resolve animated CSS-wide keywords, expand shorthands in one batch, suppress important properties, and drive animation preparation from Rust. Preserve batched C++ computation only for remaining general longhand work.
Delete interpolation helpers, accessors, and parser entry points made unused by Rust animation and value ownership. Keep calculation equality in Rust.
Return transition actions and animation overlays as owned FFI results. Remove the Rust-to-C++ callbacks previously used to deliver both batches.
Detect calculation anchors in Rust and batch Typed OM descriptions, serialization pieces, and external calculation resolution. Replace repeated Rust-to-C++ calculation calls with coarse operation results.
Batch and unify the external actions required by longhand computation. Compute OpenType tag lists in Rust and remove remaining callbacks from the computationally independent style-computation path.
Filter pseudo-element properties and own simple computed-style groups in Rust. Inline the computed size facade and move sizing values onto the Rust computed-value representation.
Own computed alignment, SVG reset, and surround values in Rust. Compute position-area values there as well, further reducing C++ computed-style storage and Rust-to-C++ style-computation seams.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Libraries/LibWeb/CSS/Rust/src/style_compute.rs (1)
1935-1941: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment doesn't cover
COMPUTED_KIND_STYLE_VALUE. The replacement for that kind travels incomputed_data, notvalue; worth a word so the FFI contract reads unambiguously.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/Rust/src/style_compute.rs` around lines 1935 - 1941, Update the documentation for the computed-value struct fields, especially computed_kind and value, to explicitly describe COMPUTED_KIND_STYLE_VALUE and state that its replacement is stored in computed_data rather than value. Keep the existing explanations for COMPUTED_KIND_UNCHANGED and the other kinds accurate and make the FFI storage contract unambiguous.Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp (1)
44-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hoisting the
retain/adopt/adopt_optionalhelpers into a shared header.The identical trio is now duplicated across the migrated subclasses (e.g.
BasicShapeStyleValue.cpplines 25-27 and 59-68). A small shared inline helper next toStyleValue::adopt_rust_style_value_datawould keep the retain/adopt contract in one place as more subclasses migrate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp` around lines 44 - 53, Move the duplicated retain/adopt helpers currently defined in EasingStyleValue.cpp into a shared header alongside StyleValue::adopt_rust_style_value_data, exposing reusable inline helpers for required and optional values. Update EasingStyleValue and the other migrated subclasses, including BasicShapeStyleValue, to use the shared helpers while preserving their existing pointer and null-handling behavior.Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cpp (1)
88-91: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
path()round-trips through serialize/re-parse on every construction.Rust stores the serialized instruction string, so every C++ facade rebuild re-runs
SVG::AttributeParser::parse_path_data. That is a non-trivial cost for shapes adopted repeatedly during animation/cascade, and re-parsing is also a potential fidelity risk if serialization ever normalizes differently than the original input. Consider caching the parsed path per data pointer if this shows up in profiles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cpp` around lines 88 - 91, Cache the parsed path for each underlying shape data pointer in the case 6 branch of BasicShapeStyleValue construction, reusing the cached Path on repeated facade creation instead of calling SVG::AttributeParser::parse_path_data each time. Preserve the existing fill rule and path parsing behavior, and ensure the cache lifetime safely tracks the referenced shape data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Libraries/LibWeb/CSS/Rust/src/style_compute.rs`:
- Around line 1935-1941: Update the documentation for the computed-value struct
fields, especially computed_kind and value, to explicitly describe
COMPUTED_KIND_STYLE_VALUE and state that its replacement is stored in
computed_data rather than value. Keep the existing explanations for
COMPUTED_KIND_UNCHANGED and the other kinds accurate and make the FFI storage
contract unambiguous.
In `@Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cpp`:
- Around line 88-91: Cache the parsed path for each underlying shape data
pointer in the case 6 branch of BasicShapeStyleValue construction, reusing the
cached Path on repeated facade creation instead of calling
SVG::AttributeParser::parse_path_data each time. Preserve the existing fill rule
and path parsing behavior, and ensure the cache lifetime safely tracks the
referenced shape data.
In `@Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.cpp`:
- Around line 44-53: Move the duplicated retain/adopt helpers currently defined
in EasingStyleValue.cpp into a shared header alongside
StyleValue::adopt_rust_style_value_data, exposing reusable inline helpers for
required and optional values. Update EasingStyleValue and the other migrated
subclasses, including BasicShapeStyleValue, to use the shared helpers while
preserving their existing pointer and null-handling behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 61529311-2a12-4878-beca-033dfd9d0cdd
📒 Files selected for processing (158)
Documentation/CSSGeneratedFiles.mdLibraries/LibWeb/Animations/Animation.cppLibraries/LibWeb/Animations/Animation.hLibraries/LibWeb/Animations/KeyframeEffect.cppLibraries/LibWeb/Animations/KeyframeEffect.hLibraries/LibWeb/CMakeLists.txtLibraries/LibWeb/CSS/CSSCounterStyleRule.hLibraries/LibWeb/CSS/CSSImportRule.cppLibraries/LibWeb/CSS/CSSImportRule.hLibraries/LibWeb/CSS/CSSNamespaceRule.hLibraries/LibWeb/CSS/CSSPropertyRule.hLibraries/LibWeb/CSS/CSSScopeRule.cppLibraries/LibWeb/CSS/CSSScopeRule.hLibraries/LibWeb/CSS/CSSStyleProperties.cppLibraries/LibWeb/CSS/CSSStyleProperties.hLibraries/LibWeb/CSS/CSSTransition.cppLibraries/LibWeb/CSS/CSSTransition.hLibraries/LibWeb/CSS/CascadedProperties.cppLibraries/LibWeb/CSS/CascadedProperties.hLibraries/LibWeb/CSS/ColorInterpolation.cppLibraries/LibWeb/CSS/ColorInterpolation.hLibraries/LibWeb/CSS/ComputedProperties.cppLibraries/LibWeb/CSS/ComputedProperties.hLibraries/LibWeb/CSS/ComputedValues.cppLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/ContainerQuery.hLibraries/LibWeb/CSS/CustomPropertyData.cppLibraries/LibWeb/CSS/EasingFunction.cppLibraries/LibWeb/CSS/GridTrackPlacement.hLibraries/LibWeb/CSS/GridTrackSize.cppLibraries/LibWeb/CSS/GridTrackSize.hLibraries/LibWeb/CSS/Interpolation.cppLibraries/LibWeb/CSS/Interpolation.hLibraries/LibWeb/CSS/InvalidationSet.hLibraries/LibWeb/CSS/Length.hLibraries/LibWeb/CSS/Parser/Parser.cppLibraries/LibWeb/CSS/Parser/Parser.hLibraries/LibWeb/CSS/Parser/Types.cppLibraries/LibWeb/CSS/Parser/Types.hLibraries/LibWeb/CSS/Parser/ValueParsing.cppLibraries/LibWeb/CSS/PercentageOr.hLibraries/LibWeb/CSS/PreferredContrast.cppLibraries/LibWeb/CSS/PreferredContrast.hLibraries/LibWeb/CSS/PreferredMotion.cppLibraries/LibWeb/CSS/PreferredMotion.hLibraries/LibWeb/CSS/PseudoElementPropertyGroups.txtLibraries/LibWeb/CSS/Rust/build.rsLibraries/LibWeb/CSS/Rust/src/animation.rsLibraries/LibWeb/CSS/Rust/src/calc.rsLibraries/LibWeb/CSS/Rust/src/cascaded_properties.rsLibraries/LibWeb/CSS/Rust/src/color_conversion.rsLibraries/LibWeb/CSS/Rust/src/color_interpolation.rsLibraries/LibWeb/CSS/Rust/src/computed_values.rsLibraries/LibWeb/CSS/Rust/src/custom_properties.rsLibraries/LibWeb/CSS/Rust/src/ffi_stats.rsLibraries/LibWeb/CSS/Rust/src/lib.rsLibraries/LibWeb/CSS/Rust/src/property_metadata.rsLibraries/LibWeb/CSS/Rust/src/style_compute.rsLibraries/LibWeb/CSS/Rust/src/style_value.rsLibraries/LibWeb/CSS/Rust/src/transition.rsLibraries/LibWeb/CSS/RustStyleBridge.cppLibraries/LibWeb/CSS/RustStyleBridge.hLibraries/LibWeb/CSS/Serialize.cppLibraries/LibWeb/CSS/Serialize.hLibraries/LibWeb/CSS/Size.cppLibraries/LibWeb/CSS/Size.hLibraries/LibWeb/CSS/StyleComputer.cppLibraries/LibWeb/CSS/StyleComputer.hLibraries/LibWeb/CSS/StyleScope.cppLibraries/LibWeb/CSS/StyleScope.hLibraries/LibWeb/CSS/StyleStructRef.hLibraries/LibWeb/CSS/StyleValues/AbstractImageStyleValue.cppLibraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cppLibraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.hLibraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ColorFunctionStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorInterpolationMethodStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.hLibraries/LibWeb/CSS/StyleValues/ColorSchemeStyleValue.hLibraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.hLibraries/LibWeb/CSS/StyleValues/CursorStyleValue.cppLibraries/LibWeb/CSS/StyleValues/CursorStyleValue.hLibraries/LibWeb/CSS/StyleValues/DisplayStyleValue.hLibraries/LibWeb/CSS/StyleValues/EasingStyleValue.cppLibraries/LibWeb/CSS/StyleValues/EasingStyleValue.hLibraries/LibWeb/CSS/StyleValues/EmptyOptionalStyleValue.hLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.cppLibraries/LibWeb/CSS/StyleValues/FilterStyleValue.hLibraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.cppLibraries/LibWeb/CSS/StyleValues/GridAutoFlowStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTemplateAreaStyleValue.hLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cppLibraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.hLibraries/LibWeb/CSS/StyleValues/GuaranteedInvalidStyleValue.hLibraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.hLibraries/LibWeb/CSS/StyleValues/ImageStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ImageStyleValue.hLibraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/OpenTypeTaggedStyleValue.hLibraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.cppLibraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.hLibraries/LibWeb/CSS/StyleValues/RadialSizeStyleValue.hLibraries/LibWeb/CSS/StyleValues/RepeatStyleStyleValue.hLibraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.hLibraries/LibWeb/CSS/StyleValues/ScrollbarGutterStyleValue.hLibraries/LibWeb/CSS/StyleValues/ShadowStyleValue.hLibraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cppLibraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.hLibraries/LibWeb/CSS/StyleValues/StyleValue.cppLibraries/LibWeb/CSS/StyleValues/StyleValue.hLibraries/LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.hLibraries/LibWeb/CSS/StyleValues/TreeCountingFunctionStyleValue.hLibraries/LibWeb/CSS/StyleValues/URLStyleValue.hLibraries/LibWeb/CSS/StyleValues/UnicodeRangeStyleValue.hLibraries/LibWeb/Layout/FlexFormattingContext.cppLibraries/LibWeb/Layout/GridFormattingContext.hMeta/Generators/generate_libweb_css_pseudo_element.pyTests/LibWeb/TestStylePropertyMetadataParity.cppTests/LibWeb/TestStyleValueEquality.cppTests/LibWeb/Text/expected/css/animation-css-wide-keywords.txtTests/LibWeb/Text/expected/css/animation-effect-batch-rust.txtTests/LibWeb/Text/expected/css/animation-important-suppression.txtTests/LibWeb/Text/expected/css/animation-keyframe-conflict-preference.txtTests/LibWeb/Text/expected/css/calculated-animation-rust.txtTests/LibWeb/Text/expected/css/discrete-animation-rust.txtTests/LibWeb/Text/expected/css/filter-animation-rust.txtTests/LibWeb/Text/expected/css/modern-color-animation-rust.txtTests/LibWeb/Text/expected/css/repeatable-list-animation-rust.txtTests/LibWeb/Text/expected/css/shadow-animation-rust.txtTests/LibWeb/Text/expected/css/transition-effect-batch-rust.txtTests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/animations/box-shadow-interpolation.txtTests/LibWeb/Text/expected/wpt-import/css/css-masking/animations/clip-path-composition.txtTests/LibWeb/Text/expected/wpt-import/css/css-shapes/animation/shape-outside-composition.txtTests/LibWeb/Text/expected/wpt-import/css/css-transforms/animation/transform-interpolation-computed-value.txtTests/LibWeb/Text/expected/wpt-import/css/css-transitions/animations/text-shadow-interpolation.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-composition-001.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-interpolation-004.txtTests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/filter-interpolation-004.txtTests/LibWeb/Text/input/css-placeholder-transition.htmlTests/LibWeb/Text/input/css/animation-css-wide-keywords.htmlTests/LibWeb/Text/input/css/animation-effect-batch-rust.htmlTests/LibWeb/Text/input/css/animation-important-suppression.htmlTests/LibWeb/Text/input/css/animation-keyframe-conflict-preference.htmlTests/LibWeb/Text/input/css/calculated-animation-rust.htmlTests/LibWeb/Text/input/css/discrete-animation-rust.htmlTests/LibWeb/Text/input/css/filter-animation-rust.htmlTests/LibWeb/Text/input/css/modern-color-animation-rust.htmlTests/LibWeb/Text/input/css/repeatable-list-animation-rust.htmlTests/LibWeb/Text/input/css/shadow-animation-rust.htmlTests/LibWeb/Text/input/css/transition-effect-batch-rust.html
💤 Files with no reviewable changes (36)
- Libraries/LibWeb/CSS/PreferredContrast.cpp
- Libraries/LibWeb/CSS/PreferredMotion.cpp
- Libraries/LibWeb/CSS/Serialize.h
- Libraries/LibWeb/CSS/CSSStyleProperties.h
- Libraries/LibWeb/CSS/CSSNamespaceRule.h
- Libraries/LibWeb/CSS/CSSTransition.h
- Libraries/LibWeb/CSS/InvalidationSet.h
- Libraries/LibWeb/CSS/GridTrackSize.cpp
- Libraries/LibWeb/CSS/CSSScopeRule.h
- Libraries/LibWeb/CSS/PreferredContrast.h
- Libraries/LibWeb/CSS/CSSCounterStyleRule.h
- Libraries/LibWeb/CSS/Serialize.cpp
- Libraries/LibWeb/CSS/Parser/ValueParsing.cpp
- Libraries/LibWeb/CSS/ColorInterpolation.cpp
- Libraries/LibWeb/CSS/ContainerQuery.h
- Libraries/LibWeb/CSS/Length.h
- Libraries/LibWeb/CSS/ColorInterpolation.h
- Libraries/LibWeb/CSS/Parser/Types.cpp
- Libraries/LibWeb/CSS/GridTrackSize.h
- Libraries/LibWeb/CSS/ComputedProperties.h
- Libraries/LibWeb/CSS/PreferredMotion.h
- Libraries/LibWeb/CSS/StyleScope.h
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.cpp
- Libraries/LibWeb/CSS/Interpolation.h
- Libraries/LibWeb/CSS/CSSPropertyRule.h
- Libraries/LibWeb/CSS/StyleScope.cpp
- Libraries/LibWeb/CSS/Parser/Parser.cpp
- Libraries/LibWeb/CSS/CSSImportRule.cpp
- Libraries/LibWeb/CSS/CSSImportRule.h
- Libraries/LibWeb/CSS/GridTrackPlacement.h
- Libraries/LibWeb/CSS/CSSScopeRule.cpp
- Libraries/LibWeb/CSS/Size.cpp
- Libraries/LibWeb/CSS/Parser/Parser.h
- Libraries/LibWeb/CSS/Parser/Types.h
- Libraries/LibWeb/CMakeLists.txt
- Meta/Generators/generate_libweb_css_pseudo_element.py
🚧 Files skipped from review as they are similar to previous changes (93)
- Tests/LibWeb/Text/expected/css/animation-important-suppression.txt
- Tests/LibWeb/Text/expected/css/discrete-animation-rust.txt
- Tests/LibWeb/Text/expected/css/animation-keyframe-conflict-preference.txt
- Tests/LibWeb/Text/expected/css/animation-css-wide-keywords.txt
- Tests/LibWeb/Text/expected/css/shadow-animation-rust.txt
- Tests/LibWeb/Text/input/css/repeatable-list-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/ScrollbarGutterStyleValue.h
- Tests/LibWeb/Text/input/css/shadow-animation-rust.html
- Tests/LibWeb/Text/expected/css/calculated-animation-rust.txt
- Tests/LibWeb/Text/input/css-placeholder-transition.html
- Tests/LibWeb/Text/input/css/filter-animation-rust.html
- Tests/LibWeb/Text/expected/css/transition-effect-batch-rust.txt
- Tests/LibWeb/Text/input/css/discrete-animation-rust.html
- Tests/LibWeb/Text/expected/css/repeatable-list-animation-rust.txt
- Tests/LibWeb/Text/input/css/animation-important-suppression.html
- Tests/LibWeb/Text/expected/css/filter-animation-rust.txt
- Tests/LibWeb/Text/input/css/animation-keyframe-conflict-preference.html
- Libraries/LibWeb/CSS/PseudoElementPropertyGroups.txt
- Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.h
- Tests/LibWeb/Text/expected/css/modern-color-animation-rust.txt
- Libraries/LibWeb/CSS/StyleValues/GuaranteedInvalidStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.cpp
- Libraries/LibWeb/CSS/CustomPropertyData.cpp
- Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.h
- Tests/LibWeb/Text/input/css/animation-css-wide-keywords.html
- Tests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/filter-interpolation-004.txt
- Tests/LibWeb/Text/expected/wpt-import/css/filter-effects/animation/backdrop-filter-interpolation-004.txt
- Tests/LibWeb/Text/input/css/transition-effect-batch-rust.html
- Libraries/LibWeb/CSS/StyleValues/UnicodeRangeStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.h
- Tests/LibWeb/Text/input/css/modern-color-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/GridAutoFlowStyleValue.h
- Libraries/LibWeb/Animations/KeyframeEffect.h
- Tests/LibWeb/Text/input/css/animation-effect-batch-rust.html
- Libraries/LibWeb/CSS/StyleValues/TextUnderlinePositionStyleValue.h
- Tests/LibWeb/Text/expected/css/animation-effect-batch-rust.txt
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/LinearGradientStyleValue.cpp
- Tests/LibWeb/Text/input/css/calculated-animation-rust.html
- Libraries/LibWeb/CSS/StyleValues/EasingStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.cpp
- Libraries/LibWeb/CSS/RustStyleBridge.h
- Libraries/LibWeb/CSS/StyleValues/RadialGradientStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/lib.rs
- Libraries/LibWeb/CSS/Rust/src/transition.rs
- Libraries/LibWeb/CSS/StyleValues/GridTemplateAreaStyleValue.h
- Libraries/LibWeb/CSS/CSSTransition.cpp
- Libraries/LibWeb/CSS/StyleValues/ColorMixStyleValue.h
- Libraries/LibWeb/CSS/StyleComputer.h
- Libraries/LibWeb/CSS/StyleValues/RustStyleValueHandle.h
- Libraries/LibWeb/CSS/Rust/src/ffi_stats.rs
- Libraries/LibWeb/CSS/StyleValues/ColorSchemeStyleValue.h
- Tests/LibWeb/Text/expected/wpt-import/css/css-shapes/animation/shape-outside-composition.txt
- Documentation/CSSGeneratedFiles.md
- Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/GridTrackSizeListStyleValue.cpp
- Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/animations/text-shadow-interpolation.txt
- Libraries/LibWeb/CSS/StyleValues/AbstractImageStyleValue.cpp
- Libraries/LibWeb/Layout/FlexFormattingContext.cpp
- Libraries/LibWeb/CSS/StyleValues/ColorInterpolationMethodStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/ShadowStyleValue.h
- Libraries/LibWeb/Animations/KeyframeEffect.cpp
- Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.h
- Libraries/LibWeb/CSS/CSSStyleProperties.cpp
- Libraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.h
- Libraries/LibWeb/CSS/StyleValues/FontSourceStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/URLStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/color_interpolation.rs
- Tests/LibWeb/TestStylePropertyMetadataParity.cpp
- Libraries/LibWeb/Animations/Animation.h
- Libraries/LibWeb/CSS/StyleValues/ShorthandStyleValue.h
- Libraries/LibWeb/CSS/RustStyleBridge.cpp
- Libraries/LibWeb/CSS/StyleValues/ImageSetStyleValue.cpp
- Libraries/LibWeb/CSS/StyleValues/ConicGradientStyleValue.cpp
- Libraries/LibWeb/CSS/Rust/build.rs
- Libraries/LibWeb/Animations/Animation.cpp
- Libraries/LibWeb/CSS/StyleValues/FilterStyleValue.h
- Libraries/LibWeb/CSS/Rust/src/custom_properties.rs
- Libraries/LibWeb/CSS/StyleValues/RadialSizeStyleValue.h
- Libraries/LibWeb/CSS/Size.h
- Libraries/LibWeb/CSS/ComputedProperties.cpp
- Libraries/LibWeb/CSS/PercentageOr.h
- Libraries/LibWeb/CSS/Rust/src/color_conversion.rs
- Libraries/LibWeb/CSS/StyleValues/StyleValue.cpp
- Libraries/LibWeb/CSS/Rust/src/animation.rs
- Libraries/LibWeb/CSS/StyleValues/CalculatedStyleValue.cpp
- Libraries/LibWeb/CSS/Rust/src/cascaded_properties.rs
- Libraries/LibWeb/CSS/ComputedValues.h
- Libraries/LibWeb/CSS/Rust/src/computed_values.rs
- Libraries/LibWeb/CSS/ComputedValues.cpp
- Libraries/LibWeb/CSS/Rust/src/calc.rs
- Libraries/LibWeb/CSS/Rust/src/style_value.rs
This was referenced Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move shared style-value ownership, animation evaluation, transition decisions, and more computed-style work from C++ to Rust.
Rust now owns immutable style-value data and nested value lifetimes. Animation effects cross the FFI boundary once per element, where Rust performs keyframe selection, easing, interpolation, composition, and transition decisions. This removes the C++ interpolation fallbacks and several Rust-to-C++ callbacks used during style computation.
This advances the style port from isolated Rust helpers to a Rust-owned value graph that can carry values through cascade, animation, and substantial parts of computed-style construction. C++ still owns CSS parsing, DOM and layout snapshots, timelines, animation objects, event dispatch, and the remaining parser- or layout-dependent computation. Those boundaries can move separately as the parser and the rest of style computation are ported.