Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

document.currentScript from a script in a shadow tree. #477

Closed
hayatoito opened this issue Apr 1, 2016 · 41 comments
Closed

document.currentScript from a script in a shadow tree. #477

hayatoito opened this issue Apr 1, 2016 · 41 comments

Comments

@hayatoito
Copy link
Contributor

Regarding #377,

I thought that document.currentScript should return null if a running script is in a shadow tree,
however, given that "document.currentScript" is always executed by the script, this situation looks like : "the script is just accessing the script element which is running me".

It's like a this pointer... This does not seem to violate an encapsulation.

Thus, what document.currentScript should be? Is it okay to return a script element in a shadow tree?

@hayatoito
Copy link
Contributor Author

Per https://html.spec.whatwg.org/multipage/dom.html#dom-document-currentscript,

The currentScript attribute, on getting, must return the value to which it was most recently initialised. When the Document is created, the currentScript must be initialised to null.

Wait... As per the definition, I might be wrong. I forgot the case of an event listener.

1 A 'click' event listener is registered in a script in a document tree
2. <script> tag is added to a shadow tree
3. A user clicks something
4. In the click event listener, document.currentScript is accessed. => It returns the script in a shadow tree.

In this case, it looks that we would violate an encapsulation.

@hayatoito
Copy link
Contributor Author

Hmm. Per the Note:

Returns null if the Document is not currently executing a script or SVG script element (e.g., because the running script is an event handler, or a timeout).

In the event handler, it looks that document.currentScript should return null, anyway. So it does not violate an encapsulation.

Am I correct?

@annevk
Copy link
Collaborator

annevk commented Apr 1, 2016

What about the corresponding events?

Also, this would mean that if the executing script does anything within the parent, like call a function, it exposes the shadow tree in a rather trivial matter.

@hayatoito
Copy link
Contributor Author

Also, this would mean that if the executing script does anything within the parent, like call a function, it exposes the shadow tree in a rather trivial matter.

I see. That would cause an unintentional leak by calling a function such as:

var script;
void f() {
  script = document.currentScript;
  //...
}

I thought that we should let DocumentOrShadowRoot have currentScript, however, I am afraid that would not help at all. In most cases, the script needs to get the script element without any reference to its shadow root.

@annevk
Copy link
Collaborator

annevk commented Apr 1, 2016

That is a very good point, the script does not really know where it is, that is why we have this feature in the first place.

Maybe it's okay to have this and scripts just need to be careful. This is no isolation after all.

The events would still be troublesome however, but only Firefox implements those at the moment. @smaug----, thoughts?

@smaug----
Copy link

The outer world shouldn't know there is a shadow script being executed so document.currentScript should never point to a shadow <script>. And the events should be scoped.
Otherwise we just leak everything about shadow dom to the outside world.

I don't see any good way to support currentScript for shadow scripts, unless we execute shadow scripts somehow in a special scope where shadowCurrentScript or some such is explicitly exposed.

(yet another case where I think XBL2 had things right)

@annevk
Copy link
Collaborator

annevk commented Apr 1, 2016

@smaug---- it only points to the script while the script is executing though, no? So how can the outer world observe it?

@annevk
Copy link
Collaborator

annevk commented Apr 1, 2016

Also, what kind of feature would give the shadow script a pointer to itself?

@smaug----
Copy link

it only points to the script while the script is executing though, no? So how can the outer world observe it?

Just what was explained earlier. Shadow script calling some non-shadow function which accesses document.currentScript. That is accidental exposure, and it can happen when explicitly calling non-shadow functions or dispatching events from shadow script and handling them elsewhere.
The event handling case may even end up exposing shadow DOM to other shadow DOMs etc.

@annevk
Copy link
Collaborator

annevk commented Apr 1, 2016

So what is your solution?

@smaug----
Copy link

Also, what kind of feature would give the shadow script a pointer to itself?
Well, script execution would need to happen rather differently in shadow scripts, and we possibly don't want to do that. It would do effectively something like
(function(currentShadowScript) { /* the contents of the script would be here*/ } )(shadowScriptElement);

Hmm, but that brings in another question. What should happen to the variables defined in shadow scripts. Perhaps we should actually execute those scripts inside some anonymous scope.
(That would be similar to what has happened to Gecko internal frame scripts where we initially had scripts executing in the same scope but then moved to per script scope by default. The change was done when it became clear that different scripts were defining same variables and thus affecting to each others somewhat accidentally.)

@smaug----
Copy link

So what is your solution?

My current solution is to not expose shadow-script elements in document.currentScript and making events scoped. And let scripts to live with that limitation.

@hayatoito
Copy link
Contributor Author

I understand the point. However, I am afraid that we are worrying too much about this kind of exposure.
Unless the script (in a shadow tree) is calling such a trap function, this exposure never happens.

For me, it sounds "document.currentScript" is something like a thread-local variable. We do not have to stress on document used here. The point is that we have a global function which allows a script to access a thread-local variable.

It's difficult to decide where we should draw the line. However, in this particular case, I think it's okay that we have a global function which returns the script element which is currently running.

Due to the historical reason, this global function is defined in a "document".

I'm okay to restrict document.currentScript, however, we need an alternative global function for a script in a shadow tree, such as window.currentScript. WDYT?

My preferences:

[Preferred]

  1. Allow document.currentScript to return a script in a shadow tree (as long as the script is in a shadow-including document)
  2. Disallow document.currentScript to return a script in a shadow tree, but have an alternative, such as window.currentScript, to rescue a script in a shadow tree
  3. Disallow document.currentScript to return a script in a shadow tree. That's all.
    [Not preferred]

@domenic
Copy link
Collaborator

domenic commented Apr 4, 2016

Worrying about escape via shadow scripts calling non-shadow scripts seems similar to the many worries people have about closed shadow roots not being closed enough. (E.g., you can modify the running of script in those by overriding getters or setters on Object.prototype to inject your own logic.) The way to solve those kind of concerns is some kind of isolated scripting environment, in some subsequent "isolated shadow trees" proposal. But for open and closed shadow trees, I don't think it's worth trying to restrict. So I also support 1.

@rniwa
Copy link
Collaborator

rniwa commented Apr 4, 2016

What are use cases in which scripts inside a shadow DOM would need to use document.currentScript?

Until such use cases are presented, I'd assume such a feature is not needed. So I support option 3 since adding such a feature retroactively or reverting the behavior back to option 1 is easy while shipping option 1 today and changing it later to option 2 or option 3 would be backwards incompatible.

@hayatoito
Copy link
Contributor Author

The use case for a script in a shadow tree should be the same to a use case for a script in a document tree. Is there any difference between them?

@rniwa
Copy link
Collaborator

rniwa commented Apr 4, 2016

@hayatoito what are those use cases?

@hayatoito
Copy link
Contributor Author

document.currentScript.ownerDocument ? (as far as I spotted in the past...)

It looks there are a lot of other use cases, per http://stackoverflow.com/search?q=document.currentScript

@rniwa
Copy link
Collaborator

rniwa commented Apr 4, 2016

I'm not certain things listed there would apply to shadow DOM.

For example, http://stackoverflow.com/questions/32466974/using-document-currentscript-to-append-data-to-divs talks about appending data using attributes specified in the script element. Since there is no declarative syntax for shadow DOM, this wouldn't be a useful technique in practice. It would be much better to do the work using a custom element in the new web components world.

http://stackoverflow.com/questions/24986178/get-script-element-in-dom-tree/24986240, again, talks about loading JSON data from the server and generating a table out of it. That just doesn't happen with shadow DOM which lacks declarative syntax at the moment. Even if you had a declarative syntax, you wouldn't put data and script inside a shadow tree. You'd instead use a custom element with data inside of it, and then use shadowRoot.host to get that data instead.

