Bug 1486488 - Don't assume that SVGAnimationElement has a parent on bind. r=dholbert

This is a regression from bug 1450250, which removed a if (!GetCx()) early
return in this function.

That early return was wrong, both because it prevented elements that were in
shadow trees from targeting other elements, but also because that check was not
present in AfterSetAttr, which means that dynamic updates to the attribute would
work.

Pass the SVGAnimationElement itself to resolve references. That's what we do for
attribute mutations, and also it's the same behavior we have, since the ID
lookup IDTracker does only depends on containing shadow root and containing
document, and that's invariant between a kid and it's DOM parent.

Some other code has been updated to take references instead of pointers so the
null-safety of those methods is explicit.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2018-08-28 09:06:08 +00:00
Родитель 8e0cd59f7c
Коммит a1f03332c2
9 изменённых файлов: 56 добавлений и 49 удалений

Просмотреть файл

@ -144,11 +144,8 @@ IDTracker::Reset(nsIContent* aFromContent,
}
void
IDTracker::ResetWithID(nsIContent* aFromContent,
nsAtom* aID,
bool aWatch)
IDTracker::ResetWithID(Element& aFrom, nsAtom* aID, bool aWatch)
{
MOZ_ASSERT(aFromContent);
MOZ_ASSERT(aID);
if (aWatch) {
@ -158,7 +155,7 @@ IDTracker::ResetWithID(nsIContent* aFromContent,
mReferencingImage = false;
DocumentOrShadowRoot* docOrShadow = DocOrShadowFromContent(*aFromContent);
DocumentOrShadowRoot* docOrShadow = DocOrShadowFromContent(aFrom);
HaveNewDocumentOrShadowRoot(docOrShadow, aWatch, nsDependentAtomString(aID));
}

Просмотреть файл

@ -78,7 +78,7 @@ public:
* changes, so ElementChanged won't fire and get() will always return the same
* value, the current element for the ID.
*/
void ResetWithID(nsIContent* aFrom, nsAtom* aID, bool aWatch = true);
void ResetWithID(Element& aFrom, nsAtom* aID, bool aWatch = true);
/**
* Clears the reference. ElementChanged is not triggered. get() will return

Просмотреть файл

@ -57,7 +57,7 @@ nsSMILTimeValueSpec::~nsSMILTimeValueSpec()
nsresult
nsSMILTimeValueSpec::SetSpec(const nsAString& aStringSpec,
Element* aContextNode)
Element& aContextElement)
{
nsSMILTimeValueSpecParams params;
@ -80,23 +80,21 @@ nsSMILTimeValueSpec::SetSpec(const nsAString& aStringSpec,
mParams.mEventSymbol = nsGkAtoms::repeatEvent;
}
ResolveReferences(aContextNode);
ResolveReferences(aContextElement);
return NS_OK;
}
void
nsSMILTimeValueSpec::ResolveReferences(nsIContent* aContextNode)
nsSMILTimeValueSpec::ResolveReferences(Element& aContextElement)
{
if (mParams.mType != nsSMILTimeValueSpecParams::SYNCBASE && !IsEventBased())
if (mParams.mType != nsSMILTimeValueSpecParams::SYNCBASE && !IsEventBased()) {
return;
MOZ_ASSERT(aContextNode,
"null context node for resolving timing references against");
}
// If we're not bound to the document yet, don't worry, we'll get called again
// when that happens
if (!aContextNode->IsInComposedDoc())
if (!aContextElement.IsInComposedDoc())
return;
// Hold ref to the old element so that it isn't destroyed in between resetting
@ -105,7 +103,7 @@ nsSMILTimeValueSpec::ResolveReferences(nsIContent* aContextNode)
RefPtr<Element> oldReferencedElement = mReferencedElement.get();
if (mParams.mDependentElemID) {
mReferencedElement.ResetWithID(aContextNode, mParams.mDependentElemID);
mReferencedElement.ResetWithID(aContextElement, mParams.mDependentElemID);
} else if (mParams.mType == nsSMILTimeValueSpecParams::EVENT) {
Element* target = mOwner->GetTargetElement();
mReferencedElement.ResetWithElement(target);

Просмотреть файл

@ -48,9 +48,9 @@ public:
nsSMILTimeValueSpec(nsSMILTimedElement& aOwner, bool aIsBegin);
~nsSMILTimeValueSpec();
nsresult SetSpec(const nsAString& aStringSpec, Element* aContextNode);
void ResolveReferences(nsIContent* aContextNode);
bool IsEventBased() const;
nsresult SetSpec(const nsAString& aStringSpec, Element& aContextElement);
void ResolveReferences(Element& aContextElement);
bool IsEventBased() const;
void HandleNewInterval(nsSMILInterval& aInterval,
const nsSMILTimeContainer* aSrcContainer);

Просмотреть файл

@ -853,20 +853,21 @@ namespace
} // namespace
bool
nsSMILTimedElement::SetAttr(nsAtom* aAttribute, const nsAString& aValue,
nsSMILTimedElement::SetAttr(nsAtom* aAttribute,
const nsAString& aValue,
nsAttrValue& aResult,
Element* aContextNode,
Element& aContextElement,
nsresult* aParseResult)
{
bool foundMatch = true;
nsresult parseResult = NS_OK;
if (aAttribute == nsGkAtoms::begin) {
parseResult = SetBeginSpec(aValue, aContextNode, RemoveNonDOM);
parseResult = SetBeginSpec(aValue, aContextElement, RemoveNonDOM);
} else if (aAttribute == nsGkAtoms::dur) {
parseResult = SetSimpleDuration(aValue);
} else if (aAttribute == nsGkAtoms::end) {
parseResult = SetEndSpec(aValue, aContextNode, RemoveNonDOM);
parseResult = SetEndSpec(aValue, aContextElement, RemoveNonDOM);
} else if (aAttribute == nsGkAtoms::fill) {
parseResult = SetFillMode(aValue);
} else if (aAttribute == nsGkAtoms::max) {
@ -928,10 +929,10 @@ nsSMILTimedElement::UnsetAttr(nsAtom* aAttribute)
nsresult
nsSMILTimedElement::SetBeginSpec(const nsAString& aBeginSpec,
Element* aContextNode,
Element& aContextElement,
RemovalTestFunction aRemove)
{
return SetBeginOrEndSpec(aBeginSpec, aContextNode, true /*isBegin*/,
return SetBeginOrEndSpec(aBeginSpec, aContextElement, true /*isBegin*/,
aRemove);
}
@ -944,10 +945,10 @@ nsSMILTimedElement::UnsetBeginSpec(RemovalTestFunction aRemove)
nsresult
nsSMILTimedElement::SetEndSpec(const nsAString& aEndSpec,
Element* aContextNode,
Element& aContextElement,
RemovalTestFunction aRemove)
{
return SetBeginOrEndSpec(aEndSpec, aContextNode, false /*!isBegin*/,
return SetBeginOrEndSpec(aEndSpec, aContextElement, false /*!isBegin*/,
aRemove);
}
@ -1205,7 +1206,7 @@ nsSMILTimedElement::IsTimeDependent(const nsSMILTimedElement& aOther) const
}
void
nsSMILTimedElement::BindToTree(nsIContent* aContextNode)
nsSMILTimedElement::BindToTree(Element& aContextElement)
{
// Reset previously registered milestone since we may be registering with
// a different time container now.
@ -1225,12 +1226,12 @@ nsSMILTimedElement::BindToTree(nsIContent* aContextNode)
// Resolve references to other parts of the tree
uint32_t count = mBeginSpecs.Length();
for (uint32_t i = 0; i < count; ++i) {
mBeginSpecs[i]->ResolveReferences(aContextNode);
mBeginSpecs[i]->ResolveReferences(aContextElement);
}
count = mEndSpecs.Length();
for (uint32_t j = 0; j < count; ++j) {
mEndSpecs[j]->ResolveReferences(aContextNode);
mEndSpecs[j]->ResolveReferences(aContextElement);
}
}
@ -1304,7 +1305,7 @@ nsSMILTimedElement::Unlink()
nsresult
nsSMILTimedElement::SetBeginOrEndSpec(const nsAString& aSpec,
Element* aContextNode,
Element& aContextElement,
bool aIsBegin,
RemovalTestFunction aRemove)
{
@ -1323,7 +1324,7 @@ nsSMILTimedElement::SetBeginOrEndSpec(const nsAString& aSpec,
bool hadFailure = false;
while (tokenizer.hasMoreTokens()) {
auto spec = MakeUnique<nsSMILTimeValueSpec>(*this, aIsBegin);
nsresult rv = spec->SetSpec(tokenizer.nextToken(), aContextNode);
nsresult rv = spec->SetSpec(tokenizer.nextToken(), aContextElement);
if (NS_SUCCEEDED(rv)) {
timeSpecsList.AppendElement(std::move(spec));
} else {
@ -1481,13 +1482,13 @@ nsSMILTimedElement::RebuildTimingState(RemovalTestFunction aRemove)
if (mAnimationElement->HasAttr(nsGkAtoms::begin)) {
nsAutoString attValue;
mAnimationElement->GetAttr(nsGkAtoms::begin, attValue);
SetBeginSpec(attValue, mAnimationElement, aRemove);
SetBeginSpec(attValue, *mAnimationElement, aRemove);
}
if (mAnimationElement->HasAttr(nsGkAtoms::end)) {
nsAutoString attValue;
mAnimationElement->GetAttr(nsGkAtoms::end, attValue);
SetEndSpec(attValue, mAnimationElement, aRemove);
SetEndSpec(attValue, *mAnimationElement, aRemove);
}
mPrevRegisteredMilestone = sMaxMilestone;

Просмотреть файл

@ -60,7 +60,7 @@ public:
* Returns the element targeted by the animation element. Needed for
* registering event listeners against the appropriate element.
*/
mozilla::dom::Element* GetTargetElement();
Element* GetTargetElement();
/**
* Methods for supporting the ElementTimeControl interface.
@ -265,8 +265,8 @@ public:
* @param aValue The attribute value.
* @param aResult The nsAttrValue object that may be used for storing the
* parsed result.
* @param aContextNode The element to use for context when resolving
* references to other elements.
* @param aContextElement The element to use for context when resolving
* references to other elements.
* @param[out] aParseResult The result of parsing the attribute. Will be set
* to NS_OK if parsing is successful.
*
@ -274,8 +274,8 @@ public:
* otherwise.
*/
bool SetAttr(nsAtom* aAttribute, const nsAString& aValue,
nsAttrValue& aResult, Element* aContextNode,
nsresult* aParseResult = nullptr);
nsAttrValue& aResult, Element& aContextElement,
nsresult* aParseResult = nullptr);
/**
* Attempts to unset an attribute on this timed element.
@ -326,18 +326,17 @@ public:
* Called when the timed element has been bound to the document so that
* references from this timed element to other elements can be resolved.
*
* @param aContextNode The node which provides the necessary context for
* resolving references. This is typically the element in
* the host language that owns this timed element. Should
* not be null.
* @param aContextElement The element which provides the necessary context for
* resolving references. This is typically the element
* in the host language that owns this timed element.
*/
void BindToTree(nsIContent* aContextNode);
void BindToTree(Element& aContextElement);
/**
* Called when the target of the animation has changed so that event
* registrations can be updated.
*/
void HandleTargetElementChange(mozilla::dom::Element* aNewTarget);
void HandleTargetElementChange(Element* aNewTarget);
/**
* Called when the timed element has been removed from a document so that
@ -377,10 +376,10 @@ protected:
//
nsresult SetBeginSpec(const nsAString& aBeginSpec,
Element* aContextNode,
Element& aContextElement,
RemovalTestFunction aRemove);
nsresult SetEndSpec(const nsAString& aEndSpec,
Element* aContextNode,
Element& aContextElement,
RemovalTestFunction aRemove);
nsresult SetSimpleDuration(const nsAString& aDurSpec);
nsresult SetMin(const nsAString& aMinSpec);
@ -401,7 +400,7 @@ protected:
void UnsetFillMode();
nsresult SetBeginOrEndSpec(const nsAString& aSpec,
Element* aContextNode,
Element& aContextElement,
bool aIsBegin,
RemovalTestFunction aRemove);
void ClearSpecs(TimeValueSpecList& aSpecs,

Просмотреть файл

@ -185,7 +185,7 @@ SVGAnimationElement::BindToTree(nsIDocument* aDocument,
UpdateHrefTarget(hrefStr);
}
mTimedElement.BindToTree(aParent);
mTimedElement.BindToTree(*this);
}
AnimationNeedsResample();
@ -234,7 +234,7 @@ SVGAnimationElement::ParseAttribute(int32_t aNamespaceID,
// try to parse it.
if (!foundMatch) {
foundMatch =
mTimedElement.SetAttr(aAttribute, aValue, aResult, this, &rv);
mTimedElement.SetAttr(aAttribute, aValue, aResult, *this, &rv);
}
if (foundMatch) {

Просмотреть файл

@ -0,0 +1,11 @@
<script>
function start() {
document.replaceChild(id_0, document.childNodes[0]);
}
</script>
<body onload="start()">
<svg>
<animate id="id_0" begin="s" />
<ellipse id="id_1" />
</svg>
</body>

Просмотреть файл

@ -88,3 +88,4 @@ load 1402798.html
load 1419250-1.html
load 1420492.html
load 1477853.html
load 1486488.html