Bug 19391 – Dynamically created SVG script tags fail to execute

• Component: SVG
• Status: RESOLVED DUPLICATE

[jay]

open the uri, after three seconds, alternative text should be loaded from javascript in external php file.

parity: ff & Opera

[jay]

it maybe this issue is not solely related to SVG...

[Oliver Hunt]

Okay this issue is caused by SVGScriptElement not implementing insertedIntoDocument and related-fu, which means that they never execute once they're inserted into the dom.  In fact the current implementation appears to do nothing but manage scripts that are referenced at parse time -- dynamically updating the href tag also does not appear to do the required update logic.

[Rob Buis]

Created attachment 22258 [details]
First attempt

This patch is not finished since it doesnt handle for instance "childrenChanged".
Also I am not sure how to exactly dispatch the error and load events. Still there
are two nice testcases that already are getting passed, so I thought I'd try to get some early feedback :)
Cheers,

Rob.

[jay]

in a fit of pique, well more absent-mindedness it seems I had broken the reduced testcase...

it's temporarily fixed, so grab it while you can...

tx again

~:"

[Oliver Hunt]

Comment on attachment 22258 [details]
First attempt

One observation: SVGScriptElement::evaluateScript has commented out code.

However i don't believe this is correct in all cases.  the HTMLScriptElement has logic to prevent a script from being executed multiple times when attached and detached.

I'm wondering if it's worth making a ScriptElement class that has all the logic to handle attah/detach/execution, etc, although that will require multiple inheritance which will be yuck

and may screw html performance :-/

[Rob Buis]

Created attachment 22259 [details]
Some more tests

This patch also tries setting the href attr dynamically and changing the text
children of the script. The behaviour is the same as for html scripts since
I noticed FF handling the svg scripts that way.
Cheers,

Rob.

[Oliver Hunt]

Comment on attachment 22259 [details]
Some more tests

I'd like to see a patch with finishParsingChildren implemented as htmlscriptelement has, or a reason it shouldn't be implemented for svg.

Also i'd like to see tests for
* script element created, added, removed, and added again
* script element in source that is removed the added again
* script element created, added, removed, children added, re-added to the document
* ditto for a script in the source
* script created, given a source, added, removed, soure attr removed
* script created, given a source, added, removed, soure attr removed, source placed in the element, then added to the document again.
* script element (both created, and in the source) getting its source set, and changed

I think that covers it

[Oliver Hunt]

Rob, did you ever get round to writing those tests?

[Rob Buis]

Hi Oliver,

(In reply to comment #8)
> Rob, did you ever get round to writing those tests?

Still working on it once in a while. It's quite boring work actually :(
Nevertheless I'll have something to review this week, until then you can r-
if you want to clean the review queue a bit more.
Cheers,

Rob.

[Oliver Hunt]

Well the patch itself is fine, it just needs a few more tests.  Hmm, you can commit, right?

[Eric Seidel (no email)]

Yes, Rob can commit & review! :)

[Eric Seidel (no email)]

Comment on attachment 22259 [details]
Some more tests

I think it's better to be explicit about what scriptTag it is in XMLTokenizer, I would prefer you leave the HTMLNames:: when the next line is SVGNames::scriptTag. :)

Looks fine.  Wow, I wish that SVGScriptElement and HTMLScriptElement could share more code.  I think our current split solution is going to lead to lots of copy/paste bugs.

[Nikolas Zimmermann]

*** This bug has been marked as a duplicate of 20372 ***