Problem/Motivation

There is an issue with FitVids not applying to some videos embedded with oEmbed. When a video is embedded with oEmbed, the <iframe>'s src is /media/oembed/?url=https...., and the current logic in fitvids.module that handles custom vendors sets the selector to look for src URLs starting with the specified values, rather than looking for them in any part of the URL: iframe[src^="' . trim($value) . '"]'.

As a result, since the src of oEmbed embedded videos start with /media/oembed, custom vendors specified do not get FitVids applied.

On a separate, but semi-related note, YouTube videos embedded with oEmbed get their URL converted from youtube.com to youtu.be, so adding youtu.be as a custom vendor should apply to those videos after this issue is resolved.

Proposed resolution

In fitvids.module, update the custom vendor selector to search for <iframe>s matching the custom vendors in any part of the URL, rather than limited it to just those that begin with it, which will allow oEmbed videos to have FitVids applied.

Additionally, the selectors for the default providers in jquery.fitvids.js already use iframe[src*= instead of iframe[src^= to search for iframes matching any part of the src URL, so this will keep it consistent.

CommentFileSizeAuthor
#22 fitvids-fix-oembed-issue-3158833-21.patch544 bytessundhar
#21 fitvids-fix-oembed-issue-3158833-20.patch.patch544 bytessundhar
#12 fitvids-vimeo.png271.63 KBmrpauldriver
#2 fitvids-fix-oembed-issue-3158833-2.patch1.72 KBedwardchiapet

Issue fork fitvids-3158833

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

edwardchiapet created an issue. See original summary.

edwardchiapet’s picture

edwardchiapet
Location New York, NY
commented
StatusFileSize
new fitvids-fix-oembed-issue-3158833-2.patch1.72 KB

This patch should resolve the issue.

socialnicheguru’s picture

socialnicheguru commented
Status: Active » Needs review
loze’s picture

loze commented

Patch works. thanks.

abaier’s picture

abaier commented
Status: Needs review » Reviewed & tested by the community

Thanks for the patch – this also works for us. Setting this RTBC

mstrelan’s picture

mstrelan commented

Edge case to consider, you enter "youtube.com" and someone embeds a video from "totallynotyoutube.com" this is now subject to fitvid.js processing.

chegor’s picture

chegor commented

+1 RTBC

edwardchiapet’s picture

edwardchiapet
Location New York, NY
commented
Assigned: edwardchiapet » Unassigned
edwardchiapet’s picture

edwardchiapet
Location New York, NY
commented
mrpauldriver’s picture

mrpauldriver commented

I have not tested this, but in light of the comments regarding alternative YouTube domains, does this work with youtube-nocookie.com ?

mrpauldriver’s picture

mrpauldriver commented
Status: Reviewed & tested by the community » Needs work

Patch does not work for Vimeo.

mrpauldriver’s picture

mrpauldriver commented
StatusFileSize
new fitvids-vimeo.png271.63 KB

Vimeo

The video scales down responsively, but the parent oembed iframe does not scale with it.

Please see screenshot.

Probably related issue
#3060968: Some oEmbed videos do not maintain aspect ratio

mrpauldriver’s picture

mrpauldriver commented

I have found that by using the css below in my theme, I can manage without Fitvids completely and the problem with Vimeo is also resolved.

The padding-top value may cause a problem where videos arrive at different aspect ratios, but I think this would fix most situations.

.field--name-field-media-oembed-video {
  position: relative;
  overflow: hidden;
  width: 100%;
  padding-top: 56.25%; /* 16:9 Aspect Ratio (divide 9 by 16 = 0.5625) */
}

/* Then style the iframe to fit in the container div with full height and width */
.media-oembed-content {
  position: absolute;
  top: 0;
  left: 0;
  bottom: 0;
  right: 0;
  width: 100%;
  height: 100%;
}
zebda’s picture

zebda commented

#13 seems to work, only thing is that the css class should be: .field-name-field-media-oembed-video

caspervoogt’s picture

caspervoogt commented

The patch works, as does that CSS from #13. I think it's useful and appropriate for the module to support oembed iframes, without the need for custom CSS, even though the CSS from #13 works fine. RTBC

nickdickinsonwilde’s picture

nickdickinsonwilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
commented
Version: 8.x-1.x-dev » 2.0.x-dev

Patch solution seems default good. However... that would be a breaking change. I would support this if it was behind a settings flag for existing sites.

caspervoogt’s picture

caspervoogt commented

@MrPaulDriver thanks for that CSS, that helps.

imclean’s picture

imclean commented

I'm not sure this patch is necessary. Add the oembed URL (e.g. https://www.example.com/media/oembed) to "Additional video providers" to get fitvids to recognise it.

imclean’s picture

imclean commented

I'm also using this patch #2966656-34: Negotiate max width/height of oEmbed assets more intelligently.

sundhar made their first commit to this issue’s fork.

sundhar’s picture

sundhar
Location Coimbatore TN - India
commented
StatusFileSize
new fitvids-fix-oembed-issue-3158833-20.patch.patch544 bytes

#2 Patch Re-roll

sundhar’s picture

sundhar
Location Coimbatore TN - India
commented
StatusFileSize
new fitvids-fix-oembed-issue-3158833-21.patch544 bytes

Patch file renamed.

sundhar’s picture

sundhar
Location Coimbatore TN - India
commented

sundhar opened merge request !6

qpro’s picture

qpro
Primary language Spanish
Location Spain
commented

#18 works for me, thanks, it save my day, although it's a workaround that could lead to some problems because of the absolute path, for example when changing between environments with diferent domain (dev.example.com/beta.example.com/example.com)

imclean’s picture

imclean commented

@qpro, just add the URLs for all the environments, the field accepts multiple values.