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

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

unspecified
x86
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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
Created attachment 396667 [details] [diff] [review]
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?)
Attachment #396667 - Flags: review?(jwatt)
Created attachment 396668 [details] [diff] [review]
Make SVGTransform.setMatrix() copy its argument

Remove unnecessary <div> from the test.
Attachment #396667 - Attachment is obsolete: true
Attachment #396668 - Flags: review?(jwatt)
Attachment #396667 - Flags: review?(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().
Attachment #396668 - Flags: review?(jwatt) → review+

Updated

9 years ago
Assignee: nobody → cam
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

9 years ago
Status: NEW → ASSIGNED
checked in http://hg.mozilla.org/mozilla-central/rev/56e45901831a
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Blocks: 539576
You need to log in before you can comment on or make changes to this bug.