WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#40819 closed defect (bug) (fixed)

Audio widget in WP 4.8 - unsupported audio files (eliminate support for WMV and WMA files)

Reported by: Guido07111975 Owned by: westonruter
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.8
Component: Media Keywords: has-patch commit has-dev-note
Focuses: Cc:

Description

Hi members of Core team,

When using an unsupported audio file such as .wma (Windows Media Audio) it doesn't display nicely in both frontend/backend. Check attachment.

Guido

Attachments (6)

audio-widget-wma.jpg (12.5 KB) - added by Guido07111975 4 months ago.
WP 4.8 audio widget
audio-widget-dashboard-wma.jpg (45.4 KB) - added by Guido07111975 4 months ago.
WP 4.8 audio widget (dashboard)
40819.diff (80.6 KB) - added by vijustin 4 months ago.
audio-widget-add-wma.jpg (89.1 KB) - added by Guido07111975 4 months ago.
I can select .wma file when adding audio widget
40819.2.diff (1.1 KB) - added by westonruter 4 months ago.
Remove wma and wmv from being WP-supported
unsupported-4.8-beta-2.jpg (27.7 KB) - added by Guido07111975 4 months ago.
Unsupported file in 4.8 beta 2

Download all attachments as: .zip

Change History (30)

@Guido07111975
4 months ago

WP 4.8 audio widget

@Guido07111975
4 months ago

WP 4.8 audio widget (dashboard)

@vijustin
4 months ago

#1 @vijustin
4 months ago

  • Keywords has-patch needs-testing added

#2 @vijustin
4 months ago

Took a stab at adjusting the CSS to produce a better output for non supported audio files.

#3 @westonruter
4 months ago

  • Keywords reporter-feedback added; has-patch needs-testing removed

@vijustin Modifying mediaelementplayer.min.css isn't an option since it is part of a 3rd-party library. I see a similar issue was posted in https://github.com/mediaelement/mediaelement/issues/1728

@Guido07111975 Thanks for the report. You'll notice actually that if you have a WMA file uploaded to your media library, it is not listed among the files that you can select. How did you select the WMA file to begin with? Note that there is a quirk with the uploader whereby it is not restricting the file types for upload according to what is shown in the media browser. Is that how you were able to upload and select an WMA file? See https://github.com/xwp/wp-core-media-widgets/issues/128

Or did you select it via supplying a URL to embed?

#4 @vijustin
4 months ago

Working off of Version 4.8-beta1-40787, when selecting audio files to add the the Audio Widget .wma files are showing as an option for me.

#5 @Guido07111975
4 months ago

@westonruter I was able to upload .wma file directly in the media library and select it while adding the audio widget (check new attachment).

I can also upload .wma file while adding the audio widget BUT it's not listed there after upload is finished. When visiting the media library afterwards, the file is listed there.

Guido

@Guido07111975
4 months ago

I can select .wma file when adding audio widget

#6 follow-up: @westonruter
4 months ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.8

Indeed. I can see in the Ajax request that the MIME types being queried are: audio/mpeg,audio/ogg,audio/x-ms-wma,audio/mpeg,audio/wav. The audio/x-ms-wma format really shouldn't be included because it is not embeddable. If MediaElement.js isn't able to play them, then they shouldn't be included in wp.media.view.settings.embedMimes.

And in fact, I see that MediaElement.js no longer supports these formats:

Support for wmv and wma has been dropped since most of the major players are not supporting it as well.

The solution to this may be to eliminate WMV and WMA from being considered embeddable media.

@Guido07111975 @vijustin the behavior you see for embedding WMV and WMA files is no better in the context of the edit post screen, is it?

#7 @Guido07111975
4 months ago

@westonruter no strange behaviour when adding unsupported .wma file in post or page. So yes, I can add this file type in post or page.

#8 @vijustin
4 months ago

@westonruter Confirmed, when adding .wma files in the post editor the "Attachment Display Setting" defaults to Embed Media Player which creates the same issue as the audio widget. Removing aduio/x-ms-wma as a supported mime type for embeding seems like the a good solution to this problem.

I spent some time looking so I could submit a .diff, but I'm can't seem to find where WordPress defines which mime types should be supported for embedding. Looks like this one is beyond me at the moment.

