Make WordPress Core

Opened 7 years ago

Closed 17 months ago

Last modified 15 months ago

#40144 closed defect (bug) (fixed)

Media: Unable to selectively opt for MediaElement.js for just audio or video shortcodes

Reported by: westonruter's profile westonruter Owned by: joedolson's profile joedolson
Milestone: 6.2 Priority: normal
Severity: normal Version: 3.6
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

Let's say someone adds a plugin that does the following:

  • add_filter( 'wp_video_shortcode_library', function(){ return 'some_other_library'; } );
  • add_filter( 'wp_audio_shortcode_library', function(){ return 'some_other_library'; } );

If both of the filters are added, then wp-mediaelement won't be enqueued. However, if only one of the filters are added, then wp-mediaelement will still get enqueued. When wp-mediaelement is enqueued, it uses this selector to find the elements to initialize with MediaElement.js:

$( '.wp-audio-shortcode, .wp-video-shortcode' )

Naturally this means that even if I have filtered wp_video_shortcode_library to not be mediaelement, the JS will still try to set it up for MediaElement.js if I didn't also filter wp_audio_shortcode_library to not be mediaelement.

The JS needs to be aware of what the wp_video_shortcode_library and wp_audio_shortcode_library filters so that it can know whether or not to include .wp-audio-shortcode and .wp-video-shortcode among the selectors.

Originally raised in https://github.com/xwp/wp-core-media-widgets/pull/5#issuecomment-285198669

Attachments (4)

40144.0.diff (2.0 KB) - added by westonruter 7 years ago.
40144.1.diff (4.6 KB) - added by joedolson 18 months ago.
Refreshed patch
40144.2.diff (2.2 KB) - added by rudlinkon 17 months ago.
Refreshed patch UTF-8 encoded
40144.3.diff (1.9 KB) - added by rudlinkon 17 months ago.
Refreshed patch without .env file

Download all attachments as: .zip

Change History (16)

@westonruter
7 years ago

#1 @westonruter
7 years ago

  • Keywords has-patch added
  • Version set to 3.6

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


7 years ago

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


21 months ago

#4 @joedolson
21 months ago

  • Milestone changed from Awaiting Review to 6.2
  • Owner set to joedolson
  • Status changed from new to accepted

Milestoning for 6.2 after scrubbing. This makes a lot of sense.

#5 @joedolson
18 months ago

Testing instructions:

  • Create a post in the classic editor
  • Add an audio file and a video file using the [audio] and [video] shortcodes.
  • Add the two filters
add_filter( 'wp_video_shortcode_library', function(){ return 'some_other_library'; } ); 
add_filter( 'wp_audio_shortcode_library', function(){ return 'some_other_library'; } ); 

Test once with both filters available; once with only one filter enabled; one with the other filter enabled.

Before patch:

  • Both filters: both audio and video fall back to native players.
  • Audio filter: both audio and video fall back to native players.
  • Video filter: both audio and video fall back to native players.

After patch:

  • Both filters: both audio and video fall back to native players.
  • Audio filter: Audio player falls back to native player.
  • Video filter: Video player falls back to native player.

@joedolson
18 months ago

Refreshed patch

#6 @joedolson
18 months ago

  • Keywords needs-testing added

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


17 months ago

@rudlinkon
17 months ago

Refreshed patch UTF-8 encoded

@rudlinkon
17 months ago

Refreshed patch without .env file

#8 @obayedmamur
17 months ago

I have tested with the refreshed patch: https://core.trac.wordpress.org/attachment/ticket/40144/40144.3.diff

I have created a post in the classic editor and:

Added an audio file and a video file using the [audio] and [video] shortcodes.

Added these two filters in themes function.php file:

add_filter( 'wp_video_shortcode_library', function(){ return 'some_other_library'; } );
add_filter( 'wp_audio_shortcode_library', function(){ return 'some_other_library'; } );


Tested once with both filters available; once with only one filter enabled; one with the other filter enabled.

Before patch:
Both filters: Both audio and video fall back to native players - https://d.pr/i/k7OONM
Audio filter: Both audio and video fall back to native players - https://d.pr/i/Rm8NHI
Video filter: Both audio and video fall back to native players - https://d.pr/i/d75r7x

After patch:
Both filters: Both audio and video fall back to native players - https://d.pr/i/9BQuh4
Audio filter: Audio player falls back to native player - https://d.pr/i/IOshs4
Video filter: Video player falls back to native player - https://d.pr/i/eoybpX

After Patch I have found the expected result.

#9 @joedolson
17 months ago

  • Keywords commit added; needs-testing removed

This ticket was mentioned in PR #4025 on WordPress/wordpress-develop by @joedolson.


17 months ago
#10

Allow filters to disable mediaelement shortcodes selectively

Trac ticket: https://core.trac.wordpress.org/ticket/40144

#11 @joedolson
17 months ago

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

In 55271:

Media: Enable selective optout for video and audio shortcodes.

Make the JavaScript selectors for audio and video shortcodes aware of the state of the wp_video_shortcode_library and wp_audio_shortcode_library filters. Allow extenders to replace the library for either media shortcode.

Props westonruter, joedolson, rudlinkon, obayedmamur.
Fixes #40144.

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


15 months ago

Note: See TracTickets for help on using tickets.