Bug 104221 – Add support for document.currentScript

• Component: DOM
• Status: RESOLVED FIXED

[Syoichi Tsuyuhara]

>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

[Arko Saha]

Created attachment 186725 [details]
Patch

[Darin Adler]

Comment on attachment 186725 [details]
Patch

I’d also like to see some test cases that cover multiple documents, particularly subframes.

[Build Bot]

Comment on attachment 186725 [details]
Patch

Attachment 186725 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16393005

[Adam Barth]

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.

[Adam Barth]

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>

[Syoichi Tsuyuhara]

Blink supports this.
https://src.chromium.org/viewvc/blink?revision=152230&view=revision
https://src.chromium.org/viewvc/blink?revision=152237&view=revision

[Kangil Han]

I will merge those two patches. :)

[Ryosuke Niwa]

Please announce this feature on webkit-dev first: http://www.webkit.org/coding/adding-features.html

[Kangil Han]

(In reply to comment #8)
> Please announce this feature on webkit-dev first: http://www.webkit.org/coding/adding-features.html

Done, thanks! :)

[Kangil Han]

Created attachment 204815 [details]
Patch

[Build Bot]

Comment on attachment 204815 [details]
Patch

Attachment 204815 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/858412

[Build Bot]

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

[Kangil Han]

Created attachment 204877 [details]
Patch

[Ryosuke Niwa]

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?

[Kangil Han]

Created attachment 205261 [details]
Patch

[Kangil Han]

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

[Build Bot]

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

[Build Bot]

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

[Kangil Han]

(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. :)

[WebKit Commit Bot]

Comment on attachment 205261 [details]
Patch

Clearing flags on attachment: 205261

Committed r151951: <http://trac.webkit.org/changeset/151951>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.