http://stackoverflow.com/questions/19936425/dynamically-create-iframe-and-append-to-unknown-parent/19939766 is about creating an iframe when a script is loaded. Again, there is no need to insert a script element, then have it insert an iframe when creating a shadow tree. You should just do that upfront when you're creating a shadow tree. Alternatively, use a custom element to do it.

This post about "protecting" a JS object seems completely misguided: http://stackoverflow.com/questions/32739099/protecting-a-javascript-object-against-external-scripts/32739197

I don't think http://stackoverflow.com/questions/35608095/having-the-same-widget-on-a-page-multiple-time-variable-clashes/35608462#35608462 is anything to do with having to use document.currentScript. It's just badly written code overriding its own variables when ran multiple times. Using let for scoping would solve the problem, not document.currentScript.

http://stackoverflow.com/questions/403967/how-may-i-reference-the-script-tag-that-loaded-the-currently-executing-script/3695686 is about on-demand loading of other scripts within a script element, and I don't think you want to be doing that inside a shadow DOM either (it can result in each shadow DOM loading its own copy of the script). A better approach would be for whatever script creating a shadow tree to take care of it as its dependency. This is also a problem we ought be solving with ES6 modules and a new HTML-import replacement.

I've looked at a few other posts listed there but I don't think any of them benefit the use inside a shadow tree. They're either useless or actively harmful inside a shadow tree.

@smaug----
Copy link

We would we let document.currentScript to return anything in shadow DOM when we don't let event.target to do that. And in case of event dispatching, shadow script doesn't need to call
any "trap" function explicitly, but just dispatch an event in such way that light DOM can get access to it.

To be honest, I'm rather surprised anyone even questions the ''document.currentScript shouldn't return shadow script elements."

@annevk
Copy link
Collaborator

annevk commented Apr 4, 2016

@smaug---- isn't event.deepPath() exactly the same? If a shadow tree event listener invokes a function in the light tree that invokes event.deepPath() won't it similarly leak?

@smaug----
Copy link

That is quite different. event isn't any global object, so one needs to explicitly pass event in that state to some light DOM function. And no need to use deepPath() there. light DOM could just use event.currentTarget, or shadow listener could just pass some node to light dom function.

@annevk
Copy link
Collaborator

annevk commented Apr 4, 2016

@smaug---- quite a few implementations have window.event and that might yet leak to Gecko.

@smaug----
Copy link

I would expect window.event to be null when event.currentTarget points to some shadow DOM node.

@hayatoito
Copy link
Contributor Author

I'm wondering how a script in a shadow tree can access its root node (DocumentOrShadowRoot), if there is no way to access the script element.

If they can get the script element, they can access the root node easily, via currentScipt.rootNode, where a lot of useful APIs are available.

Thus, I'm wondering that window.currentRoot is what we should have? Just as a naive idea.
Is it okay to have such a context-aware API in Web? I am assuming that it's okay because we already have document.currentScript.

@esprehn
Copy link

esprehn commented Apr 4, 2016

The use cases for this are the same for the main document, ex. reading data off the <script> element.

<script src="..." data-option1="..." data-option2="..."></script>

if you appendChild that to your shadowRoot, document.currentScript is needed for the script to be able to read back option attributes. I don't think we can invent a new or fancier context aware property, that requires tagging every function to see what script created it.

@sjmiles
Copy link

sjmiles commented Apr 4, 2016

[Sorry I had verbiage here about the context of currentScript, but it was already covered, my bad].

The issue if I understand it then is primarily dispatching an event from shadow-root?

IMO, the leakage risk is less worrisome than unintended consequences of changing the behavior of currentScript from the traditional policy because of use-cases like Elliott mentions.

@rniwa
Copy link
Collaborator

rniwa commented Apr 4, 2016

The use cases for this are the same for the main document, ex. reading data off the <script> element.

<script src="..." data-option1="..." data-option2="..."></script>

Again, I don't think that's a useful thing to do inside a shadow DOM since that would mean that every instance of such a shadow DOM would try to fetch the script. Unless the script is cached and doesn't require validation, it would end up issuing a network request each time. A better approach would be using a custom element.

@travisleithead
Copy link
Member

I support not restricting this in any way (it may be a source of closed shadow root leakage, but that is not really a problem).

@domenic
Copy link
Collaborator

domenic commented Apr 5, 2016

Telecon consensus: return null always (both open and closed shadow trees)

@domenic
Copy link
Collaborator

domenic commented Apr 5, 2016

See also whatwg/html#997. We also discussed the desire for a new API in modules that introduces some kind of currentScript variable into the module scope, but doesn't put it on the global which everyone can access.

@hayatoito hayatoito added html-dom and removed v1 labels Apr 7, 2016
@rniwa
Copy link
Collaborator

rniwa commented May 5, 2016

I'm adding a test for this in web-platform-tests/wpt#2934.

@noopole
Copy link

noopole commented Mar 22, 2017

So the alternate proposal window.currentScript is thrown out with the bathwater too?

My need is not really the get the <script> element itself but some previous elements in the Shadow DOM tree, (or the shadow root, or the host).

Maybe there's a best approach that I missed (apart from using customized built-in elements... when they are implemented).

@LukasBombach
Copy link

LukasBombach commented Aug 7, 2017

+1 on accessing the shadow dom from a script node. How do you propose to do something like this

<template id="component">
<div id="some-div"></div>
<link href="/static/css/main.cacbacc7.css" rel="stylesheet">
<script src="/static/js/bundle.js" type="text/javascript"></script>
</template>

bundle.js

const myDom = window.currentScript.ownerDocument.querySelector('#some-div');

@rianby64
Copy link
Contributor

rianby64 commented Aug 7, 2017

If there's any chance to catch the currentShadowRoot instead of currentScript then, things like const myDom = window.currentScript.ownerDocument can be achieved faster. Usually, if you write currentScript then it follows te request for ownerDocument...

@rianby64
Copy link
Contributor

rianby64 commented Aug 7, 2017

Also, a good idea is to have something like

document.querySelector('#myshadowroot').defined.then(currentShadowRoot => {
});

In the same fashion as custom elements whenDefined does.

@TakayoshiKochi
Copy link
Member

Continuing discussion in the closed thread may not get attention from people.
Please open a new issue (with reference to this #477 is fine).

@annevk
Copy link
Collaborator

annevk commented Aug 9, 2017

There's no need for a new issue, this is tracked by whatwg/html#1013.

@tomalec
Copy link
Contributor

tomalec commented Dec 14, 2017

I'm sorry if I miss something. From reading this discussion I cannot see why document.currentScript violates the encapsulation in the way Custom Elements don't. As exposing shadow tree is trivial and could be done via various means, even without touching currentScript.

Consider the element:

customElements.define('expose-shadow-root',class extends HTMLElement{
    connectedCallback(){
        window.exposeShadowRoot = this.getRootNode();
    }
})

Then in putting <expose-shadow-root> into a shadow root is really violating it, but putting following script into shadow root is not.

<script>
    document.currentScript.getRootNode().getElementById('horn').addEventListener('click',()=>{alert('beep')});
</script>

However, non-violating use of currentScript is blocked while violating encapsulation via CE is not.

In my opinion as long as there is no JS encapsulation for custom elements, exposing shadow root programmatically will always be trivial.
But without currentScript accessing the currentRoot is blocked.

Yes, I know I can define and create an instance of a custom element instead of this script, but that's like using sledgehammer to crack a peanut.

@annevk
Copy link
Collaborator

annevk commented Dec 14, 2017

You can leak the shadow tree yourself, but if we provide platform APIs that leak the shadow tree it's just bad news.

@tomalec
Copy link
Contributor

tomalec commented Dec 14, 2017

How does platform API leaks itself? I put <script> document.currentScript</script> by myself in the same way I could put <expose-shadow-root> element by myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests