LibWeb: Respect CSS values for more SVG elements - #11067
Conversation
Previously we used attribute values directly, ignoring values set via CSS. We now use the values produced by the CSS cascade (which includes the attribute values as presentational hints).
|
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 selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSVG ellipse and rectangle geometry now uses computed CSS values. The implementation resolves viewport-relative lengths, 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: 1
🤖 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/SVG/SVGRectElement.cpp`:
- Around line 114-134: Update the rx/ry percentage conversions in the
SVGRectElement radius calculation to resolve rx against used_width and ry
against used_height in all three auto/non-auto branches before clamping. Add a
regression test using percentage radii where the used rectangle dimensions
differ from the viewport, verifying the radii use the rectangle dimensions
rather than viewport-relative values.
🪄 Autofix
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: b2e229f6-e457-4f51-9ee7-21056c391fcb
⛔ Files ignored due to path filters (5)
Tests/LibWeb/Ref/expected/wpt-import/svg/geometry/reftests/ellipse-ref.svgis excluded by!**/*.svgTests/LibWeb/Ref/input/wpt-import/css/css-images/support/colors-16x8.svgis excluded by!**/*.svgTests/LibWeb/Ref/input/wpt-import/svg/geometry/reftests/ellipse-001.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/geometry/parsing/height-computed.svgis excluded by!**/*.svgTests/LibWeb/Text/input/wpt-import/svg/geometry/parsing/width-computed.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
Libraries/LibWeb/SVG/SVGEllipseElement.cppLibraries/LibWeb/SVG/SVGEllipseElement.hLibraries/LibWeb/SVG/SVGImageElement.cppLibraries/LibWeb/SVG/SVGImageElement.hLibraries/LibWeb/SVG/SVGRectElement.cppLibraries/LibWeb/SVG/SVGRectElement.hLibraries/LibWeb/SVG/SVGUseElement.cppLibraries/LibWeb/SVG/SVGUseElement.hTests/LibWeb/Layout/expected/svg/rect-percentages.txtTests/LibWeb/Ref/expected/wpt-import/css/css-images/object-position-svg-001-ref.htmlTests/LibWeb/Ref/expected/wpt-import/svg/geometry/reftests/ellipse-calc-dynamic-viewport-ref.htmlTests/LibWeb/Ref/expected/wpt-import/svg/shapes/reftests/ellipse-auto-rx-textpath-ref.htmlTests/LibWeb/Ref/input/wpt-import/css/css-images/object-position-svg-001o.htmlTests/LibWeb/Ref/input/wpt-import/svg/geometry/reftests/ellipse-calc-dynamic-viewport.htmlTests/LibWeb/Ref/input/wpt-import/svg/shapes/reftests/ellipse-auto-rx-textpath.htmlTests/LibWeb/Text/expected/SVG/use-element-css-x-y.txtTests/LibWeb/Text/expected/wpt-import/svg/geometry/parsing/height-computed.txtTests/LibWeb/Text/expected/wpt-import/svg/geometry/parsing/width-computed.txtTests/LibWeb/Text/expected/wpt-import/svg/geometry/svg-image-intrinsic-size-with-cssstyle-auto.txtTests/LibWeb/Text/expected/wpt-import/svg/types/scripted/SVGGraphicsElement.getBBox-08.txtTests/LibWeb/Text/input/SVG/use-element-css-x-y.htmlTests/LibWeb/Text/input/wpt-import/svg/geometry/svg-image-intrinsic-size-with-cssstyle-auto.htmlTests/LibWeb/Text/input/wpt-import/svg/types/scripted/SVGGraphicsElement.getBBox-08.html
💤 Files with no reviewable changes (5)
- Tests/LibWeb/Ref/expected/wpt-import/css/css-images/object-position-svg-001-ref.html
- Libraries/LibWeb/SVG/SVGUseElement.h
- Libraries/LibWeb/SVG/SVGEllipseElement.h
- Tests/LibWeb/Ref/input/wpt-import/css/css-images/object-position-svg-001o.html
- Libraries/LibWeb/SVG/SVGImageElement.h
Previously we used attribute values directly, ignoring values set via CSS. We now use the values produced by the CSS cascade (which includes the attribute values as presentational hints) and implement the updated algorithm to determine used corner radii values. This change revealed a false positive SVG object-position test which has been removed from tree. This change causes a regression in `rect-percentages.html` since we now round to CSSPixels prematurely (i.e. while still in SVG user space), this is covered by a FIXME for now since it's a larger issue than just this element.
This matches the behavior described by the spec - we also now use the correct 300x150 size when we have an image without a natural size.
Previously we used attribute values directly, ignoring values set via CSS. We now use the values produced by the CSS cascade (which includes the attribute values as presentational hints).
Previously we used attribute values directly, ignoring values set via CSS. We now use the values produced by the CSS cascade (which includes the attribute values as presentational hints).
f3b48ae to
cbf8934
Compare
This fixes a few cases where previously we used attribute values directly, ignoring values set via CSS. We now use the values produced by the CSS cascade (which includes the attribute values as presentational hints).
See individual commits for details.