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

It throw error if target element inside a iframe on Chrome #97

Closed
minhtranite opened this issue Feb 19, 2020 · 11 comments · Fixed by #98
Closed

It throw error if target element inside a iframe on Chrome #97

minhtranite opened this issue Feb 19, 2020 · 11 comments · Fixed by #98

Comments

@minhtranite
Copy link

Describe the bug
If we try observe an element inside iframe it will throw an error:

Failed to execute 'observe' on 'ResizeObserver': parameter 1 is not of type 'Element"

It because when you check target instanceof Element it return false. The reason are explained here: https://stackoverflow.com/questions/52222237/instanceof-fails-in-iframe

We can try other way to check target is an element like: https://stackoverflow.com/questions/384286/how-do-you-check-if-a-javascript-object-is-a-dom-object

Or use lodash.iselement: https://www.npmjs.com/package/lodash.iselement

To Reproduce
Please try this demo on Chrome: https://codesandbox.io/s/pensive-cache-31co0

Expected behavior
It work with element inside iframe.

Frameworks/Libraries used
React

Desktop (please complete the following information):

  • OS: Any
  • Browser Chrome
@TremayneChrist
Copy link
Member

Hey @minhtranite thanks for the info. Looks easy enough to sort. Will look into it!

@TremayneChrist
Copy link
Member

This has another issue where the document has no listeners for observation, so the observation is only noticed when the scheduler is run in the main document. This could also be fixed with changes from #42

@TremayneChrist
Copy link
Member

@minhtranite as a workaround for now, you can propagate the events to the parent window. It's pretty nasty, but will get it working 😬

Here is an example based on your sample

// boxEl exists in an IFrame
this.boxEl.addEventListener('keydown', (e) => {
  const event = new Event(e.type, e); // proxy event
  event.preventDefault(); // try not to do anything unexpected
  window.dispatchEvent(event); // dispatch in main window
});

@JayaKrishnaNamburu
Copy link
Contributor

Hi @TremayneChrist thanks for the great package 🎉,
I am using this for sometime. But, when i upgraded from 2.3.0 to 3.3.0 the isElement check breaks for me. The main difference i can find from the above codesandbox example in description to my use-case is

Example above uses a ReactPortal to render the data, where in this case the portals use iframe just as a kind of placeholder to render stuff. But managed by React. But when we create a DOM element and try to add to the iframe. That's when the check breaks.

Here is the same example with two different checks with different versions
@juggle/[email protected] -> https://codesandbox.io/s/observer-330-ucif1
@juggle/[email protected] -> https://codesandbox.io/s/observer-230-vl0gi

@TremayneChrist
Copy link
Member

Hey @JayaKrishnaNamburu yep there is a bug here. So many ways to create and take ownership of elements :)

@JayaKrishnaNamburu
Copy link
Contributor

Yes agreed, I will try to take a look if i can fix this check for both the use-cases and general use-case in common 😄

@TremayneChrist
Copy link
Member

Great! I'm not sure if you've looked at the code yet but it's probably best to just test if el instanceof Element first and return. If not, continue to try and find the document/window scope. This is the current issue for your use case, as the util cannot find out what the defaultView is.

@TremayneChrist
Copy link
Member

Just seen your PR :)

@JayaKrishnaNamburu
Copy link
Contributor

Great! I'm not sure if you've looked at the code yet but it's probably best to just test if el instanceof Element first and return. If not, continue to try and find the document/window scope. This is the current issue for your use case, as the util cannot find out what the defaultView is.

Yes yes, do you think the PR is a valid fix ? or can i close it :)

@minhtranite
Copy link
Author

Hi @JayaKrishnaNamburu your problem on the demo above easy to fix by use iframe document to create element instead of top windown document.

The demo for the fix: https://codesandbox.io/s/observer-330-forked-39xy9?file=/src/App.tsx

@JayaKrishnaNamburu
Copy link
Contributor

Yes, and this behaviour is only in chrome alone 😄

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

Successfully merging a pull request may close this issue.

3 participants