Closed Bug 646850 Opened 9 years ago Closed 9 years ago

When a length base value is non-px-valued & SMIL animation ends, value ends up at length in pixels, not in its real unit

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla5

People

(Reporter: dholbert, Assigned: bzbarsky)

References

()

Details

Attachments

(2 files)

STEPS TO REPRODUCE:
 1. Load testcase.  Watch animation.

EXPECTED RESULTS:
The circles match each other exactly in their horizontal movement -- in particular, they should both snap back to their original positions after the 1-second <set> completes.

ACTUAL RESULTS:
The red circle ends up at cx=0 (BAD), while the green circle ends up at cx=100 (good).

The only difference between the circles is that one uses 'em' units for its cx base value, while the other uses 'px' units.  We hit the same problem with 'cm' units, too. (I haven't tested others, but I'm assuming this affects all non-px-valued units.)
Attached image testcase 1
The problem is that nsSVGLength2::SMILLength::ClearAnimValue does this:

  mVal->SetAnimValue(mVal->mBaseVal, mSVGElement);

But SetAnimValue assumes that the argument is in user units, not in mSpecifiedUnitType units, as far as I can tell.
Or at least that's my best guess for "the problem".  Testing now.
Note also that we're ending up at cx=10, not cx=0, in the testcase... ;)
Summary: When a length base value is non-px-valued & SMIL animation ends, value ends up at 0 → When a length base value is non-px-valued & SMIL animation ends, value ends up at length in pixels, not in its real unit
(In reply to comment #2)
> The problem is that nsSVGLength2::SMILLength::ClearAnimValue does this:
> 
>   mVal->SetAnimValue(mVal->mBaseVal, mSVGElement);
> 
> But SetAnimValue assumes that the argument is in user units, not in
> mSpecifiedUnitType units, as far as I can tell.

Nice - that sounds exactly like the problem.
Comment on attachment 523345 [details] [diff] [review]
When resetting length to the pre-animation value, make sure to not lose track of our units.

Looks good! Thanks for fixing this.
Attachment #523345 - Flags: review?(dholbert) → review+
Assignee: nobody → bzbarsky
Whiteboard: [need landing]
http://hg.mozilla.org/mozilla-central/rev/06ed3a52effd
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla2.2
Verified with Mozilla/5.0 (X11; Linux i686; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.