dom icon indicating copy to clipboard operation
dom copied to clipboard

shadowRoot.innerHTML parsing elements from another realm/iframe

Open leobalter opened this issue 4 years ago • 9 comments

Based on https://bugs.chromium.org/p/chromium/issues/detail?id=1199886#c7, I have a cross browser inconsistency in a still-clouded spec part.

This happens when shadowRoot.innerHTML renders native elements from the one document while rendering custom elements from the current document. This behavior differs between Chrome and other browsers. Matching consistency is key and this issue might require updating the spec to indicate which direction to follow.

This is the code to reproduce:

<body>
    <iframe hidden></iframe>
    <script>
        const iframe = document.querySelector('iframe');
        class Bar extends (iframe.contentWindow.HTMLElement) {
            constructor() {
                super();
                this.attachShadow({ mode: 'open' });
            }
        }
        iframe.contentWindow.customElements.define('x-bar', Bar);
        iframe.contentWindow.customElements.define('x-baz', class Baz extends (iframe.contentWindow.HTMLElement) {
            constructor() {
                super();
                this.innerHTML = '<p>x-baz from iframe</p>';
            }
        });
        class Baz extends HTMLElement {
            constructor() {
                super();
                this.innerHTML = '<p>x-baz from doc</p>';
            }
        }
        customElements.define('x-baz', Baz);
        const elmFromAnotherDoc = new Bar();
        document.body.appendChild(elmFromAnotherDoc);
        elmFromAnotherDoc.shadowRoot.innerHTML = '<p>what paragraph is this?</p><x-baz></x-baz>';
        console.log('Is instance of <p> from main window?', elmFromAnotherDoc.shadowRoot.firstChild instanceof HTMLParagraphElement);
        console.log('Is instance of <x-baz> from main window\'s registry?', elmFromAnotherDoc.shadowRoot.lastChild instanceof Baz);
    </script>
</body>

In the devtools console:

# Chrome
Is instance of <p> from main window? false
Is instance of <x-baz> from main window's registry? true

# Firefox, Safari
Is instance of <p> from main window? true
Is instance of <x-baz> from main window's registry? true

Our preference, and expectation, is for the browsers to follow the Firefox, Safari behavior, which seems appropriate for the case.


@domenic's comment in the linked thread is relevant here:

This is a very interesting convoluted case.

Note that `elmFromAnotherDoc` has:

- A relevant realm of outer frame (it's an instance of a class declared in the outer frame)
- A node document of the outer frame (it's appended to document.body)
- __proto__.__proto__ equal to HTMLElement from the inner frame

In particular this means that the setter for innerHTML, which is derived from __proto__.__proto__.__proto__ (ShadowRoot from the inner frame) runs in the inner frame realm. I.e., the current realm at the time that setter runs is the inner frame, while the relevant realm of this (= the relevant realm of elmFromAnotherDoc) is the outer frame.

Here's what the spec says. https://dom.spec.whatwg.org/#concept-create-element is the relevant algorithm:

- Step 6.1.2 constructs from the constructor for the custom element case. The constructor is looked up from the registry, which is derived from the document passed to that algorithm, which is ultimately derived from the node document (outer frame) in https://html.spec.whatwg.org/#create-an-element-for-the-token step 1.

- Step 7.1 gets the element interface. It isn't clear what realm it uses.

So per spec, the answer should be:

- Is instance of <p> from main window? unknown
- Is instance of <x-baz> from main window's registry? false

I imagine the next step here will be spec discussion to answer: (1) what do different browsers do in these different cases? (2) should these be consistent, or no? (3) what should the answer be?

leobalter avatar May 04 '21 00:05 leobalter

I don't think there is any ambiguity here. We need to start from the innerHTML's setter, which creates a new Document to parse the specified HTML in the HTML fragment parsing algorithm. Because it is a new document, the step to create a new element in it will not result in a synchronous custom element construction, nor does it get immediately upgraded to a custom element upon return from the fragment parsing algorithm.

innerHTML then replaces all with the newly created fragment within the context object, the ShadowRoot. Replacing all with the fragment will insert the fragment, which will try to update every candidate element in the inserted subtree.

The first thing that happens with trying to update an element is looking up a custom element definition given the element's node document. But by this point, we had already adopted this node into ShadowRoot's node document in step 7.1. of inserting the fragment. The node document of ShadowRoot in turn is the main document at this point since document.body.appendChild(elmFromAnotherDoc) had adopted to it.

Because we're looking up a custom element definition with the element's node document, which is the main document, we would find and only find custom elements defined in the main document.

rniwa avatar May 04 '21 01:05 rniwa

I think one issue here though is that as per https://www.w3.org/Bugs/Public/show_bug.cgi?id=20567#c74 the prototype should not change, but it seems that it does in Firefox and Safari? In particular thinking about the p element here.

Looking at a reduced test case only Firefox is different: https://software.hixie.ch/utilities/js/live-dom-viewer/saved/9241.

cc @EdgarChen @smaug----

annevk avatar May 04 '21 06:05 annevk

Thanks for your detailed and linked analysis, @rniwa. Indeed I made a mistake in mine and yours seems correct, at least for the <x-baz> case: it should be from the main window's registry.

I think the <p> case is still not clear though, since step 7.1 of create an element is not specific about which window to look up the element interface in. (I guess the HTMLElement and HTMLUnknownElement in step 6 has a similar problem, for the intermediate period between the insertion and the upgrade...). I'd say there are at least two plausible choices:

  • Use the element interface from the global associated to the document passed to "create an element"
  • Use the element interface from the current global object (i.e. the global object of the innerHTML setter)

In this case they are equivalent and equal to the other window (so <p> should not be from the main window, I think, so Chrome's behavior is correct?). But they could be different if you did something like Object.getOwnPropertyDescriptor(ShadowRoot, "innerHTML").set.call(elmFromAnotherDoc.shadowRoot, ...).

I'd appreciate a double-check on my logic as to whether <p> should be from the main window or not though, as this is a tricky area and I've already gotten things wrong once...

(All this just makes me wish globals weren't allowed to synchronously touch each other, ugh.)

domenic avatar May 04 '21 15:05 domenic

I think the <p> case is still not clear though, since step 7.1 of create an element is not specific about which window to look up the element interface in. (I guess the HTMLElement and HTMLUnknownElement in step 6 has a similar problem, for the intermediate period between the insertion and the upgrade...). I'd say there are at least two plausible choices:

Yeah, none of those specify in which realm we'd be looking up the interfaces, which is ambiguous given what you've discovered.

  • Use the element interface from the global associated to the document passed to "create an element"
  • Use the element interface from the current global object (i.e. the global object of the innerHTML setter)

In this case they are equivalent and equal to the other window (so <p> should not be from the main window, I think, so Chrome's behavior is correct?).

I'm not following. Both of those options seems to result in creating HTMLParagraphElement in the realm of the main window since the shadow host is in the main document at that point? The only reasonable option I can think of creating HTMLParagraphElement of the iframe's realm will be if we did the interface lookup via the global object of this in innerHTML's setter.

Arguably this behavior is a bit strange / unlike other JS objects since the following code will return false but I think it's a necessary oddity that arises from the fact Node has the ability to get adopted to another document and change its associated document:

iframeRange = document.body.appendChild(document.createElement('iframe')).contentDocument.createRange();
iframeRange.selectNode(document.body);
iframeRange.cloneRange().__proto__ == document.createRange().__proto__

rniwa avatar May 11 '21 04:05 rniwa

From what I remember when we discussed this during the F2F meeting, @mfreed7 mentioned that one possible explanation for the current behavior in chrome could be attributed to the customization of built-ins, in this case, the customization of the <p> element. He initial reaction (from what I can remember) was that this could be fixed to have a consistent behavior between the <p> and the custom element, which seems to be behaving correctly in all browsers, meaning the <p> should be created from the main window.

caridy avatar May 11 '21 04:05 caridy

From what I remember when we discussed this during the F2F meeting, @mfreed7 mentioned that one possible explanation for the current behavior in chrome could be attributed to the customization of built-ins, in this case, the customization of the <p> element. He initial reaction (from what I can remember) was that this could be fixed to have a consistent behavior between the <p> and the custom element, which seems to be behaving correctly in all browsers, meaning the <p> should be created from the main window.

I don't understand that. There are many ways to interpret or even misread the spec but creating customized built-ins and autonomous custom element from two different realms during a fragment parsing algorithm ain't one of them.

rniwa avatar May 11 '21 07:05 rniwa

I'm not following. Both of those options seems to result in creating HTMLParagraphElement in the realm of the main window since the shadow host is in the main document at that point? The only reasonable option I can think of creating HTMLParagraphElement of the iframe's realm will be if we did the interface lookup via the global object of this in innerHTML's setter.

I think you are right for "Use the element interface from the global associated to the document passed to "create an element"". The fragment parsing algorithm ends up being based on elmFromAnotherDoc's node document, which is the main document because of adoption.

For "Use the element interface from the current global object (i.e. the global object of the innerHTML setter)", I think the subframe is correct, because the innerHTML setter is iframe.contentWindow.ShadowRoot's innerHTML setter. This is because elmFromAnotherDoc.__proto__ === Bar, elmFromAnotherDoc.__proto__.__proto__ == iframe.contentWindow.HTMLElement.prototype, so elmFromAnotherDoc.shadowRoot will return an instance of iframe.contentWindow.ShadowRoot. I am pretty sure? (But, I keep being wrong in this thread...)

Anyway, laying out all the possible behaviors isn't too important. The question is what the behavior should be. Probably we should make <p> match <x-bar>? I think the conclusion of all this is that the simplest way to do that would be to specify the global to be the global of the document passed to "create an element". Then they would both use the main document.

domenic avatar May 11 '21 17:05 domenic

For "Use the element interface from the current global object (i.e. the global object of the innerHTML setter)", I think the subframe is correct

Sorry, where does prose appear in the spec?

I think the conclusion of all this is that the simplest way to do that would be to specify the global to be the global of the document passed to "create an element". Then they would both use the main document.

Yeah, I concur.

rniwa avatar May 11 '21 18:05 rniwa

For "Use the element interface from the current global object (i.e. the global object of the innerHTML setter)", I think the subframe is correct Sorry, where does prose appear in the spec?

I was referring to my bulleted list of plausible choices in https://github.com/whatwg/dom/issues/977#issuecomment-832028946. But, it sounds like we concur on going with the other choice, so it doesn't matter much.

domenic avatar May 11 '21 19:05 domenic