SVGLength percentage resolution fails for elements inside non-instanced <symbol> or when viewportElement() returns nullptr

Ahmad-S792 · Ahmad Saleem · commit aa1b9b223e97 · 2025-12-09T18:37:22.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=303127 rdar://165431008 Reviewed by Simon Fraser. SVGLength.value was throwing `NotSupportedError` when resolving percentage values for elements inside non-instanced <symbol> elements or display:none containers. According to the SVG specification [1], either an <svg> element or a <symbol> element that is instanced by a <use> element establishes a new viewport. For non-instanced <symbol> elements, the nearest ancestor <svg> should provide the viewport for percentage resolution. Fix viewport calculation by extending viewportElement() with a ViewportElementType enum that allows callers to specify whether they want any viewport element (including <symbol>) or specifically an <svg> element. This eliminates the need for manual ancestor walks in computeViewportSize() while preserving the original behavior for normal viewport elements. When ViewportElementType::SVGSVGOnly is specified, viewportElement() now walks up the ancestor tree to find the nearest <svg> element, skipping <symbol> elements. This matches other browser engines (Blink / Chromium and Gecko / Firefox). [1] https://svgwg.org/svg2-draft/coords.html#EstablishingANewSVGViewport NOTE: We still don't progress WPT due to floating point issue with height in WPT test but it progress for width. * Source/WebCore/svg/SVGElement.cpp: (WebCore::SVGElement::viewportElement const): * Source/WebCore/svg/SVGElement.h: * Source/WebCore/svg/SVGLengthContext.cpp: (WebCore::SVGLengthContext::computeViewportSize const): * LayoutTests/imported/w3c/web-platform-tests/svg/geometry/svg-baseval-in-display-none-expected.txt: Partial Progression Canonical link: https://commits.webkit.org/304197@main
diff --git a/LayoutTests/imported/w3c/web-platform-tests/svg/geometry/svg-baseval-in-display-none-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/svg/geometry/svg-baseval-in-display-none-expected.txt
@@ -1,4 +1,4 @@
 
 PASS With em
-FAIL With em and percentage The operation is not supported.
+FAIL With em and percentage assert_equals: r1.height expected 120 but got 120.00000762939453
 
diff --git a/Source/WebCore/svg/SVGElement.cpp b/Source/WebCore/svg/SVGElement.cpp
@@ -230,14 +230,17 @@ SVGSVGElement* SVGElement::ownerSVGElement() const
     return nullptr;
 }
 
-SVGElement* SVGElement::viewportElement() const
+SVGElement* SVGElement::viewportElement(ViewportElementType type) const
 {
     // This function needs shadow tree support - as RenderSVGContainer uses this function
-    // to determine the "overflow" property. <use> on <symbol> wouldn't work otherwhise.
+    // to determine the "overflow" property. <use> on <symbol> wouldn't work otherwise.
     auto* node = parentNode();
     while (node) {
-        if (is<SVGSVGElement>(*node) || is<SVGImageElement>(*node) || node->hasTagName(SVGNames::symbolTag))
-            return downcast<SVGElement>(node);
+        if (is<SVGSVGElement>(*node) || is<SVGImageElement>(*node))
+            return dynamicDowncast<SVGElement>(node);
+
+        if (type == ViewportElementType::Any && node->hasTagName(SVGNames::symbolTag))
+            return dynamicDowncast<SVGElement>(node);
 
         node = node->parentOrShadowHostNode();
     }
diff --git a/Source/WebCore/svg/SVGElement.h b/Source/WebCore/svg/SVGElement.h
@@ -70,7 +70,13 @@ class SVGElement : public StyledElement, public SVGPropertyOwner {
     bool isOutermostSVGSVGElement() const;
 
     SVGSVGElement* ownerSVGElement() const;
-    SVGElement* viewportElement() const;
+
+    enum class ViewportElementType : uint8_t {
+        Any, // Returns first SVGSVGElement, SVGImageElement, or symbol
+        SVGSVGOnly // Returns only SVGSVGElement
+    };
+
+    SVGElement* viewportElement(ViewportElementType = ViewportElementType::Any) const;
 
     String title() const override;
     virtual bool supportsMarkers() const { return false; }
diff --git a/Source/WebCore/svg/SVGLengthContext.cpp b/Source/WebCore/svg/SVGLengthContext.cpp
@@ -33,6 +33,7 @@
 #include "LocalFrame.h"
 #include "RenderStyleInlines.h"
 #include "RenderView.h"
+#include "SVGElement.h"
 #include "SVGElementTypeHelpers.h"
 #include "SVGSVGElement.h"
 #include "StyleLengthResolution.h"
@@ -421,6 +422,8 @@ std::optional<FloatSize> SVGLengthContext::viewportSize() const
 
 std::optional<FloatSize> SVGLengthContext::computeViewportSize() const
 {
+    using ViewportElementType = SVGElement::ViewportElementType;
+
     ASSERT(m_context);
 
     // Root <svg> element lengths are resolved against the top level viewport,
@@ -432,8 +435,8 @@ std::optional<FloatSize> SVGLengthContext::computeViewportSize() const
     if (m_context->isOutermostSVGSVGElement())
         return downcast<SVGSVGElement>(*protectedContext()).currentViewportSizeExcludingZoom();
 
-    // Take size from nearest viewport element.
-    RefPtr svg = dynamicDowncast<SVGSVGElement>(m_context->viewportElement());
+    // Take size from nearest SVGSVGElement, skipping over <symbol> elements.
+    RefPtr svg = dynamicDowncast<SVGSVGElement>(m_context->viewportElement(ViewportElementType::SVGSVGOnly));
     if (!svg)
         return std::nullopt;
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`	`1`
`2`	`2`	`PASS With em`
`3`		`-FAIL With em and percentage The operation is not supported.`
	`3`	`+FAIL With em and percentage assert_equals: r1.height expected 120 but got 120.00000762939453`
`4`	`4`