Bug 1502658 - Remove SVG use attribute change handling code from nsSVGUseFrame to SVGUseElement. r=longsonr, a=RyanVM
Most of those shouldn't rely on a frame to run.
Differential Revision:
https://phabricator.services.mozilla.com/D9996
--- a/dom/svg/SVGUseElement.cpp
+++ b/dom/svg/SVGUseElement.cpp
@@ -87,16 +87,68 @@ SVGUseElement::~SVGUseElement()
!OwnerDoc()->SVGUseElementNeedsShadowTreeUpdate(*this),
"Dying without unbinding?"
);
}
//----------------------------------------------------------------------
// nsINode methods
+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) {
+ const bool hadValidDimensions = HasValidDimensions();
+ const bool isUsed = OurWidthAndHeightAreUsed();
+ if (isUsed) {
+ SyncWidthOrHeight(aAttribute);
+ }
+
+ if (auto* frame = GetFrame()) {
+ frame->DimensionAttributeChanged(hadValidDimensions, isUsed);
+ }
+ }
+ }
+
+ if ((aNamespaceID == kNameSpaceID_XLink ||
+ aNamespaceID == kNameSpaceID_None) &&
+ aAttribute == nsGkAtoms::href) {
+ // We're changing our nature, clear out the clone information.
+ if (auto* frame = GetFrame()) {
+ frame->HrefChanged();
+ }
+ mOriginal = nullptr;
+ UnlinkSource();
+ TriggerReclone();
+ }
+}
+
+nsresult
+SVGUseElement::AfterSetAttr(int32_t aNamespaceID,
+ nsAtom* aAttribute,
+ const nsAttrValue* aValue,
+ const nsAttrValue* aOldValue,
+ nsIPrincipal* aSubjectPrincipal,
+ bool aNotify)
+{
+ ProcessAttributeChange(aNamespaceID, aAttribute);
+ return SVGUseElementBase::AfterSetAttr(aNamespaceID,
+ aAttribute,
+ aValue,
+ aOldValue,
+ aSubjectPrincipal,
+ aNotify);
+}
+
nsresult
SVGUseElement::Clone(dom::NodeInfo* aNodeInfo, nsINode** aResult) const
{
*aResult = nullptr;
SVGUseElement *it = new SVGUseElement(do_AddRef(aNodeInfo));
nsCOMPtr<nsINode> kungFuDeathGrip(it);
nsresult rv1 = it->Init();
@@ -174,17 +226,17 @@ SVGUseElement::CharacterDataChanged(nsIC
{
if (nsContentUtils::IsInSameAnonymousTree(this, aContent)) {
TriggerReclone();
}
}
void
SVGUseElement::AttributeChanged(Element* aElement,
- int32_t aNameSpaceID,
+ int32_t aNamespaceID,
nsAtom* aAttribute,
int32_t aModType,
const nsAttrValue* aOldValue)
{
if (nsContentUtils::IsInSameAnonymousTree(this, aElement)) {
TriggerReclone();
}
}
--- a/dom/svg/SVGUseElement.h
+++ b/dom/svg/SVGUseElement.h
@@ -82,16 +82,28 @@ public:
nsIURI* GetSourceDocURI();
URLExtraData* GetContentURLData() const { return mContentURLData; }
// Updates the internal shadow tree to be an up-to-date clone of the
// referenced element.
void UpdateShadowTree();
+ // Shared code between AfterSetAttr and nsSVGUseFrame::AttributeChanged.
+ //
+ // This is needed because SMIL doesn't go through AfterSetAttr unfortunately.
+ void ProcessAttributeChange(int32_t aNamespaceID, nsAtom* aAttribute);
+
+ nsresult AfterSetAttr(int32_t aNamespaceID,
+ nsAtom* aAttribute,
+ const nsAttrValue* aValue,
+ const nsAttrValue* aOldValue,
+ nsIPrincipal* aSubjectPrincipal,
+ bool aNotify) final;
+
protected:
/**
* Helper that provides a reference to the element with the ID that is
* referenced by the 'use' element's 'href' attribute, and that will update
* the 'use' element if the element that that ID identifies changes to a
* different element (or none).
*/
class ElementTracker final : public IDTracker {
--- a/layout/svg/nsSVGUseFrame.cpp
+++ b/layout/svg/nsSVGUseFrame.cpp
@@ -1,16 +1,17 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "nsSVGUseFrame.h"
+#include "mozilla/dom/MutationEvent.h"
#include "mozilla/dom/SVGUseElement.h"
#include "SVGObserverUtils.h"
using namespace mozilla;
using namespace mozilla::dom;
//----------------------------------------------------------------------
// Implementation
@@ -36,69 +37,68 @@ nsSVGUseFrame::Init(nsIContent* aC
mHasValidDimensions =
static_cast<SVGUseElement*>(aContent)->HasValidDimensions();
nsSVGGFrame::Init(aContent, aParent, aPrevInFlow);
}
nsresult
-nsSVGUseFrame::AttributeChanged(int32_t aNameSpaceID,
+nsSVGUseFrame::AttributeChanged(int32_t aNamespaceID,
nsAtom* aAttribute,
int32_t aModType)
{
- auto* useElement = static_cast<SVGUseElement*>(GetContent());
-
- if (aNameSpaceID == kNameSpaceID_None) {
- if (aAttribute == nsGkAtoms::x || aAttribute == nsGkAtoms::y) {
- // make sure our cached transform matrix gets (lazily) updated
- mCanvasTM = nullptr;
- nsLayoutUtils::PostRestyleEvent(
- useElement, nsRestyleHint(0),
- nsChangeHint_InvalidateRenderingObservers);
- nsSVGUtils::ScheduleReflowSVG(this);
- nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
- } else if (aAttribute == nsGkAtoms::width ||
- aAttribute == nsGkAtoms::height) {
- bool invalidate = false;
- if (mHasValidDimensions != useElement->HasValidDimensions()) {
- mHasValidDimensions = !mHasValidDimensions;
- invalidate = true;
- }
-
- // FIXME(emilio): This shouldn't really belong to nsSVGUseFrame, but
- // SVGUseElement.
- if (useElement->OurWidthAndHeightAreUsed()) {
- invalidate = true;
- useElement->SyncWidthOrHeight(aAttribute);
- }
-
- if (invalidate) {
- nsLayoutUtils::PostRestyleEvent(
- useElement, nsRestyleHint(0),
- nsChangeHint_InvalidateRenderingObservers);
- nsSVGUtils::ScheduleReflowSVG(this);
- }
- }
+ // Currently our SMIL implementation does not modify the DOM attributes. Once
+ // we implement the SVG 2 SMIL behaviour this can be removed
+ // SVGUseElement::AfterSetAttr's implementation will be sufficient.
+ if (aModType == MutationEvent_Binding::SMIL) {
+ auto* content = SVGUseElement::FromNode(GetContent());
+ content->ProcessAttributeChange(aNamespaceID, aAttribute);
}
- if ((aNameSpaceID == kNameSpaceID_XLink ||
- aNameSpaceID == kNameSpaceID_None) &&
- aAttribute == nsGkAtoms::href) {
- // we're changing our nature, clear out the clone information
+ return nsSVGGFrame::AttributeChanged(aNamespaceID, aAttribute, aModType);
+}
+
+void
+nsSVGUseFrame::PositionAttributeChanged()
+{
+ // make sure our cached transform matrix gets (lazily) updated
+ mCanvasTM = nullptr;
+ nsLayoutUtils::PostRestyleEvent(
+ GetContent()->AsElement(), nsRestyleHint(0),
+ nsChangeHint_InvalidateRenderingObservers);
+ nsSVGUtils::ScheduleReflowSVG(this);
+ nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+}
+
+void
+nsSVGUseFrame::DimensionAttributeChanged(bool aHadValidDimensions,
+ bool aAttributeIsUsed)
+{
+ bool invalidate = aAttributeIsUsed;
+ if (mHasValidDimensions != aHadValidDimensions) {
+ mHasValidDimensions = !mHasValidDimensions;
+ invalidate = true;
+ }
+
+ if (invalidate) {
nsLayoutUtils::PostRestyleEvent(
- useElement, nsRestyleHint(0),
+ GetContent()->AsElement(), nsRestyleHint(0),
nsChangeHint_InvalidateRenderingObservers);
nsSVGUtils::ScheduleReflowSVG(this);
- useElement->mOriginal = nullptr;
- useElement->UnlinkSource();
- useElement->TriggerReclone();
}
+}
- return nsSVGGFrame::AttributeChanged(aNameSpaceID, aAttribute, aModType);
+void
+nsSVGUseFrame::HrefChanged()
+{
+ nsLayoutUtils::PostRestyleEvent(
+ GetContent()->AsElement(), nsRestyleHint(0),
+ nsChangeHint_InvalidateRenderingObservers);
+ nsSVGUtils::ScheduleReflowSVG(this);
}
//----------------------------------------------------------------------
// nsSVGDisplayableFrame methods
void
nsSVGUseFrame::ReflowSVG()
{
--- a/layout/svg/nsSVGUseFrame.h
+++ b/layout/svg/nsSVGUseFrame.h
@@ -26,19 +26,29 @@ protected:
public:
NS_DECL_FRAMEARENA_HELPERS(nsSVGUseFrame)
// nsIFrame interface:
void Init(nsIContent* aContent,
nsContainerFrame* aParent,
nsIFrame* aPrevInFlow) override;
- nsresult AttributeChanged(int32_t aNameSpaceID,
+ // Called when the x or y attributes changed.
+ void PositionAttributeChanged();
+
+ // Called when the href attributes changed.
+ void HrefChanged();
+
+ // Called when the width or height attributes changed.
+ void DimensionAttributeChanged(bool aHadValidDimensions,
+ bool aAttributeIsUsed);
+
+ nsresult AttributeChanged(int32_t aNamespaceID,
nsAtom* aAttribute,
- int32_t aModType) override;
+ int32_t aModType) final;
#ifdef DEBUG_FRAME_DUMP
nsresult GetFrameName(nsAString& aResult) const override
{
return MakeFrameName(NS_LITERAL_STRING("SVGUse"), aResult);
}
#endif
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/svg/linking/reftests/use-hidden-attr-change.html
@@ -0,0 +1,32 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>use element reacts to attribute changes when it's not rendered</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://svgwg.org/svg2-draft/struct.html#UseElement">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1502658">
+<link rel="match" href="/svg/linking/reftests/use-descendant-combinator-ref.html">
+<style>
+</style>
+<p>
+ You should see a green square, and no red.
+</p>
+<svg
+ version="1.1"
+ xmlns="http://www.w3.org/2000/svg"
+ xmlns:xlink="http://www.w3.org/1999/xlink"
+ style="display: none">
+ <defs>
+ <g id="square">
+ <rect width="100" height="100" fill="green" />
+ </g>
+ </defs>
+ <g id="test">
+ <use />
+ </g>
+</svg>
+<script>
+ onload = () => {
+ document.querySelector("use").setAttributeNS("http://www.w3.org/1999/xlink", "href", "#square");
+ document.querySelector("svg").style.display = "";
+ }
+</script>