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

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

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.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Or at least that's my best guess for "the problem".  Testing now.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Note also that we're ending up at cx=10, not cx=0, in the testcase... ;)

Comment 5

[Daniel Holbert [:dholbert]]

(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 6

[Boris Zbarsky [:bzbarsky]]

Attachment: When resetting length to the pre-animation value, make sure to not lose track of our units.

Comment 7

[Daniel Holbert [:dholbert]]

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.

Comment 8

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/mozilla-central/rev/06ed3a52effd

Comment 9

[George Carstoiu]

Verified with Mozilla/5.0 (X11; Linux i686; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre