Skip to content

LibWeb: Implement SVGGeometryElement.getTotalLength() - #11006

Merged
gmta merged 4 commits into
LadybirdBrowser:masterfrom
Calme1709:get-total-length
Aug 5, 2026
Merged

LibWeb: Implement SVGGeometryElement.getTotalLength()#11006
gmta merged 4 commits into
LadybirdBrowser:masterfrom
Calme1709:get-total-length

Conversation

@Calme1709

Copy link
Copy Markdown
Member

This was stubbed but now returns the correct value

This was stubbed but now returns the correct value
Use the element's own computed values rather than the layout node's.

This method can be called when there is no layout node for the element
(for instance `SVGGeometryElement::getTotalLength` when the element is
`display: none`).
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

SVG geometry elements now calculate total path length after layout and style updates. SVG percentage values use element-specific viewport resolution. Circle path generation reads computed geometry values directly. WPT coverage was added for hidden rectangles, circles, and paths.

Changes

SVG geometry length

Layer / File(s) Summary
Viewport-aware SVG percentage resolution
Libraries/LibWeb/DOM/Document.h, Libraries/LibWeb/SVG/SVGElement.*, Libraries/LibWeb/SVG/SVGLength.cpp
SVG elements expose viewport dimensions for percentage resolution. SVG length calculations use this method. SVGPathLength was added as an update-layout reason.
Computed geometry length calculation
Libraries/LibWeb/SVG/SVGGeometryElement.*, Libraries/LibWeb/SVG/SVGCircleElement.cpp
get_total_length() now updates layout and style, validates computed values, and returns generated path length. Circle paths read computed cx, cy, and r values directly.
Hidden geometry WPT coverage
Tests/LibWeb/Text/input/wpt-import/svg/types/scripted/*, Tests/LibWeb/Text/expected/wpt-import/svg/types/scripted/*
Tests cover hidden rectangles, circles, and paths using different path-data sources. The recorded results include one remaining setPathData failure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SVGGeometryElement
  participant Document
  participant SVGElement
  participant SVGPath
  SVGGeometryElement->>Document: Update layout and element style
  SVGGeometryElement->>SVGElement: Resolve viewport dimensions
  SVGGeometryElement->>SVGPath: Generate path and measure total length
  SVGGeometryElement-->>SVGGeometryElement: Return ExceptionOr<float>
Loading

Possibly related PRs

Suggested reviewers: awesomekling

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description states that a stubbed function now returns the correct value, which matches the implementation of SVGGeometryElement.getTotalLength().
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/SVGElement.cpp`:
- Around line 424-432: Update viewport_size_for_percentage_resolution() to
obtain the viewport through viewport_element() instead of owner_svg_element().
Preserve the existing null check, active_view_box handling, and
viewport_size_from_layout fallback so percentage geometry in <use>-instantiated
<symbol> contents resolves against the symbol’s viewport.
🪄 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: 266cbf4b-b93c-4bde-812f-ffd57e821dc3

📥 Commits

Reviewing files that changed from the base of the PR and between bba04b3 and 7338f71.

⛔ Files ignored due to path filters (1)
  • Tests/LibWeb/Text/input/wpt-import/svg/types/scripted/SVGGeometryElement.getTotalLength-01.svg is excluded by !**/*.svg
📒 Files selected for processing (12)
  • Libraries/LibWeb/DOM/Document.h
  • Libraries/LibWeb/SVG/SVGCircleElement.cpp
  • Libraries/LibWeb/SVG/SVGElement.cpp
  • Libraries/LibWeb/SVG/SVGElement.h
  • Libraries/LibWeb/SVG/SVGGeometryElement.cpp
  • Libraries/LibWeb/SVG/SVGGeometryElement.h
  • Libraries/LibWeb/SVG/SVGLength.cpp
  • Tests/LibWeb/Text/expected/wpt-import/svg/types/scripted/SVGGeometryElement.getTotalLength-01.txt
  • Tests/LibWeb/Text/expected/wpt-import/svg/types/scripted/SVGGeometryElement.getTotalLength-02.txt
  • Tests/LibWeb/Text/expected/wpt-import/svg/types/scripted/SVGPathElement.getTotalLength-01.txt
  • Tests/LibWeb/Text/input/wpt-import/svg/types/scripted/SVGGeometryElement.getTotalLength-02.html
  • Tests/LibWeb/Text/input/wpt-import/svg/types/scripted/SVGPathElement.getTotalLength-01.html

Comment thread Libraries/LibWeb/SVG/SVGElement.cpp
@gmta
gmta merged commit 08f6ae7 into LadybirdBrowser:master Aug 5, 2026
15 checks passed
@Calme1709
Calme1709 deleted the get-total-length branch August 6, 2026 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants