Bug 1667641 - map SVG use element x and y to style r=emilio

matches current Chrome per 5a1cdd7a0e

Differential Revision: https://phabricator.services.mozilla.com/D91528

dom/svg/SVGContentUtils.cpp

@@ -800,10 +800,11 @@ bool SVGContentUtils::ParseInteger(const nsAString& aString, int32_t& aValue) {
 }
 
 float SVGContentUtils::CoordToFloat(SVGElement* aContent,
-                                    const LengthPercentage& aLength) {
+                                    const LengthPercentage& aLength,
+                                    uint8_t aCtxType) {
   float result = aLength.ResolveToCSSPixelsWith([&] {
     SVGViewportElement* ctx = aContent->GetCtx();
-    return CSSCoord(ctx ? ctx->GetLength(SVGContentUtils::XY) : 0.0f);
+    return CSSCoord(ctx ? ctx->GetLength(aCtxType) : 0.0f);
   });
   if (aLength.IsCalc()) {
     const auto& calc = aLength.AsCalc();

dom/svg/SVGContentUtils.h

@@ -310,7 +310,8 @@ class SVGContentUtils {
    * Converts a LengthPercentage into a userspace value, resolving percentage
    * values relative to aContent's SVG viewport.
    */
-  static float CoordToFloat(dom::SVGElement* aContent, const LengthPercentage&);
+  static float CoordToFloat(dom::SVGElement* aContent, const LengthPercentage&,
+                            uint8_t aCtxType = SVGContentUtils::XY);
   /**
    * Parse the SVG path string
    * Returns a path

dom/svg/SVGElement.cpp

@@ -1467,9 +1467,13 @@ void SVGElement::DidAnimateLength(uint8_t aAttrEnum) {
     nsCSSPropertyID propId =
         SVGGeometryProperty::AttrEnumToCSSPropId(this, aAttrEnum);
 
-    SMILOverrideStyle()->SetSMILValue(propId,
-                                      GetLengthInfo().mLengths[aAttrEnum]);
-    return;
+    // We don't map use element width/height currently. We can remove this
+    // test when we do.
+    if (propId != eCSSProperty_UNKNOWN) {
+      SMILOverrideStyle()->SetSMILValue(propId,
+                                        GetLengthInfo().mLengths[aAttrEnum]);
+      return;
+    }
   }
 
   nsIFrame* frame = GetPrimaryFrame();

dom/svg/SVGGeometryProperty.cpp

@@ -10,6 +10,7 @@
 #include "SVGForeignObjectElement.h"
 #include "SVGImageElement.h"
 #include "SVGRectElement.h"
+#include "SVGUseElement.h"
 
 namespace mozilla {
 namespace dom {
@@ -70,6 +71,9 @@ nsCSSPropertyID AttrEnumToCSSPropId(const SVGElement* aElement,
   if (aElement->IsSVGElement(nsGkAtoms::foreignObject)) {
     return SVGForeignObjectElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
   }
+  if (aElement->IsSVGElement(nsGkAtoms::use)) {
+    return SVGUseElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
+  }
   return eCSSProperty_UNKNOWN;
 }
 
@@ -80,11 +84,9 @@ bool IsNonNegativeGeometryProperty(nsCSSPropertyID aProp) {
 }
 
 bool ElementMapsLengthsToStyle(SVGElement const* aElement) {
-  return aElement->IsSVGElement(nsGkAtoms::rect) ||
-         aElement->IsSVGElement(nsGkAtoms::circle) ||
-         aElement->IsSVGElement(nsGkAtoms::ellipse) ||
-         aElement->IsSVGElement(nsGkAtoms::image) ||
-         aElement->IsSVGElement(nsGkAtoms::foreignObject);
+  return aElement->IsAnyOfSVGElements(nsGkAtoms::rect, nsGkAtoms::circle,
+                                      nsGkAtoms::ellipse, nsGkAtoms::image,
+                                      nsGkAtoms::foreignObject, nsGkAtoms::use);
 }
 
 }  // namespace SVGGeometryProperty

dom/svg/SVGUseElement.cpp

@@ -19,6 +19,7 @@
 #include "nsGkAtoms.h"
 #include "nsContentUtils.h"
 #include "nsIURI.h"
+#include "SVGGeometryProperty.h"
 
 NS_IMPL_NS_NEW_SVG_ELEMENT(Use)
 
@@ -82,6 +83,8 @@ SVGUseElement::~SVGUseElement() {
                         "Dying without unbinding?");
 }
 
+namespace SVGT = SVGGeometryProperty::Tags;
+
 //----------------------------------------------------------------------
 // nsINode methods
 
@@ -92,12 +95,7 @@ bool SVGUseElement::IsNodeOfType(uint32_t aFlags) const {
 void SVGUseElement::ProcessAttributeChange(int32_t aNamespaceID,
                                            nsAtom* aAttribute) {
   if (aNamespaceID == kNameSpaceID_None) {
-    if (aAttribute == nsGkAtoms::x || aAttribute == nsGkAtoms::y) {
-      if (auto* frame = GetFrame()) {
-        frame->PositionAttributeChanged();
-      }
-    } else if (aAttribute == nsGkAtoms::width ||
-               aAttribute == nsGkAtoms::height) {
+    if (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height) {
       const bool hadValidDimensions = HasValidDimensions();
       const bool isUsed = OurWidthAndHeightAreUsed();
       if (isUsed) {
@@ -487,7 +485,9 @@ gfxMatrix SVGUseElement::PrependLocalTransformsTo(
 
   // our 'x' and 'y' attributes:
   float x, y;
-  const_cast<SVGUseElement*>(this)->GetAnimatedLengthValues(&x, &y, nullptr);
+  if (!SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y>(this, &x, &y)) {
+    const_cast<SVGUseElement*>(this)->GetAnimatedLengthValues(&x, &y, nullptr);
+  }
 
   gfxMatrix childToUser = gfxMatrix::Translation(x, y);
 
@@ -550,9 +550,22 @@ SVGUseElement::IsAttributeMapped(const nsAtom* name) const {
                                                     sTextContentElementsMap,
                                                     sViewportsMap};
 
-  return FindAttributeDependence(name, map) ||
+  return name == nsGkAtoms::x || name == nsGkAtoms::y ||
+         FindAttributeDependence(name, map) ||
          SVGUseElementBase::IsAttributeMapped(name);
 }
 
+nsCSSPropertyID SVGUseElement::GetCSSPropertyIdForAttrEnum(uint8_t aAttrEnum) {
+  switch (aAttrEnum) {
+    case ATTR_X:
+      return eCSSProperty_x;
+    case ATTR_Y:
+      return eCSSProperty_y;
+    default:
+      // Currently we don't map width or height to style
+      return eCSSProperty_UNKNOWN;
+  }
+}
+
 }  // namespace dom
 }  // namespace mozilla

dom/svg/SVGUseElement.h

@@ -74,6 +74,8 @@ class SVGUseElement final : public SVGUseElementBase,
   virtual nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
 
+  static nsCSSPropertyID GetCSSPropertyIdForAttrEnum(uint8_t aAttrEnum);
+
   // WebIDL
   already_AddRefed<DOMSVGAnimatedString> Href();
   already_AddRefed<DOMSVGAnimatedLength> X();

layout/reftests/svg/dynamic-use-08.svg

@@ -0,0 +1,30 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     class="reftest-wait">
+
+  <title>Testing dynamic changes to use positioning</title>
+
+  <style>
+    .use {
+      x: 100%;
+    }
+  </style>
+  <defs>
+      <rect id="r" width="100%" height="100%" fill="red"/>
+  </defs>
+  <rect width="100%" height="100%" fill="lime"/>
+  <use id="u" xlink:href="#r"/>
+
+  <script>
+    function test() {
+      document.getElementById("u").setAttribute("class", "use");
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", test, false);
+    setTimeout(test, 4000); // fallback for running outside reftest
+  </script>
+</svg>

layout/reftests/svg/geometry-properties-in-css-ref.html

@@ -18,4 +18,8 @@
     </svg>
   </foreignObject>
   <rect x="300" y="260" width="50" height="50" fill="red" />
+  <defs>
+      <rect id="r3" width="80" height="80" fill="skyblue" />
+  </defs>
+  <use x="400" y="310" href="#r3"/>
 </svg>

layout/reftests/svg/geometry-properties-in-css.html

@@ -58,6 +58,10 @@
     y: 260px;
     height: 50px;
   }
+  use {
+    x:400px;
+    y:310px;
+  }
 </style>
 <svg>
   <g>
@@ -78,4 +82,11 @@
     </svg>
   </foreignObject>
   <image href="data:image/svg+xml,<svg width='10' height='10' xmlns='http://www.w3.org/2000/svg'><rect width='100%' height='100%' fill='red' /></svg>" />
+  <defs>
+    <g id="g">
+      <rect display="none"/>
+      <rect width="80" height="80" fill="skyblue" />
+    </g>
+  </defs>
+  <use href="#g"/>
 </svg>

layout/reftests/svg/reftest.list

@@ -192,6 +192,7 @@ random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == dynamic-textPath-03.svg
 == dynamic-use-05.svg pass.svg
 == dynamic-use-06.svg pass.svg
 == dynamic-use-07.svg pass.svg
+== dynamic-use-08.svg pass.svg
 random == dynamic-use-nested-01a.svg dynamic-use-nested-01-ref.svg
 random == dynamic-use-nested-01b.svg dynamic-use-nested-01-ref.svg
 == dynamic-use-remove-width.svg dynamic-use-remove-width-ref.svg

layout/svg/SVGUseFrame.cpp

@@ -11,8 +11,10 @@
 #include "mozilla/SVGUtils.h"
 #include "mozilla/dom/MutationEvent.h"
 #include "mozilla/dom/SVGUseElement.h"
+#include "SVGGeometryProperty.h"
 
 using namespace mozilla::dom;
+namespace SVGT = SVGGeometryProperty::Tags;
 
 //----------------------------------------------------------------------
 // Implementation
@@ -54,13 +56,23 @@ nsresult SVGUseFrame::AttributeChanged(int32_t aNamespaceID, nsAtom* aAttribute,
   return SVGGFrame::AttributeChanged(aNamespaceID, aAttribute, aModType);
 }
 
-void SVGUseFrame::PositionAttributeChanged() {
-  // make sure our cached transform matrix gets (lazily) updated
-  mCanvasTM = nullptr;
-  nsLayoutUtils::PostRestyleEvent(GetContent()->AsElement(), RestyleHint{0},
-                                  nsChangeHint_InvalidateRenderingObservers);
-  SVGUtils::ScheduleReflowSVG(this);
-  SVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+void SVGUseFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) {
+  SVGGFrame::DidSetComputedStyle(aOldComputedStyle);
+
+  if (!aOldComputedStyle) {
+    return;
+  }
+  const auto* newSVGReset = StyleSVGReset();
+  const auto* oldSVGReset = aOldComputedStyle->StyleSVGReset();
+
+  if (newSVGReset->mX != oldSVGReset->mX ||
+      newSVGReset->mY != oldSVGReset->mY) {
+    // make sure our cached transform matrix gets (lazily) updated
+    mCanvasTM = nullptr;
+    SVGObserverUtils::InvalidateRenderingObservers(this);
+    SVGUtils::ScheduleReflowSVG(this);
+    SVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+  }
 }
 
 void SVGUseFrame::DimensionAttributeChanged(bool aHadValidDimensions,
@@ -91,10 +103,8 @@ void SVGUseFrame::ReflowSVG() {
   // We only handle x/y offset here, since any width/height that is in force is
   // handled by the SVGOuterSVGFrame for the anonymous <svg> that will be
   // created for that purpose.
-  float x, y;
-  static_cast<SVGUseElement*>(GetContent())
-      ->GetAnimatedLengthValues(&x, &y, nullptr);
-  mRect.MoveTo(nsLayoutUtils::RoundGfxRectToAppRect(gfxRect(x, y, 0.0, 0.0),
+  auto [x, y] = ResolvePosition();
+  mRect.MoveTo(nsLayoutUtils::RoundGfxRectToAppRect(gfxRect(x, y, 0, 0),
                                                     AppUnitsPerCSSPixel())
                    .TopLeft());
 
@@ -111,9 +121,7 @@ void SVGUseFrame::NotifySVGChanged(uint32_t aFlags) {
   if (aFlags & COORD_CONTEXT_CHANGED && !(aFlags & TRANSFORM_CHANGED)) {
     // Coordinate context changes affect mCanvasTM if we have a
     // percentage 'x' or 'y'
-    SVGUseElement* use = static_cast<SVGUseElement*>(GetContent());
-    if (use->mLengthAttributes[SVGUseElement::ATTR_X].IsPercentage() ||
-        use->mLengthAttributes[SVGUseElement::ATTR_Y].IsPercentage()) {
+    if (StyleSVGReset()->mX.HasPercent() || StyleSVGReset()->mY.HasPercent()) {
       aFlags |= TRANSFORM_CHANGED;
       // Ancestor changes can't affect how we render from the perspective of
       // any rendering observers that we may have, so we don't need to
@@ -138,12 +146,18 @@ SVGBBox SVGUseFrame::GetBBoxContribution(const Matrix& aToBBoxUserspace,
       SVGDisplayContainerFrame::GetBBoxContribution(aToBBoxUserspace, aFlags);
 
   if (aFlags & SVGUtils::eForGetClientRects) {
-    float x, y;
-    static_cast<SVGUseElement*>(GetContent())
-        ->GetAnimatedLengthValues(&x, &y, nullptr);
+    auto [x, y] = ResolvePosition();
     bbox.MoveBy(x, y);
   }
   return bbox;
 }
 
+std::pair<float, float> SVGUseFrame::ResolvePosition() const {
+  auto* content = SVGUseElement::FromNode(GetContent());
+  return std::make_pair(SVGContentUtils::CoordToFloat(
+                            content, StyleSVGReset()->mX, SVGContentUtils::X),
+                        SVGContentUtils::CoordToFloat(
+                            content, StyleSVGReset()->mY, SVGContentUtils::Y));
+}
+
 }  // namespace mozilla

layout/svg/SVGUseFrame.h

@@ -34,9 +34,6 @@ class SVGUseFrame final : public SVGGFrame {
   void Init(nsIContent* aContent, nsContainerFrame* aParent,
             nsIFrame* aPrevInFlow) override;
 
-  // Called when the x or y attributes changed.
-  void PositionAttributeChanged();
-
   // Called when the href attributes changed.
   void HrefChanged();
 
@@ -45,7 +42,8 @@ class SVGUseFrame final : public SVGGFrame {
                                  bool aAttributeIsUsed);
 
   nsresult AttributeChanged(int32_t aNamespaceID, nsAtom* aAttribute,
-                            int32_t aModType) final;
+                            int32_t aModType) override;
+  void DidSetComputedStyle(ComputedStyle* aOldComputedStyle) override;
 
 #ifdef DEBUG_FRAME_DUMP
   nsresult GetFrameName(nsAString& aResult) const override {
@@ -60,6 +58,8 @@ class SVGUseFrame final : public SVGGFrame {
                               uint32_t aFlags) override;
 
  private:
+  std::pair<float, float> ResolvePosition() const;
+
   bool mHasValidDimensions;
 };
 

testing/web-platform/tests/svg/struct/use-getComputedStyle.html

@@ -0,0 +1,14 @@
+<!doctype html>
+<title>SVG Test: Resolved positioning inside use</title>
+<link rel="help" href="https://www.w3.org/TR/SVG2/struct.html#UseElement">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<svg>
+  <use id="use" x="1" y="2"/>
+</svg>
+<script>
+  test(() => {
+    assert_equals(getComputedStyle(use).x, "1px", "use element should have x computed to '1px'.");
+    assert_equals(getComputedStyle(use).y, "2px", "use element should have y computed to '2px'.");
+  }, "Test that we map use element positioning to style.");
+</script>