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

ReactEventListener.dispatchEvent should ignore non own documents. #3605

Conversation

azproduction
Copy link

We are writing firefox sidebar plugin with React and faced some issues with dead objects.

Due to overprivileged environment of plugin event can bubble/capture out of iframe. And if surrounded application has React instance it captures event.target of element of non own document. After reloading the iframe and trying to interact with iframe content it throws an exception about Firefox dead object(garbage collected DOM element).

+sidebar------+
|React1       |
|             |
|  +iframe-+  |
|  |React2 |  |
|  |       |  |
|  +-------+  |
|             |
+-------------+

Also it could lead to an issue with the same reactId: Error: Invariant Violation: ReactMount: Two valid but unequal nodes with the same data-reactid: .0.

Please find the example Firefox plugin along with the souce code: https://dl.dropboxusercontent.com/u/37680916/react-bug.zip

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@zpao
Copy link
Member

zpao commented Apr 22, 2015

cc @syranide. Seems pretty reasonable (and vaguely familiar to something you might have done in the past).

@syranide
Copy link
Contributor

@zpao React officially supports rendering into iframes so separating by document seems like a no-go (and this would exclude shadow documents too I think?). Ultimately I think the solution is to drop data-reactid but it may take a while (but may be possible soonish). For now I've recommended people to use #2713 (comment). It could probably be built into the data-reactid itself (or at least publicly exposed for now), but at the cost of more overhead, so I'm not sure how keen you guys are on that.

@azproduction
Copy link
Author

@syranide thanks, I'll try to generate unique react-id property for every instance.

@sebmarkbage sebmarkbage closed this Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants