615666 - SMIL animation does not work with percent units for "offset" attr on <stop> elements

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Marek Raida]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101130 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101130 Firefox/4.0b8pre

SMIL animation units does not support percents and must be converted to floats to work. I found many animation related unit issues but not identified exactly this one,so sorry, if it is duplicity of some other bug, I;m creating it to be sure to have it fixed...

Reproducible: Always

Steps to Reproduce:
1. Open enclosed testcase
2. Gradient of the circle seems static, but should be animated

Actual Results:  
static rendering

Expected Results:  
animation of stop color of radial gradient

according to spec and other browser implementation, percents should behave in exactly the same way as floating units (in other testcase)

Comment 1

[Marek Raida]

Attachment: Testcase for broken behavior

Comment 2

[Marek Raida]

Attachment: Modified testcase to use floats, which works

Comment 3

[Daniel Holbert [:dholbert]]

Confirmed in today's nightly.
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101130 Firefox/4.0b8pre

(Updating summary -- the testcase shows that this is broken for the <stop> "offset" attribute, but percent units generally work fine for other attributes, e.g. CSS-backed attributes.)

Comment 4

[Jonathan Watt [:jwatt]]

I don't think it should block since you can use 0.xx instead of xx%, but it would be nice to fix. Problem is the parsing code doesn't know whether it should be allowing the <number> it is parsing to be followed by a "%" or not.

Comment 5

[Robert Longson [:longsonr]]

We could add a PRPackedBool mPercentagesAllowed to the NumberInfo struct in nsSVGElement.h and just read that off in nsSVGNumber2::SMILNumber::ValueFromString

Comment 6

[Jonathan Watt [:jwatt]]

Sounds like a plan. :)

Comment 7

[Robert Longson [:longsonr]]

Attachment: patch

Comment 8

[Jonathan Watt [:jwatt]]

Comment on attachment 495251 [details] [diff] [review]
patch

Nice. :) r=jwatt

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 495251 [details] [diff] [review]
patch

> IsPercentagesAllowedNumber

NumberAllowsPercentages sounds better

Comment 10

[Jonathan Watt [:jwatt]]

(In reply to comment #9)
> NumberAllowsPercentages sounds better

I was going to remark on that in my review but decided not to bother. Given your comment I changed it to NumberAttrAllowsPercentage for checkin though.

Pushed http://hg.mozilla.org/mozilla-central/rev/b1a4d82fb98a