Bug 104221 - Add support for document.currentScript
Summary: Add support for document.currentScript
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kangil Han
URL: http://whatwg.org/html#dom-document-c...
Keywords: BlinkMergeCandidate, HTML5, WebExposed
Depends on:
Blocks: 32934
  Show dependency treegraph
 
Reported: 2012-12-05 22:41 PST by Syoichi Tsuyuhara
Modified: 2014-01-29 09:05 PST (History)
22 users (show)

See Also:


Attachments
Patch (7.52 KB, patch)
2013-02-05 16:45 PST, Arko Saha
no flags Details | Formatted Diff | Diff
Patch (16.14 KB, patch)
2013-06-17 06:05 PDT, Kangil Han
no flags Details | Formatted Diff | Diff
Patch (16.20 KB, patch)
2013-06-17 19:32 PDT, Kangil Han
no flags Details | Formatted Diff | Diff
Patch (18.73 KB, patch)
2013-06-22 22:53 PDT, Kangil Han
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from APPLE-EWS-6 for win-future (797.72 KB, application/zip)
2013-06-23 01:32 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Syoichi Tsuyuhara 2012-12-05 22:41:46 PST
>document.currentScript
>
>Returns the script element that is currently executing. In the case of reentrant script execution, returns the one that most recently started executing amongst those that have not yet finished executing.
>Returns null if the Document is not currently executing a script element (e.g. because the running script is an event handler, or a timeout).
http://html.spec.whatwg.org/multipage/dom.html#dom-document-currentscript
http://html5.org/r/7551

Firefox 4+(Gecko 2+) only supported.
https://bugzilla.mozilla.org/show_bug.cgi?id=587931
https://developer.mozilla.org/en-US/docs/DOM/document.currentScript
Comment 1 Arko Saha 2013-02-05 16:45:43 PST
Created attachment 186725 [details]
Patch
Comment 2 Darin Adler 2013-02-05 16:48:28 PST
Comment on attachment 186725 [details]
Patch

Iā€™d also like to see some test cases that cover multiple documents, particularly subframes.
Comment 3 Build Bot 2013-02-05 19:38:39 PST
Comment on attachment 186725 [details]
Patch

Attachment 186725 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16393005
Comment 4 Adam Barth 2013-02-05 19:53:14 PST
Comment on attachment 186725 [details]
Patch

Alos, we should test defer, async, and scripts run by nested calls to document.write.  We should also test scripts running in SVG-in-HTML in all those permutations.
Comment 5 Adam Barth 2013-02-05 19:55:58 PST
Comment on attachment 186725 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186725&action=review

> Source/WebCore/dom/Document.cpp:552
> +    m_currentScript = 0;

This should be part of the initializer list.

> Source/WebCore/dom/Document.h:1573
> +    HTMLScriptElement* m_currentScript;

How do you know this pointer remains valid through the script's execution?  Perhaps this should be a RefPtr.

> Source/WebCore/dom/Document.idl:178
> +    readonly attribute HTMLScriptElement currentScript;

What happens when we're executing a non-HTML script element?

> Source/WebCore/dom/ScriptElement.cpp:301
> +    if (m_element->isHTMLElement())
> +        document->setCurrentScript(static_cast<HTMLScriptElement*>(m_element));

Please also test the cases of nesting HTML and non-HTML script elements.

> Source/WebCore/dom/ScriptElement.cpp:313
> +    document->setCurrentScript(oldScriptElement);

How do you know oldScriptElement still points of a valid element?  Perhaps oldScriptElement needs to be a RefPtr<HTMLScriptElement>
Comment 7 Kangil Han 2013-06-17 00:15:02 PDT
I will merge those two patches. :)
Comment 8 Ryosuke Niwa 2013-06-17 00:16:47 PDT
Please announce this feature on webkit-dev first: http://www.webkit.org/coding/adding-features.html
Comment 9 Kangil Han 2013-06-17 03:12:07 PDT
(In reply to comment #8)
> Please announce this feature on webkit-dev first: http://www.webkit.org/coding/adding-features.html

Done, thanks! :)
Comment 10 Kangil Han 2013-06-17 06:05:42 PDT
Created attachment 204815 [details]
Patch
Comment 11 Build Bot 2013-06-17 07:05:06 PDT
Comment on attachment 204815 [details]
Patch

Attachment 204815 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/858412
Comment 12 Build Bot 2013-06-17 11:19:17 PDT
Comment on attachment 204815 [details]
Patch

Attachment 204815 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/865197
Comment 13 Kangil Han 2013-06-17 19:32:10 PDT
Created attachment 204877 [details]
Patch
Comment 14 Ryosuke Niwa 2013-06-20 19:11:42 PDT
Comment on attachment 204877 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204877&action=review

> Source/WebCore/dom/ScriptElement.cpp:285
> +static bool isHTMLScriptElement(Element* element)

Add inline maybe?

> Source/WebCore/dom/ScriptElement.cpp:290
> +static bool isSVGScriptElement(Element* element)

Ditto.

> Source/WebCore/dom/ScriptElement.cpp:323
> +            if (isHTMLScriptElement(m_element))
> +                document->pushCurrentScript(toHTMLScriptElement(m_element));
> +

Don't we need to do this in HTMLScriptRunner::executePendingScriptAndDispatchEvent?
Also, can we use a RAII object to do this? Perhaps we can rename IgnoreDestructiveWriteCountIncrementer and do it there?
Comment 15 Kangil Han 2013-06-22 22:53:44 PDT
Created attachment 205261 [details]
Patch
Comment 16 Kangil Han 2013-06-22 23:04:15 PDT
Took rniwa's comment into consideration. :)

(In reply to comment #14)
> (From update of attachment 204877 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204877&action=review
> 
> > Source/WebCore/dom/ScriptElement.cpp:285
> > +static bool isHTMLScriptElement(Element* element)
> 
> Add inline maybe?

Done.

> 
> > Source/WebCore/dom/ScriptElement.cpp:290
> > +static bool isSVGScriptElement(Element* element)
> 
> Ditto.

Done.

> 
> > Source/WebCore/dom/ScriptElement.cpp:323
> > +            if (isHTMLScriptElement(m_element))
> > +                document->pushCurrentScript(toHTMLScriptElement(m_element));
> > +
> 
> Don't we need to do this in HTMLScriptRunner::executePendingScriptAndDispatchEvent?
> Also, can we use a RAII object to do this? Perhaps we can rename IgnoreDestructiveWriteCountIncrementer and do it there?

IMHO,
  1. As note describes, this is the point where script is actually run
  2. IgnoreDestructiveWriteCountIncrementer is for http://www.whatwg.org/specs/web-apps/current-work/#ignore-destructive-writes-counter so document is manipulated as specification
Comment 17 Build Bot 2013-06-23 01:32:55 PDT
Comment on attachment 205261 [details]
Patch

Attachment 205261 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/980061

New failing tests:
fast/media/mq-transform-03.html
fast/forms/search-event-delay.html
fast/media/mq-transform-02.html
platform/win/accessibility/multiple-select-element-role.html
Comment 18 Build Bot 2013-06-23 01:32:58 PDT
Created attachment 205265 [details]
Archive of layout-test-results from APPLE-EWS-6 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-6  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 19 Kangil Han 2013-06-23 17:44:10 PDT
(In reply to comment #17)
> (From update of attachment 205261 [details])
> Attachment 205261 [details] did not pass win-ews (win):
> Output: http://webkit-queues.appspot.com/results/980061
> 
> New failing tests:
> fast/media/mq-transform-03.html
> fast/forms/search-event-delay.html
> fast/media/mq-transform-02.html
> platform/win/accessibility/multiple-select-element-role.html

I don't think this patch is related to above failed cases. :)
Comment 20 WebKit Commit Bot 2013-06-24 19:39:26 PDT
Comment on attachment 205261 [details]
Patch

Clearing flags on attachment: 205261

Committed r151951: <http://trac.webkit.org/changeset/151951>
Comment 21 WebKit Commit Bot 2013-06-24 19:39:30 PDT
All reviewed patches have been landed.  Closing bug.