@westonruter
4 months ago

Remove wma and wmv from being WP-supported

#9 @westonruter
4 months ago

  • Keywords has-patch added; needs-patch removed
  • Summary changed from Audio widget in WP 4.8 - unsupported audio files to Audio widget in WP 4.8 - unsupported audio files (eliminate support for WMV and WMA files)

@vijustin @Guido07111975 try 40819.2.diff

#10 @vijustin
4 months ago

@westonruter Ahh man can't believe I couldn't find that.

On a positive note after applying the patch locally the Audio Widget no longer offers .wma files as an option and adding media during post editing no longer offers the 'embedded media player' as an option and instead defaults to 'link to media file".

#11 @westonruter
4 months ago

  • Keywords 2nd-opinion added

Need a 2nd opinion on the impacts for removing WMA and WMV format support.

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


4 months ago

#13 @westonruter
4 months ago

  • Keywords commit needs-dev-note added; 2nd-opinion removed

This will probably need a dev note.

#14 @westonruter
4 months ago

  • Owner set to westonruter
  • Status changed from new to accepted

#15 @westonruter
4 months ago

  • Component changed from Widgets to Media

#16 follow-up: @Guido07111975
4 months ago

@westonruter I'm having the same experience as @vijustin describes:

On a positive note after applying the patch locally the Audio Widget no longer offers .wma files as an option and adding media during post editing no longer offers the 'embedded media player' as an option and instead defaults to 'link to media file".

Besides this I'm wondering whether it's possible to do a filetype check upon upload. I can upload all supported filetypes while adding the (audio/video/image) widget.

Guido

#17 @joemcgill
4 months ago

Based on MediaElement.js dropping support for these filetypes, I think it makes sense for us to remove embed support since the current behavior creates confusing user expectations.

#18 in reply to: ↑ 16 @westonruter
4 months ago

Replying to Guido07111975:

Besides this I'm wondering whether it's possible to do a filetype check upon upload. I can upload all supported filetypes while adding the (audio/video/image) widget.

Yes. I believe this should be addressed with the resolution of https://github.com/xwp/wp-core-media-widgets/issues/128

Essentially, when opening the media modal for the audio widget, the uploader should prevent uploading WMA files at that time. @joemcgill is there a trac ticket for this anywhere?

#19 @westonruter
4 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 40813:

Media: Remove core embedding support for WMV and WMA files since MediaElement.js has discontinued supporting them.

Plugins may continue to add embedding support for these file formats by re-adding them via the wp_video_extensions and wp_audio_extensions filters while also implementing fallback rendering routines via the wp_video_shortcode_override and wp_audio_shortcode_override filters.

See #39994, #39995.
Fixes #40819.

#20 in reply to: ↑ 6 ; follow-up: @afercia
4 months ago

Replying to westonruter:

Support for wmv and wma has been dropped since most of the major players are not supporting it as well.

I may be wrong, but I think that applies to MediaElement starting with version 3. WordPress still uses 2.22, see #39686.
Regardless, FWIW I'd agree with removing core embedding support for WMV and WMA files. Not sure what should be done with these two lines: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/mediaelement/wp-mediaelement.js?rev=40659#L6

#22 in reply to: ↑ 20 @westonruter
4 months ago

Replying to afercia:

Not sure what should be done with these two lines: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/mediaelement/wp-mediaelement.js?rev=40659#L6

I don't see any harm in keeping them around for now, but eventually they can be removed.

#23 @Guido07111975
4 months ago

Hi,

Just installed 4.8 beta 2 and I notice an unsupported file still isn't listed very nice in dashboard. Check attachment.

@vijustin added this attachment last week: https://core.trac.wordpress.org/attachment/ticket/40819/40819.diff

Guido

@Guido07111975
4 months ago

Unsupported file in 4.8 beta 2

#24 @westonruter
4 months ago

Patching mediaelementplayer.min.css is not really an option. The issue needs to be fixed in MediaElement.js itself, and it may already be fixed since core is using ME.js 2.22 whereas ME.js 4.x is now out and is in the process of being upgraded to. See #39686.

Note: See TracTickets for help on using tickets.