WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#24764 closed enhancement (fixed)

Support mediaelement.js YouTube sources in the video shortcode

Reported by: jancbeck Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.6
Component: Embeds Keywords: has-patch
Focuses: Cc:
PR Number:

Description

mediaelement.js supports youtube videos as source urls (see: mediaelementjs.com/examples/?name=youtube) but currently that feature is unused in favor of an embedded iframe.

It appears to me that embed urls are passed to the video shortcode as source attribute so a possible fix would be to have the shortcode accept youtube urls like this: [video src="http://www.youtube.com/watch?v=oAiVsbXVP6k"]

Attachments (1)

24764.diff (1.7 KB) - added by wonderboymusic 6 years ago.

Download all attachments as: .zip

Change History (15)

#1 @jancbeck
6 years ago

  • Cc dev@… added

#2 @ocean90
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

That sounds like plugin material. WP Embed works like a charm, there is no need for another shortcode to just display the video in another way.

#3 follow-up: @knutsp
6 years ago

Embedding doesn't always work like a charm when using the the visual editor. If you have some formatting turned on each line may be surrounded by a html element with style. This makes WordPress not see, or detect, the URL as on it's own line.

I the text editor it works like a charm. I always instruct my clients to paste the URL using the text editor, or use the shortcodes provided by Jetpack.

#4 @helen
6 years ago

  • Summary changed from Use mediaelement.js for youtube embeds to Support mediaelement.js YouTube sources in the video shortcode

I think the ticket title was a bit misleading, actually - I don't think jancbeck was proposing that we ditch oEmbed for YouTube, just that our mediaelement.js implementation should support a YouTube URL as the source. I'll ask wonderboymusic to come take a look - I don't know what it does/doesn't do now.

#5 in reply to: ↑ 3 @ocean90
6 years ago

Replying to knutsp:

This makes WordPress not see, or detect, the URL as on it's own line.

See #25387.

#6 follow-up: @wonderboymusic
6 years ago

  • Keywords needs-patch added
  • Milestone set to 3.9
  • Resolution wontfix deleted
  • Status changed from closed to reopened

This is actually pretty easy to accomplish, all it requires is switching the mime to video/youtube when src equals a YouTube-y URL - ME.js supports this out of the box.

I will throw it onto the AV2.0 pile for now for investigation - sorry big bad ocean90 closed this

#7 in reply to: ↑ 6 @jancbeck
6 years ago

Replying to wonderboymusic:

This is actually pretty easy to accomplish, all it requires is switching the mime to video/youtube when src equals a YouTube-y URL - ME.js supports this out of the box.

This is exactly what I ment. Sorry for not being more clear about this.

@wonderboymusic
6 years ago

#8 @wonderboymusic
6 years ago

  • Keywords has-patch added; needs-patch removed

24764.diff will play YouTube vids chromeless using ME.js - it's pretty cool. Supports the following patterns of YouTube URL as src

[video src="http://www.youtube.com/watch?v=oAiVsbXVP6k"]

[video src="http://youtube.com/watch?v=oAiVsbXVP6k"]

[video src="http://youtu.be/oAiVsbXVP6k"]

[video src="https://www.youtube.com/watch?v=oAiVsbXVP6k"]

[video src="https://youtube.com/watch?v=oAiVsbXVP6k"]

[video src="https://youtu.be/oAiVsbXVP6k"]

#9 @wonderboymusic
6 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from reopened to closed

In 27063:

Let the video shortcode accept a YouTube URL as the value of its src attribute, as MediaElement.js supports Chromeless YouTube videos by using a pseudo-mime-type video/youtube.

HTTP and HTTPS www, non-www, and short url fronts are supported:
http://www.youtube.com/watch
https://www.youtube.com/watch
http://youtube.com/watch
https://youtube.com/watch
http://youtu.be
https://youtu.be

Fixes #24764.

#10 @Viper007Bond
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Presumably that is not supposed to be a double dollar sign on $fallback on line 1228.

We should've had a unit test that would have exposed this due to the non-existent variable.

#11 @wonderboymusic
6 years ago

Those are supposed to be interpolated like that, are you getting a non-existent var error? you shouldn't

#12 @Viper007Bond
6 years ago

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

No errors because no testing -- I was just reading through commits. I saw $fallback on line 1225 and just 3 lines later I saw $$fallback so I assumed it was a mistake. I failed to notice that $$fallback was in use elsewhere in the file.

Sorry for the false alarm! Oops. :(

Last edited 6 years ago by Viper007Bond (previous) (diff)

#13 follow-up: @Fab1en
6 years ago

Hi @wonderboymusic. Would it be possible to enhance a bit this fix to make it possible to add other video providers ? You could introduce an array similar to this one :

$video_provider = array(
    'video/youtube' => '#^https?://(:?www\.)?(:?youtube\.com/watch|youtu\.be/)#'
);

and pass it to a filter so that plugin developers can add new providers. Then, they would just need to use a js video player that support those providers. For example, I [forked mediaelement.js](https://github.com/johndyer/mediaelement/pull/663) to make it support Dailymotion videos in the same way it supports YouTube ones.

#14 in reply to: ↑ 13 @SergeyBiryukov
6 years ago

Replying to Fab1en:

Would it be possible to enhance a bit this fix to make it possible to add other video providers ?

Feel free to create a new ticket with that suggestion.

Note: See TracTickets for help on using tickets.