Bug 86376 - Incorrect cloning behavior for scripts: "already started" flag not copied
Summary: Incorrect cloning behavior for scripts: "already started" flag not copied
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: EasyFix
Depends on:
Blocks:
 
Reported: 2012-05-14 08:35 PDT by Boris Zbarsky
Modified: 2012-05-18 12:11 PDT (History)
6 users (show)

See Also:


Attachments
testcase (286 bytes, text/html)
2012-05-14 08:35 PDT, Boris Zbarsky
no flags Details
Patch (6.01 KB, patch)
2012-05-14 17:27 PDT, Pablo Flouret
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.