-
Notifications
You must be signed in to change notification settings - Fork 565
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
fix playPause bugs by directly playPause video element #259
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto-confirm-when-paused
unpauses the video each time because it is paused programmatically (instead of emulating the space key), is it expected?
providers/song-controls-front.js
Outdated
if (videoStream.paused) { | ||
videoStream.play(); | ||
} else { | ||
videoStream.yns_pause(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uncaught TypeError: videoStream.yns_pause is not a function
shouldn't it be
videoStream.yns_pause(); | |
videoStream.pause(); |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats weird... where did you get that error?
pause()
doesn't work on the youtube video stream.
yns_pause()
is a function that I extracted from the actual youtube video stream and it works in all my tests
I didn't test it with auto-confirm-when-paused
, we'll have to see what to do about that
maybe use this new ipc("playPause") only if auto-confirm-when-paused
isn't enabled
and else use spacebar as before. but then it wont fix this issue for users that have that plugin enabled :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it depends on the platform 🤔 -> A workaround can be to check if yns_pause
is an existing function, in which case it can be called, otherwise call pause
.
Agreed about using it only if auto-confirm-when-paused
isn't enabled, it's not perfect but this way it won't break stuff!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized 2 things:
auto-confirm-when-paused
was enabled for me all along while testingpause()
works only when YoutubeNonStop isn't active (because YoutubeNonStop overrides the original function)
yns_pause()
is the originalpause()
function that YoutubeNonStop saved and registered
source: YoutubeNonStop.overrideVideoPause()
With the latest changes, it should all work flawlessly for you right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok after some local testing ✅
Thanks for the improvement, merging!
this PR makes song-controls.playPause() affect directly the video element in the page. (instead of just pressing space)
This fixes several bugs when pausing the video when it isn't focused
(from tray/taskbar/notifications)
fix #145 , fix #129
also sneaky change because of previous comments:
song-info-front is directly required instead of checking if file exist (since we already know it exist)