LibWeb: Implement the CSS d property - #10958
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (22)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (20)
📝 WalkthroughWalkthroughThe change adds CSS ChangesSVG d property
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SVGElement
participant CSSParser
participant ComputedValues
participant SVGPathElement
SVGElement->>CSSParser: parse the d presentation attribute as path()
CSSParser->>ComputedValues: store the computed d value
SVGPathElement->>ComputedValues: request computed d
ComputedValues-->>SVGPathElement: return d style value
SVGPathElement-->>SVGPathElement: build Gfx::Path geometry
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt`:
- Line 165: Regenerate the expected outputs through the WPT import workflow
rather than editing them directly: update
Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt
at 165-165,
Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-discrete.txt
at 6-6,
Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-relative-absolute.txt
at 7-7, and
Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-single.txt
at 7-7. Apply the expectation changes upstream or via the repository’s import
process.
In
`@Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-relative-absolute.txt`:
- Around line 3-6: Fix the underlying SVG path d-interpolation failures before
regenerating expected outputs. Update both
Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-relative-absolute.txt
lines 3-6 and
Tests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-single.txt
lines 3-6 only after the suites pass, so the snapshots record intended WPT
results rather than failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ae9cb6f2-477d-40ff-ae46-72c9e7f986e1
⛔ Files ignored due to path filters (8)
Tests/LibWeb/Ref/expected/wpt-import/svg/path/property/d-none-ref.svgis excluded by!**/*.svgTests/LibWeb/Ref/expected/wpt-import/svg/path/property/priority-ref.svgis excluded by!**/*.svgTests/LibWeb/Ref/input/wpt-import/svg/path/property/d-none.svgis excluded by!**/*.svgTests/LibWeb/Ref/input/wpt-import/svg/path/property/priority.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/path/property/d-interpolation-discrete.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/path/property/d-interpolation-relative-absolute.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/path/property/d-interpolation-single.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/path/property/getComputedStyle.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
Libraries/LibWeb/CSS/ComputedProperties.cppLibraries/LibWeb/CSS/ComputedValues.cppLibraries/LibWeb/CSS/ComputedValues.hLibraries/LibWeb/CSS/Parser/Parser.hLibraries/LibWeb/CSS/Parser/PropertyParsing.cppLibraries/LibWeb/CSS/Properties.jsonLibraries/LibWeb/Rust/src/css/computed_value_types.rsLibraries/LibWeb/Rust/src/css/computed_values.rsLibraries/LibWeb/SVG/SVGElement.cppLibraries/LibWeb/SVG/SVGPathElement.cppLibraries/LibWeb/SVG/SVGPathElement.hTests/LibWeb/Text/expected/css/CSSStyleDeclaration-has-indexed-property-getter.txtTests/LibWeb/Text/expected/css/CSSStyleProperties-all-supported-properties-and-default-values.txtTests/LibWeb/Text/expected/css/getComputedStyle-print-all.txtTests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txtTests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-discrete.txtTests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-relative-absolute.txtTests/LibWeb/Text/expected/wpt-import/svg/path/property/d-interpolation-single.txtTests/LibWeb/Text/expected/wpt-import/svg/path/property/getComputedStyle.txtTests/LibWeb/Text/expected/wpt-import/svg/styling/presentation-attributes-relevant.txtTests/LibWeb/Text/expected/wpt-import/svg/styling/presentation-attributes-special-cases.txtTests/LibWeb/Text/input/wpt-import/svg/path/property/resources/interpolation-test-common.js
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/SVG/SVGPathElement.h
Pass `d` presentational attributes through the cascade in preparation for switching <path> to respect cascaded CSS values for `d`.
Previously we directly used the `d` attribute's value. Now we use the value produced by the CSS cascade (which includes the attribute value as a presentational hint).
a21fa53 to
e797dd5
Compare
Previously we directly used the attribute value for a
<path>element's path.This PR implements the
dCSS property and instead uses the cascaded value (including the attribute value as a presentational hint).