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

Content from iframes is still not fully supported #133

Open
mflorea opened this issue Nov 22, 2023 · 5 comments
Open

Content from iframes is still not fully supported #133

mflorea opened this issue Nov 22, 2023 · 5 comments

Comments

@mflorea
Copy link

mflorea commented Nov 22, 2023

It seems the fix from #126 is not enough to properly support iframes. The root problem is that the realm of the JavaScript interface (e.g. Text) of a DOM node is not always synchronized with the node's owner document. For instance, for a given DOM text node the following is not always true:

textNode instanceof textNode.ownerDocument.defaultView.Text

One such case when this is not true is when a DOM node is adopted. Adopting a DOM node changes its owner document, but it seems it doesn't change its prototype in all browsers. See https://jsfiddle.net/f7pbmue6/2/

  • Firefox (119) updates the prototype
  • Chrome (119) doesn't update the prototype

Judging by https://bugzilla.mozilla.org/show_bug.cgi?id=1470017 my feeling is that Chrome's behavior is the standard one which means the prototype and the owner document doesn't have to be in sync. If that is the case then using instanceof for checking the type of a DOM node becomes very fragile, since the node's prototype can be from any realm (you can imagine having multiple iframes and adopting nodes between them). We would need a way to find the (original) realm in which the DOM node was created, but I'm not sure this is possible.

@johanneswilm
Copy link
Member

johanneswilm commented Nov 22, 2023

@mflorea How about checking for element.__proto__.constructor.name ? Will that always work? I am just assuming that not checking for attributes or comparing string values is something done for performance reasons.

@mflorea
Copy link
Author

mflorea commented Nov 22, 2023

Using something like element.__proto__.constructor.name feels very hackish to me. Can you remind me what is the issue with checking node.nodeType? Its value is a number and there are predefined constants for it, like Node.ELEMENT_NODE, which should make:

node.nodeType === Node.TEXT_NODE

super fast. I don't understand what's wrong with this.

@johanneswilm
Copy link
Member

@mflorea The issue is that that will only let us distinguish between text nodes, comment nodes and regular elements. We even need to find things like HTMLTextAreaElement. Maybe, after all, the only option is to check for attributes if we are to support all the possible ways of using iframes. That also feels hackish.

@johanneswilm
Copy link
Member

This seems relevant: https://groups.google.com/g/mozilla.dev.platform/c/bfwNKKiAxcw

This is more than a decade old. They seem to suggest comparing things from the prototype. Some of the other methods, like Components.utils.getGlobalForObject(el) seem to have been removed from Chrome since.

@johanneswilm
Copy link
Member

If we don't want to access the prototype through __proto__, another way is Object.getPrototypeOf(el).constructor.name. To check whether an element is an Element at all, the answer seems to be something like typeof el === 'object' && el.constructor !== Object && !Object.prototype.hasOwnProperty.call(el, 'nodeType') && el.nodeType !== undefined

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

No branches or pull requests

2 participants