Problem/Motivation

A YouTube URL like https://www.youtube.com/embed/videoID is a valid URL and valid YouTube video ID can be extracted from this URL. But right now this is not possible due to YouTube::getIdFromInput() regex limitations.

Proposed resolution

Improve the video ID detection in YouTube::getIdFromInput() to allow video embed URLs like https://www.youtube.com/embed/videoID.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2899093-2.patch, failed testing. View results

Sam152’s picture

Status: Needs work » Closed (works as designed)

There is already a provider written to handle youtube playlists. I think this already works as expected.

claudiu.cristea’s picture

Issue summary: View changes
Status: Closed (works as designed) » Needs work

It's not about playlists, I'll remove that from the ticket. It's about improving the regex for YouTube to allow URLs like https://www.youtube.com/embed/<videoID>, which are valid URLs.

claudiu.cristea’s picture

Status: Needs work » Needs review

EDIT: comment deleted

claudiu.cristea’s picture

OK, Now this resolves also the https://www.youtube.com/embed/<videoID> case, but not https://m.youtube.com/embed/<videoID> (which is not allowed).

Other improvements:

  • Added the mobile host alternative: http://m.youtube.com/...
  • Allow missing completely the protocol: youtube.com/... or www.youtube.com/...
Sam152’s picture

+++ b/src/Plugin/video_embed_field/Provider/YouTube.php
@@ -82,7 +82,7 @@ class YouTube extends ProviderPluginBase {
-    preg_match('/^https?:\/\/(www\.)?((?!.*list=)youtube\.com\/watch\?.*v=|youtu\.be\/)(?<id>[0-9A-Za-z_-]*)/', $input, $matches);
+    preg_match('#^(?:https?://)?(?:youtu\.be/|(?!.*list=)(?:www\.|(?!.*embed/)m\.)?youtube\.com/(?:(?:watch)?\?(?:.*&)?v=|embed/))(?<id>[0-9A-Za-z_-]+)#i', $input, $matches);

What about creating two regexes? This is getting really big.

claudiu.cristea’s picture

In fact this is true also for Vimeo. https://player.vimeo.com/video/225133231 is a iframe embed URL but should be a valid video URL.

slashrsm’s picture

I can confirm that the patch works as expected. I added support for Vimeo as proposed in #10.

slashrsm’s picture

While working on migration also ran into cases with implicit protocol on Youtube. Also covered that.

MaskOta’s picture

Status: Needs review » Needs work

This is probably not the scope of this issue but... shouldn't we also try to parse any additional options that the user might want to input?

for example if you have:

https://www.youtube.com/embed/TLzf4ovpVWY?autoplay=1&controls=0&loop=1&p...

the video would then autoplay, loop etc.

claudiu.cristea’s picture

Status: Needs work » Needs review

If it's not in the scope of the issue please don't switch the status.

MaskOta’s picture

gaurav.bajpai’s picture

Comment 12 is working for me for mobile youtube URL (m.youtube.com)

Anybody’s picture

Status: Needs review » Needs work

Please also extend the regex to allow "https://www.youtube-nocookie.com/embed" which is used for privacy (GDPR, ...).
We should allow users to also paste that code. The parameters are the same as for youtube. I don't think it would make sense to have a separate issue for that patch because it's very close to this one.

idimopoulos’s picture

Status: Needs work » Needs review
FileSize
11.04 KB
12.47 KB

I did some minor changes to this patch.

  • I split out the regex string into multiple a bit cleaner regexes.
  • I added the youtube-nocookie domain for embedded URLs as this issue is already for embedded URLs.
  • I updated the way the ID is calculated based on upstream references. Previously, we were grabbing only letters and numbers, however, according to https://webapps.stackexchange.com/questions/54443/format-for-id-of-youtu... no assumptions can be made either on the characters allowed or the length of the format. Thus, the id is calculated given on specific URL related characters like the character #
  • I fixed a minor case in the regex where the https://// was acceptable.
idimopoulos’s picture

The latest patch from #18 also applies to 8.x-2.x. Should we clone the issue for that too?

Also, I postponed https://www.drupal.org/project/video_embed_field/issues/3060201 in favor of the current ticket.

zaporylie’s picture

Version: 8.x-1.x-dev » 8.x-2.x-dev
Status: Needs review » Reviewed & tested by the community

Since 2.x is the current default and this patch applies there I'm changing the targeting version. Version will be changed back once patch gets in to 8.x-2.x first.

I reviewed the patch + applied on the project with variety of youtube urls (many different formats) and, thanks to #18, all these formats are now supported. I don't see any remaining blocking issues hence I'm changing status to RTBC.

Chris Matthews’s picture

Sam152’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

With the advent of Media in core, the Video Embed Field module has moved to being minimally maintained. Only issues which assist in the migration to Media in core will be committed. To read more about this decision, please see: #3089599: Maintenance status for Video Embed Field now that media is in core.

mkindred’s picture

Patch #18 worked for me in D9.2.9.

GagikSargsyanWeb’s picture

Patch #18 worked for me too in D9.3.15, PHP 7.4.27