512636 - SVGTransform.setMatrix() should copy the matrix argument, not adopt it

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090825 Minefield/3.7a1pre
Build Identifier: 

This is covered by an SVG 1.1 erratum:

  http://www.w3.org/2003/01/REC-SVG11-20030114-errata#mention-live-values

which has been incorporated into the 1.1SE editor's draft:

  http://dev.w3.org/SVG/profiles/1.1F2/publish/coords.html#__svg__SVGTransform__matrix

Reproducible: Always

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Make SVGTransform.setMatrix() copy its argument

This also removes a comment from GetMatrix() wondering whether a copy of the matrix should be returned.  SVGTransform.matrix should be live, so a copy shouldn't be returned.

Not sure if I like the six get[ABCDEF]()s followed by the six set[ABCDEF]()s.  I was thinking of adding an Assign() method to nsSVGMatrix, but then I saw that the type of the SVGTransform.setMatrix() argument was nsIDOMSVGMatrix, and I don't know enough about how the interface system works to get an nsSVGMatrix from that.  (Is a static_cast<> good enough?)

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Make SVGTransform.setMatrix() copy its argument

Remove unnecessary <div> from the test.

Comment 3

[Jonathan Watt [:jwatt]]

Comment on attachment 396668 [details] [diff] [review]
Make SVGTransform.setMatrix() copy its argument

Great, thanks Cam. :-)

>+  // check that setMatrix() took a copy of m
>+  is(m != m2, true, 't.matrix identity');

You could just use ok() here instead of is().

Comment 4

[Robert Longson [:longsonr]]

checked in http://hg.mozilla.org/mozilla-central/rev/56e45901831a