Create more appropriate default sizing for oembed videos

As all the modern browsers supports the aspect-ratio property. I have used the same and fixes the issue. please review

This will work for a 16/9 video - do we want to (or should we?) try to solve for other use cases? I don't know if there is a standard approach to a core theme, but either:

1 - We could add classes that allow for common video ratios, probably something like:

16:9 (Widescreen) 16:9
9:16 (Vertical) ...
1:1 (Square) ...
4:3 (Fullscreen) ...
21:9 (Cinematic Widescreen)

2 - We could look to calculate the ratio? I am aware of https://github.com/davatron5000/FitVids.js as jquery solution, and am also aware that jquery is no longer native in 9.x onwards. Is there a javascript approach in core that would help?

Of course these points are moot if its ok for the theme to be opinionated about 16/9 ratios being expected!

As mentioned in #4, this is a very rigid solution, but I don't think adding more, different rigid solutions is the answer. Instead, we probably want to access the iframe's set width/height. Those attributes are set here, but I'm not really sure how we would best make use of them. I'll reach out to the media maintainers to see if they have any input.

New patch attached with new approach that should be more flexible.

Could this be related ✨ Negotiate max width/height of oEmbed assets more intelligently Needs work

Yes, media using oEmbed in core isn't fully responsible and there are issues in certain scenarios dealing with dimensions (or lack of) that come from the providers. For example:

🐛 Some oEmbed videos do not maintain aspect ratio Needs work
🐛 oEmbed videos are not fully responsive Needs work
✨ Negotiate max width/height of oEmbed assets more intelligently Needs work

each focusing on one aspect of the same problem, IMO.

So ultimately, I believe this should be solved by the core media module in a generic way for all providers. Having that said, if in the meantime Olivero has a way to workaround it in a generic and simple way, maybe that's reasonable, and also serves as example of how sites can do it in their themes? We could leave a @todo in the workaround to remove it when the iframe is made fully responsible by the core module?

Also a little suggestion on the latest patch:

+++ b/core/themes/olivero/olivero.theme
@@ -359,6 +359,19 @@ function olivero_preprocess_radios(&$variables) {
+  if ($variables['entity_type'] === 'media' && $variables['field_name'] === 'field_media_oembed_video') {

This field can be named anything really. It would be great if we could target this in a more generic way. How about something like this instead?

  if (!empty($variables['element']['#object']) &&
    ($variables['element']['#object'] instanceof \Drupal\media\MediaInterface) &&
    ($variables['element']['#object']->getSource() instanceof \Drupal\media\Plugin\media\Source\OEmbedInterface)) {
    ...
  }

Thanks for the feedback, Marcos!

Moved to a MR because I hate making interdiffs.

Surprised the bot hasn't got this. But build failure in MR.

Also FWIW I'm reviewing ✨ Negotiate max width/height of oEmbed assets more intelligently Needs work right now too.

Trying to fix the CC Failed and improved one line of code.

Please review.

Added 1 comment to MR.

Can anyone confirm ✨ Negotiate max width/height of oEmbed assets more intelligently Needs work solve the problem.

Hi @smustgrave, that issue doesn't fully solve the issue that this issue brings up. While it will return something that isn't 200x113 if you set a maxwidth and maxheight, it only does so for the <video>. The enclosing iframe will create black bars around the video depending on the width of the viewport or width of the container the iframe is in. It's definitely better than before though!

Now that #2966656 ✨ Negotiate max width/height of oEmbed assets more intelligently Needs work is in, I think maybe the issue is the styling of the iframe's aspect ratio.

From what I can tell the only CSS that gets added by default to the iframe is:

.media-oembed-content {
  max-width: 100%;
  border: none;
  background-color: transparent;
}

That max-width: 100% keeps the iframe from breaking out of its container, making it responsive, but does nothing for maintaining the aspect ratio. If you have a 16/9 video and shrink your browser down to a mobile width size, the video becomes a square. It will still play the video itself at 16/9, but there will be two huge black bars on top and bottom. Ideally, those won't be there.

Using CSS like in #3 won't work as that will affect ALL oEmbeds, such as podcast players, 3D models, etc. Anything that can be embedded via oEmbed by the oEmbed Providers module for example. I tried it and had a too tall Podbean player.

I tested a YouTube embed in WordPress, both a 16/9 one and a different video (https://www.youtube.com/watch?v=UDIQwGb-4YQ) that isn't 16/9, but it looks like WP always embeds at 16/9 and you're just stuck with black bars. So maybe that's the answer here, you'd just need to deal with it in your theme. But if we could manually choose what we want the video's aspect ratio to be, on a per-video basis, that would be great. I'm not sure how to do that though.

For the record, changing the above code to this:

.media-oembed-content {
  max-width: 100%;
  border: none;
  background-color: transparent;
  aspect-ratio: 16/9;
  height: auto;
}

will get rid of the black bars, but then you're hard-coding the aspect-ratio for all oEmbeds. which isn't good. The only thing I can think of is a field on the media item that lets you select an aspect ratio, and then it will add CSS classes (or inline styles) to the media item somehow. 🤷🏻‍♂️

The iframe element will have width="" and height="" attributes. The width and height values returned from the oembed provider will allow you to work out the aspect ratio, but this would require more than just CSS to make use of.

That said, I tend to use FitVids → to solve this problem.

Thank you @dalemoore for the CSS. This works for my case. I am only embedding Vimeo Videos. So default aspect-ratio:16/9 is fine for me.