Summary: | Dynamically created SVG script tags fail to execute | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | jay <jay> | ||||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | eric, mjs, oliver | ||||||
Priority: | P2 | Keywords: | HasReduction | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
URL: | http://www.openicon.org/temp/safari-phpJS-broken.svg | ||||||||
Attachments: |
|
Description
jay
2008-06-04 06:48:07 PDT
it maybe this issue is not solely related to SVG... 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. 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.
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 ~:" 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 :-/
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.
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
Rob, did you ever get round to writing those tests? 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. Well the patch itself is fine, it just needs a few more tests. Hmm, you can commit, right? Yes, Rob can commit & review! :) 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.
*** This bug has been marked as a duplicate of 20372 *** |