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

execution exception for empty video src #772

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

Can-Chen
Copy link
Contributor

when using vidstack/player to play videos, if the area where the video is rendered is not scrolled, the video src will be empty at this time. After calling reactToPrint, the browser's printing function will not be invoked and no errors will be thrown.

@MatthewHerbst
Copy link
Owner

Hi @Can-Chen, apologies for the delayed response here. I just tried testing this out with the local example and was not able to reproduce it. Are you able to share a Codesandbox or similar where I can see this issue?

@Can-Chen
Copy link
Contributor Author

Can-Chen commented Dec 23, 2024

react-test.zip

problems will occur when the src of the video element is empty.

@MatthewHerbst
Copy link
Owner

I'm not able to download files right now on the computer I have access to; could you please either use Codesandbox or post the code in the comment? Thank you

@Can-Chen
Copy link
Contributor Author

@MatthewHerbst
Copy link
Owner

MatthewHerbst commented Dec 23, 2024

Thanks! When I go to that example I do not see a problem. I am using Chrome 131. What browser are you using?

Screen.Recording.2024-12-23.at.9.32.15.AM.mov

@Can-Chen
Copy link
Contributor Author

after the page is rendered, the video element will trigger when it has never appeared in the viewport.

2024-12-24.mp4

@MatthewHerbst
Copy link
Owner

Ah, I see now, thank you!

@MatthewHerbst
Copy link
Owner

I wonder if there is a way to make it work instead of having an error; I will look for a solution. If I cannot find one I will merge this PR

@Can-Chen
Copy link
Contributor Author

ok

@MatthewHerbst
Copy link
Owner

Going to merge this while I look for a fix, I think this is good protection to have either way. Thank you!!

@MatthewHerbst MatthewHerbst merged commit 1541738 into MatthewHerbst:master Dec 25, 2024
@MatthewHerbst
Copy link
Owner

I do wonder if this is an issue with MediaPlayer. I am not able to replicate this issue with the video in the react-to-print examples: https://codesandbox.io/p/sandbox/rzdhd

@MatthewHerbst
Copy link
Owner

Yeah, I believe the root issue is with the MediaPlayer. I filed an issue here: vidstack/player#1548
This check is still good to have though to be safe against bad video node constructions, so thank you, I will release a new version with this fix shortly.

@Can-Chen
Copy link
Contributor Author

Yeah, I believe the root issue is with the MediaPlayer. I filed an issue here: vidstack/player#1548 This check is still good to have though to be safe against bad video node constructions, so thank you, I will release a new version with this fix shortly.

The properties "load" and "posterLoad" of the MediaPlayer need to be set to "eager".

https://vidstack.io/docs/player/core-concepts/loading/?styling=default-theme#load-strategies

@MatthewHerbst
Copy link
Owner

Yeah, re-reading the spec, src is definitely optional in many scenarios. No need to try and "fix" it, this check you've added is enough 🥳

@MatthewHerbst
Copy link
Owner

Published the fix in version 3.0.4. Thanks so much for the help @Can-Chen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants