-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Clarify "settings object" things when calling a custom element constructor #2381
Comments
Specifically, what needs handling is entry/incumbent stuff around the Construct() invocations. |
I think most of the time these are invoked synchronously from code that has already set up things, but I'll do an audit. |
Seems like we'd at least want to set the incumbent to whatever set up the custom element definition, etc... |
Consider following examples: <!-- a.html -->
<!DOCTYPE html>
<iframe src="b.html"></iframe>
<script>
class UnresolvedElement extends frames[0].HTMLElement {};
frames[0].customElements.define('unresolved-element', UnresolvedElement);
</script>
<!-- b.html -->
<!DOCTYPE html>
<unresolved-element></unresolved-element>
[1] https://html.spec.whatwg.org/multipage/scripting.html#element-definition and <!-- a.html -->
<!DOCTYPE html>
<iframe src="b.html"></iframe>
<script>
class UnresolvedElement extends frames[0].HTMLElement {};
frames[0].defineCustomElement(UnresolvedElement);
</script>
<!-- b.html -->
<!DOCTYPE html>
<unresolved-element></unresolved-element>
<script>
window.defineCustomElement = (definition) => {
customElements.define('unresolved-element', definition);
};
</script>
Above two examples have different incumbent when custom element constructor is invoked (If my understanding is correct). Are the incumbent setting objects expected in above two examples? |
I suppose we should probably make the incumbents match. I'm a bit loathe to put more work into the incumbent mess, but it's better to be consistent with the rest of the spec I guess. Do you have a similar example where #concept-create-element might be affected, or just upgrades so far? |
High-level question: are we any closer to figuring out what incumbent means, if anything, in non-Gecko implementations? Last time we discussed this nobody really knew and they might not even have such a concept, so I'm wondering if we're just keeping Gecko technical debt alive because we don't really know what else to do? |
Non-Gecko implementations certainly have a concept of "incumbent" (e.g. they do it for postMessage if you call it in the simple way). It's just really ad-hoc and kinda broken in some of them.
Do you mean the concept of incumbent at all, or its implementation? The concept is required for Window.postMessage to work at all. The implementation is not too complicated if you've already made the hard case (which is the normal case!) work. |
And maybe the point here is that there are no constructors in the web platform which examine the incumbent, in which case it doesn't so much matter what we do with it around the Construct() call. Using a bound postMessage will throw if you try to Construct() it, of course. |
Apologies, I didn't realize there is one observable point where we are in fact interoperable. Do we have bugs filed against Chrome/Edge/WebKit to implement incumbent properly then? |
It's possible that the incumbent stuff in Location is interoperable too, in the simple cases. Would need to test... I can't speak to bugs existing or not. |
similar examples for #concept-create-element, <!-- a.html -->
<!DOCTYPE html>
<iframe src="b.html"></iframe>
<script>
class MyCustomElement extends frames[0].HTMLElement {};
frames[0].customElements.define('my-custom-element', MyCustomElement);
frames[0].document.createElement('my-custom-element');
</script>
<!-- b.html -->
<!-- simply be an empty document -->
[1] https://dom.spec.whatwg.org/#dom-document-createelement and <!-- a.html -->
<!DOCTYPE html>
<iframe src="b.html"></iframe>
<script>
class MyCustomElement extends frames[0].HTMLElement {};
frames[0].customElements.define('my-custom-element', MyCustomElement);
frames[0].createCustomElement('my-custom-element');
</script>
<!-- b.html -->
<!DOCTYPE html>
<script>
window.createCustomElement = (name) => {
document.createElement(name);
};
</script>
|
Thanks @EdgarChen, I'll work on fixing.
Well, it matters because it runs the |
Yes, but at that point the incumbent will be whatever the global of the Realm of the MyCustomElement constructor is, assuming that constructor is not a built-in function. The incumbent setup around calling into script is only needed when directly invoking built-in functions, basically. Any time you've made it out into an actual scripted function, your incumbent is now the global of that function. |
@domenic did you file any when you write web-platform-tests? If not, if you can point me to a web-platform-tests test that cover this, I'm happy to do it. I'd really like Chrome and WebKit engineers to get involved somehow in this ongoing issue. |
Right, OK... Hmm. So this issue doesn't need any fix? There are certainly no constructors on the web platform that inspect the incumbent and will also pass the requirements of Or do you think we should still fix it, because relying on this kind of tenuous connection between parts of the system to uphold our invariants is fragile? If we fix it in that case though, it sounds like it's an un-testable change, right? I hope I'm not completely lost here :)
I don't think we're quite there yet. We need to work through the rest of #1430 first, writing tests to discover which of those cases actually use incumbent (my guess is only ~3 of them will end up doing so, across 2+ browsers). Once we have an actual list of places on the platform that definitely use incumbent everywhere, we'll have a good test suite. And then we can work on fleshing out that test suite to cover edge cases that only Gecko gets right. |
Apart from https://html.spec.whatwg.org/multipage/comms.html#dom-broadcastchannel but I guess that can never be a subclass of HTMLElement? Or can it, if people mess with the proto chains? |
Ugh... upon further inspection, it looks like registering BroadcastChannel as an element constructor will pass through define(). Any attempts to create an element with the registered tag name will then call Construct(BroadcastChannel), and inspect the returned value, notice it doesn't implement the HTMLElement brand check, and throw. But BroadcastChannel will still be called... I guess in this case we get lucky since passing no arguments to BroadcastChannel will immediately throw, before it inspects the incumbent? So again, very fragile... probably we should fix this, but doing so will be un-testable. Right? (Of course this all assumes BroadcastChannel actually uses the incumbent settings object in implementations, which remains to be proven.) |
Yes. I can't figure out a way to do the equivalent of bind() with ctors in ES; I think there is nothing right now. But that doesn't mean something won't get added. :( I agree having it not be testable is quite unfortunate.
It sure seems to in Gecko, fwiw. I can't speak for others. |
I think there is an observable consequence here besides just settings object stuff. Namely, we currently don't {prepare to, clean up after} {running script, calling a callback}. In particular this means that a microtask checkpoint does not happen at the proper time. That seems bad; it should be happening after the constructor. This observable consequence was noticed at https://bugs.chromium.org/p/chromium/issues/detail?id=703802. Blink seems to already trigger a microtask checkpoint here. We should fix this and write web platform tests. |
This helps move some of the heavy lifting into IDL, to avoid issues like whatwg/html#2381.
This fixes part of #2381 (the other part is in DOM's "create an element").
Fixes part of whatwg/html#2381. The other part is covered by whatwg/html#2457.
Fixes part of whatwg/html#2381. The other part is covered by whatwg/html#2457.
This fixes part of #2381 (the other part is in DOM's "create an element").
This helps move some of the heavy lifting into IDL, to avoid issues like whatwg/html#2381.
Fixes part of whatwg/html#2381. The other part is covered by whatwg/html#2457.
This fixes part of #2381 (the other part is in DOM's "create an element"). Note this has two consequences, as discussed there: * Properly managing the entry and incumbent concepts. This is technically unobservable in the current spec landscape. * Ensuring microtasks are run, if the construct is initiated with an empty stack (such as during the parser).
Fixes part of whatwg/html#2381. The other part is covered by whatwg/html#2457. Tests: web-platform-tests/wpt#5208
This fixes part of #2381 (the other part is in DOM's "create an element"). Note this has two consequences, as discussed there: * Properly managing the entry and incumbent concepts. This is technically unobservable in the current spec landscape. * Ensuring microtasks are run, if the construct is initiated with an empty stack (such as during the parser). Tests: web-platform-tests/wpt#5208
Fixed by whatwg/dom#430 and #2457 with tests at web-platform-tests/wpt#5208 :). |
This fixes part of whatwg#2381 (the other part is in DOM's "create an element"). Note this has two consequences, as discussed there: * Properly managing the entry and incumbent concepts. This is technically unobservable in the current spec landscape. * Ensuring microtasks are run, if the construct is initiated with an empty stack (such as during the parser). Tests: web-platform-tests/wpt#5208
There are two cases will call custom element constructor:
Both of them seems not handle settings object. Do they need to handle settings object? Or it is already handled in some place.
@domenic @annevk
The text was updated successfully, but these errors were encountered: