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.

Sign up for GitHub

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

Jump to bottom

fix: empty src attr in StrictMode #1538

Merged
merged 2 commits into from
Feb 3, 2023
Merged

fix: empty src attr in StrictMode #1538

merged 2 commits into from
Feb 3, 2023

Conversation

Gerpea
Copy link
Contributor

@Gerpea Gerpea commented Nov 11, 2022

This was referenced Nov 16, 2022
Closed
Closed
@tmilewski
Copy link

@cookpete This is blocking us as well, and we can confirm that @Gerpea's fix works. A merge and publish would be amazing. Thanks!

@willenglishiv
Copy link

same, experiencing the same issue here, fix would be awesome!

@masterdimar
Copy link

same problem here. the fix would be welcome

@Nicopa
Copy link

Nicopa commented Dec 27, 2022

I got the same problem and the fix works.

@Gerpea
Copy link
Contributor Author

Gerpea commented Jan 11, 2023

@cookpete Hi, we would appreciate it if you could review the fix

@Emmaccen
Copy link

Yup! This is the "Fix" 🚀

@AryanJ-NYC
Copy link

@cookpete ?

@AntonOfTheWoods
Copy link

Has anyone seen @cookpete on a ticket or git in the last few months? Maybe he has abandoned the repo (or is no longer able to manage it) and it's time to fork?

@cookpete
Copy link
Owner

cookpete commented Feb 2, 2023

I’m here. I’m alive.

It’s been over seven years since I started this project. I never thought it would become this popular. Life changes a lot in seven years. I have much less free time now. I would love to spend more time on this. I will try to in the future. Bear with me. Apologies to keep everyone waiting for so long.

@tmilewski
Copy link

@cookpete No worries, thanks a lot for your continued support!

@cookpete
Copy link
Owner

cookpete commented Feb 3, 2023

Thanks to @Gerpea for the original fix but it’s actually as simple as setting the src in the componentDidMount of FilePlayer.js

The original fix would have called load twice for every player, including calling getSDK twice, which loads all the third party scripts. It seemed quite bug prone so I simplified it a bit.

Regardless, thanks so much for the PR and pointing me in the right direction to fix this.

@cookpete cookpete merged commit 5c7ffa2 into cookpete:master Feb 3, 2023
@Gerpea
Copy link
Contributor Author

Gerpea commented Feb 3, 2023

🎉 Thank you @cookpete

This was referenced Feb 3, 2023
Closed
Closed
Closed
@Awais914
Copy link

Awais914 commented Feb 6, 2023

@cookpete Thank you for the fix. In which version will this fix come?

@cookpete
Copy link
Owner

cookpete commented Feb 9, 2023

Published in v2.11.1

@niktverd niktverd mentioned this pull request Mar 29, 2023
Closed
anampartho added a commit to anampartho/react-player that referenced this pull request Apr 13, 2023
@anampartho
Fix cookpete#1604 - FilePlayer does not work if I passed an array of …
93a8897 
…urls

PR cookpete#1538 was setting the src of the video element to undefined. Because
of this, even if the users passed an array of urls, the video element
would not play becuase it got a `src` of `undefined`;

After fixing the undefined `src` issue, another PR was making a infinite
loop issue when user passed an array of urls. There, the author set the
`srcObject` to null. There as well, a check for `url` prop was missing.

This PR aims to fix both of these overlooked aspects.
@anampartho anampartho mentioned this pull request Apr 13, 2023
Merged
cookpete pushed a commit to anampartho/react-player that referenced this pull request Aug 31, 2023
@anampartho @cookpete
Fix cookpete#1604 - FilePlayer does not work if I passed an array of …
0f145ed 
…urls

PR cookpete#1538 was setting the src of the video element to undefined. Because
of this, even if the users passed an array of urls, the video element
would not play becuase it got a `src` of `undefined`;

After fixing the undefined `src` issue, another PR was making a infinite
loop issue when user passed an array of urls. There, the author set the
`srcObject` to null. There as well, a check for `url` prop was missing.

This PR aims to fix both of these overlooked aspects.
cookpete pushed a commit that referenced this pull request Aug 31, 2023
@anampartho
Fix #1604 - FilePlayer does not work if I passed an array of urls (#1612
64e5cd3 
)

PR #1538 was setting the src of the video element to undefined. Because
of this, even if the users passed an array of urls, the video element
would not play becuase it got a `src` of `undefined`;

After fixing the undefined `src` issue, another PR was making a infinite
loop issue when user passed an array of urls. There, the author set the
`srcObject` to null. There as well, a check for `url` prop was missing.

This PR aims to fix both of these overlooked aspects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

v2.11.0 not playing MP4 URL files
10 participants
@Gerpea @tmilewski @willenglishiv @masterdimar @Nicopa @Emmaccen @AryanJ-NYC @AntonOfTheWoods @cookpete @Awais914