Closed Bug 615666 Opened 11 years ago Closed 11 years ago

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


(Core :: SVG, defect)

Not set





(Reporter: marek.raida, Assigned: longsonr)





(3 files)

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)
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.)
OS: Windows 7 → All
Hardware: x86 → All
Summary: SMIL animation does not work with percent units → SMIL animation does not work with percent units for "offset" attr on <stop> elements
Version: unspecified → Trunk
Ever confirmed: true
blocking2.0: --- → ?
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.
We could add a PRPackedBool mPercentagesAllowed to the NumberInfo struct in nsSVGElement.h and just read that off in nsSVGNumber2::SMILNumber::ValueFromString
Sounds like a plan. :)
Assignee: nobody → longsonr
Attached patch patchSplinter Review
Attachment #495251 - Flags: review?(jwatt)
Comment on attachment 495251 [details] [diff] [review]

Nice. :) r=jwatt
Attachment #495251 - Flags: review?(jwatt) → review+
Attachment #495251 - Flags: approval2.0?
blocking2.0: ? → ---
Whiteboard: [need approval]
Comment on attachment 495251 [details] [diff] [review]

> IsPercentagesAllowedNumber

NumberAllowsPercentages sounds better
Attachment #495251 - Flags: approval2.0? → approval2.0+
(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.

Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [need approval]
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.