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

'Back to video player' doesnt work when not on video page #624

Closed
NotTheReal-Jamie opened this issue Oct 9, 2020 · 5 comments · Fixed by #4978
Closed

'Back to video player' doesnt work when not on video page #624

NotTheReal-Jamie opened this issue Oct 9, 2020 · 5 comments · Fixed by #4978
Labels
enhancement New feature or request player-related Player-related changes are currently on hold due to the ongoing player migration.

Comments

@NotTheReal-Jamie
Copy link

Pressing the 'back to video player' button in the pop-up video player doesn't do anything besides closing the PIP-player when you're not on the video's page in the Freetube app.

See here for an example of this in action.

@GilgusMaximus GilgusMaximus added the bug Something isn't working label Oct 9, 2020
@GilgusMaximus
Copy link
Contributor

Thanks for the reminder. This is definitely an issue that has to be sorted out. I'm going to add it to the target list for release 0.10.0

@PrestonN
Copy link
Member

PrestonN commented Oct 9, 2020

I think I might consider this working as intended.

There isn't any way to differentiate wanting to close the PiP player and wanting to continue the video in the main player. Since you've left the video screen, we'd have to re-grab that information so that you could place it in the main player again.

If you leave the main video page and come back to that same video, then the PiP will have the video continue where you left off once closed.

I'd consider this a feature and probably a low priority one for me at least. Either a toggle will need to be created in the settings to always load the video back up, or a toast message will need to be displayed that when clicked on will take you back to that video. Both methods will need logic added to allow setting the desired timestamp on a route change.

@PrestonN PrestonN added enhancement New feature or request and removed bug Something isn't working labels Oct 9, 2020
@NotTheReal-Jamie
Copy link
Author

NotTheReal-Jamie commented Oct 9, 2020

Since the button is labelled as "back to video player" I can't imagine that this it's working as intended like this. It going back to the video player [of the video which is playing] and continuing on where it was would also be in line with how PIP functions in Firefox and other applications.

@PrestonN
Copy link
Member

PrestonN commented Oct 9, 2020

As I mentioned, in the background there is nothing different between clicking on "Back to video player" and clicking on the exit button, so we can't use that information to determine the user's intentions when PiP is closed. What if the user didn't want to keep watching that video? What if the user had PiP paused, was watching a different video, and decided to close the PiP player? We don't want it to take over and navigate the user away from that other video in that situation.

The only way we can make this work is if we know 100% that the user's intention is that they want to continue watching the video. This will need to be done via a prompt or a setting toggle. If you come back to the video while it's still in PiP, then we can comfortably say that the video should skip to where it was in PiP, which is how it currently behaves.

We need new logic to determine the user's intention when we close a video, which is why it's a new feature and not a bug. In it's current iteration, since we cannot determine the user's intention, it is working as intended, though that doesn't mean it can't be changed.

@SebTota
Copy link
Contributor

SebTota commented May 8, 2021

Copied from #966 (comment)

I'm sorry for creating a PR before continuing this issue here.

I propose, and already created a PR for, a solution that will check if the video is currently paused or playing to determine if the PiP window should close or return to the regular video window.

This works because the underlying difference between the "Close" and "Back to video player" mode is that the "Close" button pauses the video before sending the "leavepictureinpicture" event. Therefore, we can just check if the video player is paused or not when the "leavepictureinpicture" event is triggered to determine if we should close the PiP window or close the PiP window and navigate back to the original video screen.

If the video is paused: Close PiP window
If the video is not paused: Close PiP window and re-navigate to the regular video window

The one down side to this is that if the video in the PiP window is paused and the user clicks the "Back to video player" button, the window close and not return to the original videos window. It will be closed as if the "Close" button is pressed, for the same reason as described above.

One other thing I added is to make sure the current video progress time of the PiP video is saved before closing the PiP window to make sure you resume in the regular video window where the PiP window left off. I think this is something that should be looked into further though because it seems that the progress of watching a video in PiP mode after navigating away from the video page is not tracked.

Let me know if this works or not and I can close the PR if this is not an implementation you want to add. Again, I apologize for not re-opening this issue before creating a PR.

To make sure all credit is given where it's due, the idea came from this stack-overflow post:
https://stackoverflow.com/questions/55991590/how-to-determine-type-of-leaving-picture-in-picture-mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request player-related Player-related changes are currently on hold due to the ongoing player migration.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants