Bug 86376

Summary: Incorrect cloning behavior for scripts: "already started" flag not copied
Product: WebKit Reporter: Boris Zbarsky <bzbarsky>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, koivisto, pf, rniwa, webkit.review.bot
Priority: P2 Keywords: EasyFix
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
testcase
none
Patch none

Description Boris Zbarsky 2012-05-14 08:35:01 PDT
Created attachment 141736 [details]
testcase

See attached testcase.  It should say "PASS".

The relevant spec is at http://www.whatwg.org/specs/web-apps/current-work/#already-started and says:

  The cloning steps for script elements must set the "already started" flag on the copy if it is
  set on the element being cloned.
Comment 1 Adam Barth 2012-05-14 14:33:01 PDT
This should be easy to fix.
Comment 2 Boris Zbarsky 2012-05-14 15:52:48 PDT
One other note: ideally the SVG behavior would be fixed along with the HTML one, though of course the HTML spec can't require behavior for SVG scripts...
Comment 3 Pablo Flouret 2012-05-14 17:27:14 PDT
Created attachment 141824 [details]
Patch
Comment 4 Pablo Flouret 2012-05-14 17:37:10 PDT
I missed the SVG comment, i'll see if i can make a testcase and fix if needed.
Comment 5 Pablo Flouret 2012-05-15 15:35:58 PDT
There's already http://trac.webkit.org/browser/trunk/LayoutTests/svg/dom/SVGScriptElement/script-clone-rerun.svg?format=txt which tests that "already started" is copied over when the element is cloned, but the bug here wasn't in the cloning code itself, it was just observable through it.

I'm not very proficient in svg, so if anyone thinks the actual bug here could also be an issue in svg i'd appreciate a testcase or a pointer on how to make one.
Comment 6 Boris Zbarsky 2012-05-15 15:55:22 PDT
There's a testcase at https://bugzilla.mozilla.org/attachment.cgi?id=621309 that shows an <svg:script> running after being imported from a DOMParser-generated document.

I assume the attached patch fixes that situation for <html:script>, right?
Comment 7 Pablo Flouret 2012-05-15 17:45:43 PDT
(In reply to comment #6)
> There's a testcase at https://bugzilla.mozilla.org/attachment.cgi?id=621309 that shows an <svg:script> running after being imported from a DOMParser-generated document.
> 
> I assume the attached patch fixes that situation for <html:script>, right?

WebKit doesn't support text/html in DOMParser yet, afaik. But that testcase does indeed fail for svg and xhtml.

Implementation-wise, the xml case is a bit trickier to fix since DOMParser uses Document::createElement, which uses the automatically generated element constructors. Also, is there even enough info at that point to decide whether 'already started' should be set or not?

Adam, any ideas?

Btw, should there also be a note for the xml case here:
http://html5.org/specs/dom-parsing.html#domparser
Comment 8 Boris Zbarsky 2012-05-15 18:24:45 PDT
If I read the specs right, the "already started" flag is set whenever the element is inserted into a document...  That should include DOMParser and XMLHttpRequest automatically.

By the way, WebKit seems to fail the similar XMLHttpRequest testcase:

test.xml:
  <root>
    <script xmlns="http://www.w3.org/1999/xhtml">
      document.body.textContent = 'FAIL';
    </script>
  </root>

test.html:

  <!DOCTYPE html>
  <body>PASS
    <script>
      var xhr = new XMLHttpRequest();
      xhr.open("GET", "test.xml", false);
      xhr.send();
      document.body.appendChild(
        document.importNode(xhr.responseXML.querySelector("script"), true));
    </script>
  </body>
Comment 10 WebKit Review Bot 2012-05-18 12:11:41 PDT
Comment on attachment 141824 [details]
Patch

Clearing flags on attachment: 141824

Committed r117611: <http://trac.webkit.org/changeset/117611>
Comment 11 WebKit Review Bot 2012-05-18 12:11:50 PDT
All reviewed patches have been landed.  Closing bug.