Bug 280350
Summary: | [SVG2] Sync `getTotalLength()` with web specification to throw exception when non-renderable | ||
---|---|---|---|
Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
Component: | SVG | Assignee: | 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
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 | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/136719548>
Ahmad Saleem
Pull request: https://github.com/WebKit/WebKit/pull/34273
EWS
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
Re-opened since this is blocked by bug 282343
Ahmad Saleem
NOTE: Take feedback from this PR (thanks to Said) - https://github.com/WebKit/WebKit/pull/35873