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

fix playPause bugs by directly playPause video element #259

Merged
merged 5 commits into from
May 7, 2021
Merged

fix playPause bugs by directly playPause video element #259

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b72599
directly playPause video element
Araxeus May 6, 2021
a98b894
lint
Araxeus May 6, 2021
d274e80
minify
Araxeus May 6, 2021
88e738c
check if `yns_pause` exists
Araxeus May 7, 2021
e00be8f
lint
Araxeus May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions preload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const { remote } = require("electron");
const config = require("./config");
const { fileExists } = require("./plugins/utils");
const setupFrontLogger = require("./providers/front-logger");
const setupSongControl = require("./providers/song-controls-front");
const setupSongInfo = require("./providers/song-info-front");

const plugins = config.plugins.getEnabled();

Expand Down Expand Up @@ -37,8 +39,10 @@ document.addEventListener("DOMContentLoaded", () => {
});

// inject song-info provider
const songInfoProviderPath = path.join(__dirname, "providers", "song-info-front.js")
fileExists(songInfoProviderPath, require(songInfoProviderPath));
setupSongInfo();

// inject song-control provider
setupSongControl();

// inject front logger
setupFrontLogger();
Expand Down
18 changes: 18 additions & 0 deletions providers/song-controls-front.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { ipcRenderer } = require("electron");

let videoStream;
module.exports = () => {
videoStream = document.querySelector(".video-stream");

ipcRenderer.on("playPause", () => {
if (!videoStream) {
videoStream = document.querySelector(".video-stream");
}

if (videoStream.paused) {
videoStream.play();
} else {
videoStream.yns_pause();
Copy link
Owner

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

Suggested change
videoStream.yns_pause();
videoStream.pause();

?

Copy link
Collaborator Author

@Araxeus Araxeus May 6, 2021 •
edited
Loading

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 :(

Copy link
Owner

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!

Copy link
Collaborator Author

@Araxeus Araxeus May 7, 2021 •
edited
Loading

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:

  1. auto-confirm-when-paused was enabled for me all along while testing
  2. pause() works only when YoutubeNonStop isn't active (because YoutubeNonStop overrides the original function)
    yns_pause() is the original pause() function that YoutubeNonStop saved and registered

source: YoutubeNonStop.overrideVideoPause()

With the latest changes, it should all work flawlessly for you right?

}
});
};
2 changes: 1 addition & 1 deletion providers/song-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module.exports = (win) => {
// Playback
previous: () => pressKey(win, "k"),
next: () => pressKey(win, "j"),
playPause: () => pressKey(win, "space"),
playPause: () => win.webContents.send("playPause"),
like: () => pressKey(win, "_"),
dislike: () => pressKey(win, "+"),
go10sBack: () => pressKey(win, "h"),
Expand Down
9 changes: 4 additions & 5 deletions providers/song-info-front.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ ipcRenderer.on("update-song-info", async (_, extractedSongInfo) => {
});

const injectListener = () => {
var oldXHR = window.XMLHttpRequest;
const oldXHR = window.XMLHttpRequest;
function newXHR() {
var realXHR = new oldXHR();
const realXHR = new oldXHR();
realXHR.addEventListener(
"readystatechange",
() => {
if (realXHR.readyState == 4 && realXHR.status == 200) {
if (realXHR.responseURL.includes("/player")) {
if (realXHR.readyState === 4 && realXHR.status === 200
&& realXHR.responseURL.includes("/player")) {
// if the request contains the song info, send the response to ipcMain
ipcRenderer.send("song-info-request", realXHR.responseText);
}
Araxeus marked this conversation as resolved.
Show resolved Hide resolved
}
},
false
Expand Down