Last Comment Bug 574603 - SVG: script should modify DOM animation
: SVG: script should modify DOM animation
Status: RESOLVED INVALID
: testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Jet Villegas (:jet)
Mentors:
http://honte.eu/temp/scriptDOMbug.svg
: 574769 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-25 02:20 PDT by jonathan chetwynd
Modified: 2010-06-25 13:49 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.17 KB, image/svg+xml)
2010-06-25 02:20 PDT, jonathan chetwynd
no flags Details
testcase (1.28 KB, image/svg+xml)
2010-06-25 05:06 PDT, jonathan chetwynd
no flags Details
original testcase with s/begin.click/0/ (1.16 KB, image/svg+xml)
2010-06-25 10:46 PDT, Daniel Holbert [:dholbert]
no flags Details

Description jonathan chetwynd 2010-06-25 02:20:53 PDT
Created attachment 453994 [details]
testcase

SVG 1.1
19.3 Animation using the SVG DOM

states:
If scripts are modifying the same attributes or properties that are being animated by SVG's animation elements, the scripts modify the base value for the animation. If a base value is modified while an animation element is animating the corresponding attribute or property, the animations are required to adjust dynamically to the new base value.

No testcase is currently linked, 
Please find attached a proposed testcase.
Comment 1 jonathan chetwynd 2010-06-25 02:23:24 PDT
Should DOM or script be notified?
Comment 2 jonathan chetwynd 2010-06-25 02:59:21 PDT
DOM animation tested using fakeSmile
Comment 3 Robert Longson 2010-06-25 03:21:52 PDT
Is this an issue without fakeSmile?
Comment 4 Daniel Holbert [:dholbert] 2010-06-25 03:26:42 PDT
> <set attributeName="fill" to="green" begin="fillGreen.click"   />

This animation never gets activated, because it depends on event-based SMIL timing, which we don't support. (that's bug 485157)

So the instruction to "click the green rectangle" in your testcase doesn't actually have any effect (in Firefox).

