Bug 280350

Summary: [SVG2] Sync `getTotalLength()` with web specification to throw exception when non-renderable
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: SVGAssignee: Ahmad Saleem <ahmad.saleem792>
Status: REOPENED    
Severity: Normal CC: commit-queue, sabouhallawa, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=282253
Bug Depends on: 282343    
Bug Blocks:    

Ahmad Saleem
Reported 2024-09-25 13:51:38 PDT
Hi Team, While looking into failing WPT test case, I stumbled upon another mismatch in web-spec alignment: Web Spec: https://svgwg.org/svg2-draft/types.html#__svg__SVGGeometryElement__getTotalLength "If current element is a non-rendered element, and the UA is not able to compute the total length of the path, then throw an InvalidStateError." So when I look into our code, we return `0` rather than Exception. WebKit Code: https://github.com/WebKit/WebKit/blob/3c1c55324a3609e8dab5bc4b4bacac0d58cde486/Source/WebCore/svg/SVGGeometryElement.cpp#L49 & https://github.com/WebKit/WebKit/blob/3c1c55324a3609e8dab5bc4b4bacac0d58cde486/Source/WebCore/svg/SVGPathElement.cpp#L181 ^ We need to update definitions in `.h` as well. Changing them to `ExceptionOr<float>` and then return `InvalidStateError`. __ Code: ExceptionOr<float> SVGGeometryElement::getTotalLength() const { protectedDocument()->updateLayoutIgnorePendingStylesheets({ LayoutOptions::ContentVisibilityForceLayout }, this); auto* renderer = this->renderer(); if (!renderer) return Exception { ExceptionCode::InvalidStateError }; if (CheckedPtr renderSVGShape = dynamicDowncast<LegacyRenderSVGShape>(renderer)) return renderSVGShape->getTotalLength(); if (CheckedPtr renderSVGShape = dynamicDowncast<RenderSVGShape>(renderer)) return renderSVGShape->getTotalLength(); ASSERT_NOT_REACHED(); return Exception { ExceptionCode::InvalidStateError }; } and call-site in `getPointAtLength`: // Spec: Clamp distance to [0, length]. auto totalLength = getTotalLength(); if (totalLength.hasException()) return totalLength.releaseException(); distance = clampTo<float>(distance, 0, totalLength.releaseReturnValue()); while in SVGPathElement.cpp: ExceptionOr<float> SVGPathElement::getTotalLength() const { protectedDocument()->updateLayoutIgnorePendingStylesheets({ LayoutOptions::ContentVisibilityForceLayout }, this); if (pathByteStream().isEmpty()) return Exception { ExceptionCode::InvalidStateError, "The element's path is empty."_s }; return getTotalLengthOfSVGPathByteStream(pathByteStream()); } and same on call-site. ___ Test Case (from Blink) - https://jsfiddle.net/3zwLk15h/ (We pass now - Firefox is not following spec here as well and fails this). Thanks!
Attachments
Radar WebKit Bug Importer
Comment 1 2024-09-25 17:53:08 PDT
Ahmad Saleem
Comment 2 2024-09-25 18:02:53 PDT
EWS
Comment 3 2024-09-26 14:24:31 PDT
Committed 284311@main (48ed528d4e27): <https://commits.webkit.org/284311@main> Reviewed commits have been landed. Closing PR #34273 and removing active labels.
WebKit Commit Bot
Comment 4 2024-10-30 11:17:32 PDT
Re-opened since this is blocked by bug 282343
Ahmad Saleem
Comment 5 2024-11-01 07:28:32 PDT
NOTE: Take feedback from this PR (thanks to Said) - https://github.com/WebKit/WebKit/pull/35873
Note You need to log in before you can comment on or make changes to this bug.