802890 - SVG/SMIL: nsSVGAnimationElement::UpdateHrefTarget should probably use ResetWithId instead of Reset()

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

Just realized that targeted <animate> elements (which we've supported since bug 491080 in 2009) should probably be using nsReferencedElement::ResetWithId() (added in bug 474743 in 2010), instead of calling Reset() with a custom-created URI.

As noted at http://www.w3.org/TR/SVG11/animate.html#HrefAttribute , "The target element must be part of the current SVG document fragment", which means ResetWithId() is a reasonable thing to use.  (It assumes same-document.)

Comment 1

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=f3dba336b00f772f71bd285924c2a20d1470b0bb

Comment 2

[Robert Longson [:longsonr]]

Attachment: Bug 802890 - SVGAnimationElement::UpdateHrefTarget should call ResetWithID rather than ResetToURIFragmentID as animation targets must be in the local document r=emilio

Comment 3

[Robert Longson [:longsonr]]

Should we take this opportunity to change the API of ResetWithID so that it takes a const nsAString& rather than an atom. There are two existing callers in the tree https://searchfox.org/mozilla-central/search?q=ResetWithID&path=&case=true&regexp=false and both start with a string that they convert to an atom. We then convert that atom back into a string: https://searchfox.org/mozilla-central/source/dom/base/IDTracker.cpp#131 except that we need to assign an atom to mWatchID but we do that internally in IDTracker's other API i.e. https://searchfox.org/mozilla-central/source/dom/base/IDTracker.cpp#124

Comment 4

[Robert Longson [:longsonr]]

Actually it's not that straightforward to use nsAString here. https://searchfox.org/mozilla-central/source/dom/smil/SMILParserUtils.cpp#347

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0714a9f1f8f
SVGAnimationElement::UpdateHrefTarget should call ResetWithID rather than ResetToURIFragmentID as animation targets must be in the local document r=emilio

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d0714a9f1f8f