WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 3 days ago

#48562 assigned defect (bug)

Audio keeps playing on closing media/attachment details popup in WP Admin

Reported by: amolv Owned by: antpb
Milestone: 5.7 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description

/wp-admin/upload.php

Click on audio file, it opens audio in Media Popup, play audio, if we click on next attachment/media audio stops playing but if we just close "Attachment Details" popup audio keeps playing in the background.

Attachments (2)

48562.1.patch (733 bytes) - added by Mista-Flo 5 months ago.
48562.2.patch (712 bytes) - added by Mista-Flo 3 days ago.
Improve patch, pause the player instead of refreshing everything

Download all attachments as: .zip

Change History (19)

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


15 months ago

#2 @antpb
15 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to antpb
  • Status changed from new to assigned

Sure enough, looks like this is happening for me too. :)

This ticket was brought up in the recent Media component meeting. Just to clarify reproduction steps, be sure to be in grid view of the Media Library when selecting the file. Then push play, close, see error.

@afercia mentioned in the meeting that this also existed in 5.2 so is not a regression from the recent 5.3 release.

Thanks for reporting this @amolv !

#3 @afercia
15 months ago

Looks like this happens for videos as well :)

#4 @Mista-Flo
5 months ago

I can reproduce as well, it's hard to know where is it configured in the code

@Mista-Flo
5 months ago

#5 @Mista-Flo
5 months ago

  • Keywords has-patch added; needs-patch removed

All right, so I finally ended up finding the code.

I have uploaded a patch that works, but I'm not sure about what I do exactly, so it might be a 'bad fix'.

I saw that when switching to the next attachment in the modal, it stops the audio/video. So I looked at the code, and I saw that it fires a refresh event. This refresh event call the rerender function which do some stuff in the background that stops the audio. So I fired that event in the close event as well and it worked.

#6 @justinahinon
5 months ago

48562.1.patch is fine and fixes the issue both for audios and videos files.

Last edited 5 months ago by justinahinon (previous) (diff)

This ticket was mentioned in Slack in #core-media by florian-tiar. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 months ago

#10 follow-up: @Clorith
3 months ago

Wondering if it might be a better approach to pause the playback, instead of triggering a refresh of the modal that's already been dismissed here (you still won't be able to resume if you open the same file, just like now, but I did notice that refreshing it triggers the update meta AJAX call when no changes have been made, while pausing did not appear to do so).

You should be able to achieve a similar approach to your current trigger, but instead you'll want to either trigger a click event on the mejs-pause button, or loop over any MediaElement instance and call the pause() function on them to do so.

This ticket was mentioned in Slack in #core-media by florian-tiar. View the logs.


4 days ago

#12 @antpb
4 days ago

  • Milestone changed from Future Release to 5.7

This is really close to being solved and would greatly improve the media library experience. As mentioned above, we may want to just pause the playback instead of refreshing things. I like the idea. Adding this to 5.7 so we can track along as we near release. :)

#13 @antpb
4 days ago

worth noting, if the pause approach is difficult or provides new bugs to solve, I think the refresh approach is a-ok.

@Mista-Flo
3 days ago

Improve patch, pause the player instead of refreshing everything

#14 in reply to: ↑ 10 @Mista-Flo
3 days ago

  • Keywords needs-testing added

Replying to Clorith:

Wondering if it might be a better approach to pause the playback, instead of triggering a refresh of the modal that's already been dismissed here (you still won't be able to resume if you open the same file, just like now, but I did notice that refreshing it triggers the update meta AJAX call when no changes have been made, while pausing did not appear to do so).

You should be able to achieve a similar approach to your current trigger, but instead you'll want to either trigger a click event on the mejs-pause button, or loop over any MediaElement instance and call the pause() function on them to do so.

Hi Clorith, thanks for your proposal, I just triggered a mejs-pause button pause, and it works, so I guess it's the simplest way to fix the issue.

This ticket was mentioned in Slack in #core-media by florian-tiar. View the logs.


3 days ago

#16 follow-up: @hellofromTonya
3 days ago

  • Keywords needs-testing-info added

As this ticket has been marked ready for testing, please provide need more information for testers to manually test the patch (including at Test Scrubs):

  • What are the steps to reproduce the reported problem?
  • What are the steps to test?
  • Are there any testing dependencies, such as a plugin or script?
  • What is the expected behavior after applying the patch?

#17 in reply to: ↑ 16 @Mista-Flo
3 days ago

  • Keywords needs-testing-info removed

Replying to hellofromTonya:

As this ticket has been marked ready for testing, please provide need more information for testers to manually test the patch (including at Test Scrubs):

  • What are the steps to reproduce the reported problem?
  • What are the steps to test?
  • Are there any testing dependencies, such as a plugin or script?
  • What is the expected behavior after applying the patch?

To reproduce:

  1. Go to the upload.php page (grid mode)
  2. Search for an audio file (or upload one long enough to test)
  3. Click on the audio file and play
  4. Close the media popup, the audio is still playing

To test:

  1. Apply the patch to your wordpress-develop environment
  2. Run npm run build:dev if you're vhost is set for src folder, otherwise npm run build

Try again the same steps to reproduce the issue, the audio should stop when closing the modal.

That's it.

Last edited 3 days ago by Mista-Flo (previous) (diff)
Note: See TracTickets for help on using tickets.