Bug 1665402 - fix SVGGeometry.getTotalLength to work if the element is not attached to the document or is display:none r=emilio
authorlongsonr <longsonr@gmail.com>
Thu, 17 Sep 2020 07:50:43 +0000
changeset 549156 f6aa73f7188f9354cb4677ad38e85e62b96d5134
parent 549155 5581f76448f653032c54fe166eb1f00204b59991
child 549157 4fe124e5111a4612a407d5645c9fa8323f3d2ee0
push id126589
push userlongsonr@gmail.com
push dateThu, 17 Sep 2020 13:23:00 +0000
treeherderautoland@f6aa73f7188f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1665402
milestone82.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1665402 - fix SVGGeometry.getTotalLength to work if the element is not attached to the document or is display:none r=emilio fallback to how we used to do it if there's no styles Differential Revision: https://phabricator.services.mozilla.com/D90440
dom/svg/SVGCircleElement.cpp
dom/svg/SVGEllipseElement.cpp
dom/svg/SVGForeignObjectElement.cpp
dom/svg/SVGRectElement.cpp
dom/svg/test/test_getTotalLength.xhtml
testing/web-platform/tests/svg/types/scripted/SVGGeometryElement.getPointAtLength-03.svg
--- a/dom/svg/SVGCircleElement.cpp
+++ b/dom/svg/SVGCircleElement.cpp
@@ -122,18 +122,23 @@ bool SVGCircleElement::GetGeometryBounds
     return true;
   }
 
   return false;
 }
 
 already_AddRefed<Path> SVGCircleElement::BuildPath(PathBuilder* aBuilder) {
   float x, y, r;
-  SVGGeometryProperty::ResolveAllAllowFallback<SVGT::Cx, SVGT::Cy, SVGT::R>(
-      this, &x, &y, &r);
+  if (!SVGGeometryProperty::ResolveAllAllowFallback<SVGT::Cx, SVGT::Cy,
+                                                    SVGT::R>(this, &x, &y,
+                                                             &r)) {
+    // This function might be called for element in display:none subtree
+    // (e.g. getTotalLength), we fall back to use SVG attributes.
+    GetAnimatedLengthValues(&x, &y, &r, nullptr);
+  }
 
   if (r <= 0.0f) {
     return nullptr;
   }
 
   aBuilder->Arc(Point(x, y), r, 0, Float(2 * M_PI));
 
   return aBuilder->Finish();
--- a/dom/svg/SVGEllipseElement.cpp
+++ b/dom/svg/SVGEllipseElement.cpp
@@ -134,19 +134,23 @@ bool SVGEllipseElement::GetGeometryBound
   }
 
   return false;
 }
 
 already_AddRefed<Path> SVGEllipseElement::BuildPath(PathBuilder* aBuilder) {
   float x, y, rx, ry;
 
-  SVGGeometryProperty::ResolveAllAllowFallback<SVGT::Cx, SVGT::Cy, SVGT::Rx,
-                                               SVGT::Ry>(this, &x, &y, &rx,
-                                                         &ry);
+  if (!SVGGeometryProperty::ResolveAllAllowFallback<SVGT::Cx, SVGT::Cy,
+                                                    SVGT::Rx, SVGT::Ry>(
+          this, &x, &y, &rx, &ry)) {
+    // This function might be called for element in display:none subtree
+    // (e.g. getTotalLength), we fall back to use SVG attributes.
+    GetAnimatedLengthValues(&x, &y, &rx, &ry, nullptr);
+  }
 
   if (rx <= 0.0f || ry <= 0.0f) {
     return nullptr;
   }
 
   EllipseToBezier(aBuilder, Point(x, y), Size(rx, ry));
 
   return aBuilder->Finish();
--- a/dom/svg/SVGForeignObjectElement.cpp
+++ b/dom/svg/SVGForeignObjectElement.cpp
@@ -76,19 +76,17 @@ gfxMatrix SVGForeignObjectElement::Prepe
   gfxMatrix fromUserSpace =
       SVGGraphicsElement::PrependLocalTransformsTo(aMatrix, aWhich);
   if (aWhich == eUserSpaceToParent) {
     return fromUserSpace;
   }
   // our 'x' and 'y' attributes:
   float x, y;
 
-  if (GetPrimaryFrame()) {
-    SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y>(this, &x, &y);
-  } else {
+  if (!SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y>(this, &x, &y)) {
     // This function might be called for element in display:none subtree
     // (e.g. getScreenCTM), we fall back to use SVG attributes.
     const_cast<SVGForeignObjectElement*>(this)->GetAnimatedLengthValues(
         &x, &y, nullptr);
   }
 
   gfxMatrix toUserSpace = gfxMatrix::Translation(x, y);
   if (aWhich == eChildToUserSpace) {
--- a/dom/svg/SVGRectElement.cpp
+++ b/dom/svg/SVGRectElement.cpp
@@ -188,19 +188,32 @@ void SVGRectElement::GetAsSimplePath(Sim
   }
 
   aSimplePath->SetRect(x, y, width, height);
 }
 
 already_AddRefed<Path> SVGRectElement::BuildPath(PathBuilder* aBuilder) {
   float x, y, width, height, rx, ry;
 
-  SVGGeometryProperty::ResolveAllAllowFallback<
-      SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height, SVGT::Rx, SVGT::Ry>(
-      this, &x, &y, &width, &height, &rx, &ry);
+  if (!SVGGeometryProperty::ResolveAllAllowFallback<
+          SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height, SVGT::Rx, SVGT::Ry>(
+          this, &x, &y, &width, &height, &rx, &ry)) {
+    // This function might be called for element in display:none subtree
+    // (e.g. getTotalLength), we fall back to use SVG attributes.
+    GetAnimatedLengthValues(&x, &y, &width, &height, &rx, &ry, nullptr);
+    // If either the 'rx' or the 'ry' attribute isn't set, then we have to
+    // set it to the value of the other:
+    bool hasRx = mLengthAttributes[ATTR_RX].IsExplicitlySet();
+    bool hasRy = mLengthAttributes[ATTR_RY].IsExplicitlySet();
+    if (hasRx && !hasRy) {
+      ry = rx;
+    } else if (hasRy && !hasRx) {
+      rx = ry;
+    }
+  }
 
   if (width <= 0 || height <= 0) {
     return nullptr;
   }
 
   rx = std::max(rx, 0.0f);
   ry = std::max(ry, 0.0f);
 
--- a/dom/svg/test/test_getTotalLength.xhtml
+++ b/dom/svg/test/test_getTotalLength.xhtml
@@ -20,27 +20,37 @@ https://bugzilla.mozilla.org/show_bug.cg
     <rect id="r1" x="5em" y="6em" width="20%" height="30%" />
   </symbol>
 </svg>
 
 <pre id="test">
 <script class="testbody" type="application/javascript">
 SimpleTest.waitForExplicitFinish();
 
-function expectValue(id, expected) {
-  isfuzzy(document.getElementById(id).getTotalLength(),
-          expected, expected * 0.02,
-          `getTotalLength() on element id="${id}" returned the wrong value`);
-}
-
 function run() {
-  expectValue("path1", 55.19);
+  isfuzzy(document.getElementById("path1").getTotalLength(),
+          55.19, 0.02,
+          'getTotalLength() on element id="path1" returned the wrong value');
 
   let r1 = document.getElementById("r1");
-  is(r1.getTotalLength(), 200, "getTotalLength() should work for display:none element");
+  is(r1.getTotalLength(), 200, "getTotalLength() should work for non-rendered element");
+
+  let r2 = document.createElementNS("http://www.w3.org/2000/svg", "rect");
+  r2.setAttribute("width", 200);
+  r2.setAttribute("height", 300);
+  is(r2.getTotalLength(), 1000, "getTotalLength() should work for a rect element not in the document");
+
+  let c = document.createElementNS("http://www.w3.org/2000/svg", "circle");
+  c.setAttribute("r", 200);
+  isfuzzy(c.getTotalLength(), 2 * Math.PI * 200, 0.2, "getTotalLength() should work for a circle element not in the document");
+
+  let e = document.createElementNS("http://www.w3.org/2000/svg", "ellipse");
+  e.setAttribute("rx", 200);
+  e.setAttribute("ry", 200);
+  isfuzzy(e.getTotalLength(), 2 * Math.PI * 200, 0.2, "getTotalLength() should work for an ellipse element not in the document");
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run);
 </script>
 </pre>
 </body>
--- a/testing/web-platform/tests/svg/types/scripted/SVGGeometryElement.getPointAtLength-03.svg
+++ b/testing/web-platform/tests/svg/types/scripted/SVGGeometryElement.getPointAtLength-03.svg
@@ -1,29 +1,41 @@
 <svg xmlns="http://www.w3.org/2000/svg"
      xmlns:xlink="http://www.w3.org/1999/xlink"
      xmlns:html="http://www.w3.org/1999/xhtml">
-  <title>When SVGGeometryElement.getPointAtLength is called with an element that is not in the document, throw exception</title>
+  <title>When SVGGeometryElement.getPointAtLength is called with an element that is not in the document, either succeed or throw exception</title>
   <html:link rel="help" href="https://svgwg.org/svg2-draft/types.html#__svg__SVGGeometryElement__getPointAtLength"/>
   <html:script src="/resources/testharness.js"/>
   <html:script src="/resources/testharnessreport.js"/>
   <script><![CDATA[
   test(function() {
     var pathElement = document.createElementNS("http://www.w3.org/2000/svg", "path");
     assert_throws_dom("InvalidStateError", function() { pathElement.getPointAtLength(700) });
   }, document.title + " with SVGPathElement");
 
   test(function() {
     var rectElement = document.createElementNS("http://www.w3.org/2000/svg", "rect");
     rectElement.setAttribute("rx", 0);
     rectElement.setAttribute("ry", 0);
     rectElement.setAttribute("width", 200);
     rectElement.setAttribute("height", 300);
-    assert_throws_dom("InvalidStateError", function() { rectElement.getPointAtLength(300); });
+    try {
+       let rectPoint = rectElement.getPointAtLength(300);
+       assert_equals(rectPoint.x, 200, 'unexpected x position');
+       assert_equals(rectPoint.y, 100, 'unexpected y position');
+    } catch (e) {
+       assert_equals(e.name || e.type, 'InvalidStateError', 'Wrong exception, expected InvalidStateError');
+    }
   }, document.title + " with SVGRectElement");
 
   test(function() {
     var circleElement = document.createElementNS("http://www.w3.org/2000/svg", "circle");
     circleElement.setAttribute("r", 10);
-    assert_throws_dom("InvalidStateError", function() { circleElement.getPointAtLength(100); });
+    try {
+       let circlePoint = circleElement.getPointAtLength(100);
+       assert_approx_equals(circlePoint.x, 10, 0.2, 'unexpected x position');
+       assert_approx_equals(circlePoint.y, 0, 0.2, 'unexpected y position');
+    } catch (e) {
+       assert_equals(e.name || e.type, 'InvalidStateError', 'Wrong exception, expected InvalidStateError');
+    }
   }, document.title + " with SVGCircleElement");
   ]]></script>
 </svg>