targetWithinDownshift always return false with different execution environment

[KaiminHuang]

Hi there,

I recently found an issue with downshift while the component is rendered in a different execution environment using React Portal.

What you did:
Rendering a downshift component which is declared in an iframe module into the parent window DOM using Portal.

Problem description:
The existing downshift code always check whether the target element is an instance of the original environment Node (in my case, iframeWindow.Node).

Different scopes have different execution environments. This means that they have different built-ins (different global object, different constructors, etc.). This may result in unexpected results. For instance, [] instanceof window.frames[0].Array will return false, because Array.prototype !== window.frames[0].Array.prototype and arrays inherit from the former.

quote from MDN instanceof and multiple context (e.g. frames or windows)

What happened:
It resets downshift state on mouse up.

Relevant code or config

child instanceof Node always return false because the target element is rendered in a different environment.

function isOrContainsNode(parent, child) {
  return (
    parent === child ||
    (child instanceof Node && parent.contains && parent.contains(child))
  )
}

Suggested solution:
🤔 I can think of two possible solutions here:

1. use the environment prop. And replace child instanceof Node with child instanceof environment.document.Node
2. Or follow this example from MDN, and change it to child instanceof child.ownerDocument.defaultView.Node

[silviuaavram]

Thanks for posting this. Will also take a look to understand what's going on.

[silviuaavram]

Ok, got it @KaiminHuang . I did not get the solution #1 to work, maybe it's environment.Node instead of environment.document.Node?

Can you create a PR with the fix? We should also write a unit test for this, somehow.

[silviuaavram]

Can you check with 6.1.2-alpha.0?

[KaiminHuang]

Ok, got it @KaiminHuang . I did not get the solution #1 to work, maybe it's environment.Node instead of environment.document.Node?

Can you create a PR with the fix? We should also write a unit test for this, somehow.

Yes, my mistake. It should just be environment.Node.

[KaiminHuang]

@silviuaavram Thanks for you help friend. I think there might be a mistake in v6.1.2-alpha.0.
The Node variable exist on window or environment rather than document.
But in onMouseUp, seems like you are trying to pass down environment.document. Instead we should pass down environment. It might be easier to explain if you can share me your branch link.

[silviuaavram]

I think I deleted it by accident. Can you create a PR with the fix and we can take it from there?

[KaiminHuang]

Hi @silviuaavram, could I get your review on this PR?

[silviuaavram]

Fixed by #1264

[silviuaavram]

@all-contributors please add @KaiminHuang for code, bug

[allcontributors]

@silviuaavram

I've put up a pull request to add @KaiminHuang! 🎉

[rrabii]

@silviuaavram @KaiminHuang the issue is still there in iOS browsers when using iframe, works fine on other platforms.