I'd expect that if you change the testcase to begin at a specific time value (e.g. 0), it should produce the expected results.
Comment 5 Daniel Holbert [:dholbert] 2010-06-25 03:27:30 PDT
(In reply to comment #4)
> I'd expect that if you change the testcase to begin at a specific time value
> (e.g. 0), it should produce the expected results.

er, s/change the testcase/change the <set> element/
Comment 6 jonathan chetwynd 2010-06-25 04:33:51 PDT
Daniel,

an attachment testcase would be appreciated.

I am using nightlies, the difficulty I have is understanding what is implemented:
https://developer.mozilla.org/En/SVG_in_Firefox
lists nothing

https://bugzilla.mozilla.org/show_bug.cgi?id=468996
no testcase but states fixed, 

etc.

do you have a current SMIL with SVG status uri?
Comment 7 jonathan chetwynd 2010-06-25 05:06:42 PDT
Created attachment 454007 [details]
testcase
Comment 8 jonathan chetwynd 2010-06-25 05:10:00 PDT
please note that animate begin does not appear to have been implemented, probably this should be tested to work before this bug is closed, even if set, the green rectangle, is not to be included.
Comment 9 jonathan chetwynd 2010-06-25 05:13:21 PDT
testcase uses animate to change fill, then script to change fill,

alert reports DOM fill as changed, but colour seen is not changed by script.
Comment 10 Robert Longson 2010-06-25 05:17:48 PDT
We've implemented set, we've not implemented begin="fillGreen.click" as stated in already in comment 4.
Comment 11 jonathan chetwynd 2010-06-25 08:55:19 PDT
Robert, thanks for  mentioning set, assertions are great, working testcases better, and a list of what's implemented might help, anyone might think this was a secret project....

as is evident from the new testcase the conclusions in #4 are plain wrong,
Comment 12 Robert Longson 2010-06-25 09:03:06 PDT
The list of what's implemented or not is here in bugzilla. What's missing are things listed as dependencies on the main animation bug i.e. bug 216462 That list is - some non length attributes, event based animations (which is what your example has), animateColor and seeking to before the current interval.

In your latest testcase you have this...
<set attributeName="fill" to="green" begin="fillGreen.click" />

So the conclusions in comment 4 still stand. This form of begin is not supported yet, it is event based animation.
Comment 13 jonathan chetwynd 2010-06-25 09:25:10 PDT
#12 Robert I'm wondering if you are demented? there are 2 tests in the one testcase, as mentioned in #8 the  2nd test is for begin=, however the first test is also broken, give it a whirl if it works for you we can limit this to ppc
the instructions  are   on the testcase, with further notes in  #9

still confused add to the bug spam, you are an expert!

cheers
Comment 14 Daniel Holbert [:dholbert] 2010-06-25 10:46:15 PDT
Created attachment 454077 [details]
original testcase with s/begin.click/0/

(In reply to comment #13)
> #12 Robert I'm wondering if you are demented?

Jonathan: Wow -- there's no need for such rudeness to Robert, particularly when we've already told you what's causing the testcase to not function in Firefox.  

Since you're so intent on us making the change to the testcase for you, I've done so here, to avoid further back-and-forth.

(In reply to comment #6)
> do you have a current SMIL with SVG status uri?

Brian Birtles maintains a more detailed SMIL-specific list here:
http://brian.sol1.net/svg/status.php
Comment 15 Daniel Holbert [:dholbert] 2010-06-25 10:47:49 PDT
Resolving as dupe of bug 485157, since that's the thing preventing this bug's original testcases from working.

*** This bug has been marked as a duplicate of bug 485157 ***
Comment 16 jonathan chetwynd 2010-06-25 12:43:51 PDT
Guys this is ridiculous this bug has nothing to do with event handling.

filing a new bug to avoid further  spam
Comment 17 Daniel Holbert [:dholbert] 2010-06-25 12:47:13 PDT
(In reply to comment #16)
> Guys this is ridiculous this bug has nothing to do with event handling.

Jonathan: Both of your testcases rely on "begin.click" to trigger the <set>.  That is event-based SMIL timing, which we don't support. I don't know how I can be any clearer.

> filing a new bug to avoid further  spam

Please don't, if it's about this same testcase.
Comment 18 jonathan chetwynd 2010-06-25 13:00:05 PDT
Daniel,

its not the same testcase, but the testcase in  this bug does not  rely on  begin.click, that is the secondary test, now removed in new bug.

the primary test is animate changes fill then script does not change fill
please check, confirmed for ppc.

I also dont know how to be any clearer

appreciated!!
Comment 19 jonathan chetwynd 2010-06-25 13:10:56 PDT
fwiw I took this to the WG at the time of filing 
http://lists.w3.org/Archives/Public/www-svg/2010Jun/0187.html

at the least it would be helpful to have a testcase  for the specification, which is my proposal, of course it does need a good testcase, and explanation
Comment 20 Daniel Holbert [:dholbert] 2010-06-25 13:14:36 PDT
(In reply to comment #18)
> the primary test is animate changes fill then script does not change fill
> please check, confirmed for ppc.

Ok, I think I understand you -- I think you're saying that in attachment 454007 [details] (your latest testcase here), you expect a click on the circle to have a visible effect, but it doesn't.

The current behavior is actually correct -- it's because your <animate> node has a "from" attribute, which masks the base value in the animation sandwich model.  So basically, the script changes the underlying value, but it (correctly) doesn't get a chance to "shine through" because there's an animation in effect, and this particular animation (from-to) doesn't care about the underlying value.

If you changed the testcase to use a "by" animation (with no from attribute), then the scripted change to 'fill' *would* have a visible effect, because by animations add to (instead of replacing) the underlying value.

Note also that if you specify attributeType="XML" on your animation, then the animation will affect the XML attribute as opposed to the CSS property, and your alert message (which queries the *attribute*) will produce a value that matches what you see.
Comment 21 Daniel Holbert [:dholbert] 2010-06-25 13:17:45 PDT
(In reply to comment #19)
> fwiw I took this to the WG at the time of filing 
> http://lists.w3.org/Archives/Public/www-svg/2010Jun/0187.html

The title of your attachment there:
>	<title>Script should over-ride set attributeName="fill" testcase</ 
title>
is incorrect.  Scripted modifications change the DOM, which affects the base values of animations that care about base values (e.g. 'by' animations).  However, scripted changes do not *override* animations.  In particular, in cases of animations that mask the underlying attribute-value (as <set> does), the scripted changes won't have a visible effect.
Comment 22 jonathan chetwynd 2010-06-25 13:22:41 PDT
Daniel,

what doesn't make sense to me, and is being reported, is that after 5 seconds, once the animation is finished the script still has no effect.

whereas had there been no animation the script obviously would have an effect.
is this  the intention? seem bizarre if it is.

the rationale during an animation is possible, but after one seems odd to me.
Comment 23 Daniel Holbert [:dholbert] 2010-06-25 13:30:16 PDT
(In reply to comment #22)
> what doesn't make sense to me, and is being reported, is that after 5 seconds,
> once the animation is finished the script still has no effect.

That happens because you have fill="freeze" on the <animate> node, which makes it continue to apply its final value as the animated value, indefinitely.

If you remove the fill="freeze", then the animation will run its course and then stop having any effects whatsoever once it's done.
Comment 24 Daniel Holbert [:dholbert] 2010-06-25 13:39:01 PDT
Note that the <set> has the same effect in my modified version of your first testcase -- i.e. that <set> element applies its animated value indefinitely, too.

In that case, it's not because of a fill="freeze" (since that's not present) but rather because the dur attribute isn't set, and so it defaults to "indefinite", which means the animation lasts forever.
Comment 25 jonathan chetwynd 2010-06-25 13:40:17 PDT
Daniel,

many thanks, In had considered that, when adding the fill="freeze" but as this is the obvious way  to get declarative animation to have an after effect, seems hard to avoid.

guess they weren't made to mix and match...

cheers again
Comment 26 Daniel Holbert [:dholbert] 2010-06-25 13:48:07 PDT
No problem, glad to help.

Un-duping, since the main misunderstanding didn't end up being about event-based timing after all, and resolving as INVALID since it was a misunderstanding rather than a bug.
Comment 27 Daniel Holbert [:dholbert] 2010-06-25 13:49:20 PDT
*** Bug 574769 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.