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

Previewed media continues playback when preview is collapsed #644

Open
e-five256 opened this issue Mar 29, 2024 · 3 comments
Open

Previewed media continues playback when preview is collapsed #644

e-five256 opened this issue Mar 29, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@e-five256
Copy link
Member

Describe the bug

Described here

When an entry preview includes a video or other media playback, clicking play and collapsing the preview will not pause the media. Expectations appear to be that media playback is paused when hidden.

This functionality was changed in #296 to no longer re-download every time the preview is expanded / collapsed, which is likely what caused this to change, as before the entire preview was removed from the DOM and every expand was a new AJAX request to refetch it.

On which Mbin instance did you find the bug?
All

Which Mbin version was running on the instance?
1.5.0

To Reproduce
Steps to reproduce the behavior:

  1. Open a thread with a video preview, such as this one
  2. Expand the embedded preview
  3. Click play on the video
  4. Collapse the embedded preview
  5. Observe audio is still playing
  6. Expand the embedded preview
  7. Observe the video never paused

Expected behavior

The media to pause playback

Additional context

PR that changed functionality of embedded previews

@e-five256 e-five256 added the bug Something isn't working label Mar 29, 2024
@e-five256 e-five256 changed the title Previewed media continues playback when minimized Previewed media continues playback when preview is collapsed Mar 29, 2024
@e-five256
Copy link
Member Author

e-five256 commented Mar 29, 2024

I did some cursory investigations into this. There is a JS method .pause(), but either I got the selection wrong or it might not work. There is also a youtube api for iframes to stop videos, but that is very specific to youtube when I imagine we would prefer another solution.


There was the suggestion to just rollback this change and always refetch the content, or at least destroy videos. From my perspective:

  1. The original change was done with reason, which was requested by server owners to not hit their server every time someone expanded and collapsed the inline preview
  2. Obviously images would ideally be cached by the browser, but videos seem a bit different, I imagine it downloads the video each time. Might be ok for people with good internet. I get an amazing 500MB of data per month on my phone plan. I think it's important to try to save data where we can, and throwing out and redownloading the exact same content seems a bit hasty

Open to any other thoughts / suggestions. @asdfzdfj especially yours as you helped me out with the original PR, not sure if you have more thoughts now or ideas on how to handle this request. (And maybe the answer is just .pause() and I was just doing it wrong. I just got the impression from browsing stackoverflow that there wasn't a super supported method of doing this, but could've just not found the right answers)

@asdfzdfj
Copy link
Contributor

asdfzdfj commented Apr 4, 2024

being able to "suspend" dom nodes, stash them when hiding and restore them back with the same state when unhide would be really dope and I'll do it if I could, but alas I don't know enough html/js to do that (if anyone else knows, feel free to chime in or give some pointers)

the best I could think of right now is kinda hasty compromise: rollback the change, but also make the ajax embed fetch remembers the html response on successful fetch (refactor that out into a function or something), and so when it's about to fetch the embed again it could instead immediately return this stashed html to use as normal without hitting the endpoint again

Copy link
Contributor

This issue is stale because it has been open 50 days with no activity. Remove stale label or comment or this will be closed in 6 days.

@github-actions github-actions bot added the Stale Inactivity for too long label May 25, 2024
@BentiGorlich BentiGorlich removed the Stale Inactivity for too long label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants