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

Ensure that the YouTube <iframe /> is really in the DOM before calling #180

Closed
wants to merge 3 commits into from

Conversation

iantanwx
Copy link

This just ensures that the iFrame is mounted in the DOM before calling this.player.stopVideo(), or else an uncaught error results. I encountered this in the course of embedding this component in a draft-js based editor with cut and paste operations, causing the iFrame to be removed from the DOM before componentWillUnmount was called.

@cookpete
Copy link
Owner

I'm not sure why this is necessary. What is the harm in still calling stopVideo even if the iframe is somehow removed from the DOM before componentWillUnmount is called?

@iantanwx
Copy link
Author

iantanwx commented Apr 24, 2017 via email

@iantanwx
Copy link
Author

iantanwx commented Apr 24, 2017 via email

@cookpete
Copy link
Owner

Ah yeah I see the error now. Checking the iframe is in the DOM can be simplified to

document.body.contains(this.player.getIframe())

However, a fatal error occurs on every player method without this check (not just stop()) so I think a bit of refactoring is needed to apply this check to every method call.

The real solution is: do not let a non-react action remove the player from the DOM 😉

@cookpete cookpete closed this in 16fd888 Apr 27, 2017
@iantanwx
Copy link
Author

iantanwx commented May 2, 2017

Thanks, the problem is that draft-js has to allow the native cut event to occur. Otherwise the system clipboard will not be populated with the correct data. This causes the iframe to be removed from the DOM before state updates, which is why this check was necessary. There is no good way to fix this without a long and protracted PR to the draft-js repo, being as it is used in production by Facebook. Thanks for the help, I noticed you committed another change that I believe fixes the issue.

david-hub024 pushed a commit to david-hub024/React_VideoPlayer that referenced this pull request Dec 23, 2018
david-hub024 pushed a commit to david-hub024/React_VideoPlayer that referenced this pull request May 23, 2020
albanqoku added a commit to albanqoku/react-player that referenced this pull request Feb 24, 2021
Webmaster1116 added a commit to Webmaster1116/video-player that referenced this pull request May 20, 2021
webmiraclepro added a commit to webmiraclepro/video-player that referenced this pull request Sep 9, 2022
philip-luther added a commit to philip-luther/react-player that referenced this pull request Nov 22, 2024
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 this pull request may close these issues.

2 participants