[Webkit-unassigned] [Bug 280350] New: [SVG2] Sync `getTotalLength()` with web specification to throw exception when non-renderable

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 25 13:51:38 PDT 2024


https://bugs.webkit.org/show_bug.cgi?id=280350

            Bug ID: 280350
           Summary: [SVG2] Sync `getTotalLength()` with web specification
                    to throw exception when non-renderable
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: SVG
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: ahmad.saleem792 at gmail.com
                CC: sabouhallawa at apple.com, zimmermann at kde.org

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!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20240925/d06877ed/attachment.htm>


More information about the webkit-unassigned mailing list