Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Giant space above all Youtube video in Posts #10109

Closed
Pikkals opened this issue Sep 22, 2018 · 99 comments · Fixed by #10477
Closed

Giant space above all Youtube video in Posts #10109

Pikkals opened this issue Sep 22, 2018 · 99 comments · Fixed by #10477
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.

Comments

@Pikkals
Copy link

Pikkals commented Sep 22, 2018

Describe the bug
Giant space above all Youtube video when VIEWING Posts in Gutenberg: Version 3.9.0

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'ALL POSTS.'
  2. Click on 'EDIT' A POST YOU KNOW HAS A YOUTUBE VIDEO
  3. Scroll down to 'YOUTUBE VIDEO BLOCK'
  4. You CAN'T SEE the error in the actual Post Edit, but you can see the giant space when VIEWING the post.

Expected behavior
There should be reasonable space around the Youtuve video.

Screenshots
error - space above youbue video

Desktop (please complete the following information):

  • OS: WINDOWS 10
  • Browser CHROME
  • Version 68.0.3440.106 (Official Build) (64-bit)

Additional context
Latest version of Gutenberg: Version 3.9.0

@mkaz
Copy link
Member

mkaz commented Sep 22, 2018

Thanks for reporting the bug, agreed that does not look right, and I'd imagine frustrating.

I see the whitespace when I visit your site, however I'm not able to reproduce it. I installed the Hueman Free theme, and using the Autooptimize plugin, which I noticed are what you use on your site at: https://www.discerningtheworld.com/

When I create a post with a YouTube embed, I'm not able to get the large space.

To help anyone else when troubleshooting:

I noticed on your embed there is an additional classname wp-embed-aspect-4-3 which when present adds the padding at the top. I can see in the code if the frame width and height are specified that it will add, but I did not see how that would be specified directly in Gutenberg.

Code in: packages/block-library/src/embed/index.js

Can you let me know how you added the embed, and if there is anything you did after adding the embed to help reproduce and solve the issue?

@Pikkals
Copy link
Author

Pikkals commented Sep 22, 2018

Hi mkaz

Yes I saw that "wp-embed-aspect-4-3" in the CSS and because I am not a programmer at all, I had no idea if this should be there or not.

So I will remove it and come back to you.

I added the embed video, by adding a block, then clicking Youtube and only adding the youtube video url nothing else, just the plain url to the video. Everything was working fine till the latest version upgrade.

@Pikkals
Copy link
Author

Pikkals commented Sep 22, 2018

Hi mkaz

Ahhhhh Ha! "wp-embed-aspect-4-3" is indeed the culprit, I removed it and the giant space is gone. But now the big question, where is this additional CSS coming from ? It was not there yesterday. All was hunky dory until I updated to new version of Gutenberg last night.

I see on other posts it's added additional CSS "wp-has-aspect-ratio wp-embed-aspect-16-9". And it too is making a big white space above all my videos.

What do you suggest I do? I can't sit and manually go through 600 articles removing this CSS that's appeared out of nowhere. Help.

@mkaz
Copy link
Member

mkaz commented Sep 22, 2018

ok, good to see we were able to narrow it down.

Will need to investigate further to see what it was trying to address and figure out a solution.

@Pikkals
Copy link
Author

Pikkals commented Sep 22, 2018

You got me thinking.

I deactivated the Autoptimize plugin and taaadaaaa.... problem resolved.

I will play around with the settings and see if I can get Autoptimize to work without breaking my site...

Do you recommend another plugin similar to Autoptimize?

@SolespireMarcus
Copy link

You got me thinking.

I deactivated the Autoptimize plugin and taaadaaaa.... problem resolved.

I will play around with the settings and see if I can get Autoptimize to work without breaking my site...

Do you recommend another plugin similar to Autoptimize?

In the meantime of having the plugin deactivated, you should report that problem to the developers of Autoptimize: https://wordpress.org/plugins/autoptimize/#developers

@Pikkals
Copy link
Author

Pikkals commented Sep 23, 2018

Thank you Marcus.

I have contacted Autoptimiae support here: https://wordpress.org/support/topic/giant-space-above-all-youtube-video-when-viewing-posts-in-gutenberg-v-3-9-0/

@futtta
Copy link

futtta commented Sep 23, 2018

Heya everyone;
I'm Autoptimize's developer :-)

I actually still see the gap even with AO disabled, cfr. this screenshot;

image

There's a padding-top:75% and a padding-top:50% (both for .wp-block-embed__wrapper::before) in wp-content/plugins/gutenberg/build/block-library/style.css. Disabling those rules in the inspector "fixes" the problem?

Happy hunting!
frank

@Pikkals
Copy link
Author

Pikkals commented Sep 23, 2018

Goodgrief, the error is back... I'm now confused... Never the less...

Frank you are a superstar!!!! 🥇

@mkaz
Copy link
Member

mkaz commented Sep 23, 2018

Thanks for jumping in and looking @futtta - agreed, I don't think it is a conflict with Autoptimize.

To help troubleshoot:

@Pikkals what steps did you do when adding the YouTube embed? It looks like older posts do not have that same issue with embeds, likely because they were not created in Gutenberg. Can you try to delete and add the YouTube block again in that post?

For @notnownikki and @jasmussen who worked on the issue:

It looks like an unintended consequence from this PR #9770 in 3.9.0 the iframe width/height is being detected properly and the additional aspect ratio class is added, however, when the new class is added it includes a large padding at the top.

It looks like the styles in #9550 are what is causing the issue here, but I'm not able to reproduce in my tests when adding a block, using same theme as user.

@chrisvanpatten chrisvanpatten added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks Needs Testing Needs further testing to be confirmed. labels Sep 23, 2018
@chrisvanpatten
Copy link
Member

chrisvanpatten commented Sep 23, 2018

Hi all -

I took a look at this and I think this may be a plugin conflict although I don't believe it's Autoptimize in this case.

When I insert a YouTube embed on my own site, I see this HTML rendered out:

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube wp-has-aspect-ratio wp-embed-aspect-16-9">
	<div class="wp-block-embed__wrapper">
		<iframe width="660" height="371" src="https://www.youtube.com/embed/GLwz5IJ4AsY?feature=oembed" frameborder="0" allow="autoplay; encrypted-media" allowfullscreen=""></iframe>
	</div>
</figure>

However when I look at a YouTube embed on your site, I see the following:

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube wp-has-aspect-ratio wp-embed-aspect-4-3">
	<div class="wp-block-embed__wrapper">
		<div class="video-container">
			<span class="embed-youtube" style="text-align:center; display: block;">
				<iframe class="youtube-player" type="text/html" width="640" height="360" src="https://www.youtube.com/embed/Bp00Wh-oaOQ?version=3&amp;rel=1&amp;fs=1&amp;autohide=2&amp;showsearch=0&amp;showinfo=1&amp;iv_load_policy=1&amp;start=108&amp;wmode=transparent" allowfullscreen="true" style="border:0;"></iframe>
			</span>
		</div>
	</div>
</figure>

Note all the extra <div> and <span> tags and CSS classes.

@Pikkals could you provide a list of plugins on your site? I would guess that another plugin is attempting to handle the responsive video, and is colliding with the way Gutenberg handles it.

@Jeanneavelo
Copy link

Hi,

Same problem on my blog with Hueman free theme. I don't use any editor plugin (could other kind of plugins be involved ?).

See for example at the bottom of that post modified with Gutenberg 3.9.0 today : http://jeanneavelo.fr/2018/02/22/un-carrefour-urbain-ordinaire-aux-pays-bas/

Older posts (i.e. not edited with Gutenbegrg 3.9.0) are not concerned.

@chrisvanpatten
Copy link
Member

Sounds then like it's an issue with the Hueman Free theme in particular then. Perhaps worth reaching out to the theme developer about this.

Also possibly worth looking at whether this should be another opt-in theme feature via add_theme_support to avoid potential conflicts. That ship may have sailed since this feature is now in Gutenberg, but might at least be worth thinking about.

@chrisvanpatten chrisvanpatten added [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. and removed Needs Testing Needs further testing to be confirmed. labels Sep 23, 2018
@Pikkals
Copy link
Author

Pikkals commented Sep 24, 2018

Hi guys

I am using the Hueman Pro theme. I will contact the developer and send them here to have a look.

And yes you are right Jeanneavelo posts that have not been edited with Gutenberg are fine.

Ok, I have logged a ticket with Hueman support.

@Pikkals
Copy link
Author

Pikkals commented Sep 24, 2018

Hi chrisvanpatten

Here is a list of all my plugins:

THEME | VERSION : Hueman Pro | v1.1.3
WP VERSION : 4.9.8
- Accelerated Mobile Pages (version 0.9.97.16)
- Agreeable (version 1.5)
- Akismet Anti-Spam (version 4.0.8)
- Auto-Post To Instagram (version 1.4.3)
- Autoptimize (version 2.3.4)
- Childify Me (version 1.2.0)
- Comment Approved (version 1.5.2.3)
- Comments-advanced (version 2.0)
- Comment Toolbar (version 1.4.3)
- Custom Taxonomy Order (version 2.9.5)
- Disable Gutenberg Autosave (version 1.0.1)
- Display All Image Sizes (version 1.1.6)
- EU Cookie Law (version 3.0.5)
- Force Regenerate Thumbnails (version 2.0.6)
- Glue for Yoast SEO & AMP (version 0.4.3)
- Google Analytics Dashboard for WP (GADWP) (version 5.3.5)
- Google Language Translator (version 5.0.48)
- Gutenberg (version 3.9.0)
- Jetpack by WordPress.com (version 6.5)
- List category posts (version 0.78.1)
- MailChimp (version 1.5.7)
- News Announcement Scroll (version 8.8.6)
- Quick Featured Images (version 13.3.2)
- Recent Posts Widget With Thumbnails (version 6.2.1)
- RefTagger (version 2.1.2)
- Schema (version 1.7.1)
- SEO Image Toolbox (version 3.3.1)
- Simple Comment Editing (version 2.1.11)
- Simple Tags (version 2.4.7)
- Sucuri Security - Auditing, Malware Scanner and Hardening (version 1.8.18)
- Target Blank In Posts And Comments (version 3.2)
- Term Management Tools (version 1.1.4)
- VaultPress (version 1.9.6)
- wp-Monalisa (version 4.6)
- WP 404 Auto Redirect to Similar Post (version 0.7.7)
- WP Edit (version 4.0.3) (I'VE DEACTIVATED THIS PLUGIN)
- WP Super Cache (version 1.6.4)
- Yoast SEO Premium (version 8.2.2)

@jasmussen
Copy link
Contributor

Good ticket, thank you.

So this issue happens because embeds created in Gutenberg are now responsive out of the box. The issue here is that if the theme itself also provides responsiveness for embeds through filtering the output, you end up with a double fix, which makes this extra space appear.

There are a number of ways to fix this, @chrisvanpatten I would appreciate your thoughts here as well.

Option 1: Move the responsive Gutenblock CSS from style.scss to theme.scss. The pro is this means Gutenblocks are only responsive if the theme author asks for it, and is therefore aware of it. Con: it means we can't provide responsive videos out of the box.

Option 2: We make it a user toggle, so a user can toggle the responsiveness off on a per-embed basis, if we default to On.

Option 3: Here's some CSS that seems to fix it in the case of THIS particular theme:

.wp-block-embed__wrapper div, .wp-block-embed__wrapper span {
    padding: inherit;
    position: static;
}

But this is a little hacky because it assumes a particular way of adding responsiveness to the videos.

What do you think? CC: @mtias

@Pikkals
Copy link
Author

Pikkals commented Sep 24, 2018

Hmmmm

I see I have a plugin called - WP Edit (version 4.0.3) installed,

I am going to deactivate this and see what happens.

Ok, I see I still have the problem. I'll keep this plugin deactivated as I don't see the need for it.

@notnownikki
Copy link
Member

Option 2: We make it a user toggle, so a user can toggle the responsiveness off on a per-embed basis, if we default to On.

I had this on my list to suggest too, thinking of wider themes and narrower videos where it might not look that great.

jasmussen added a commit that referenced this issue Sep 24, 2018
This PR fixes #10109.

But it does so in a semi-nuclear way, by removing the responsiveness by default, and making it something a theme author has to opt into by opting into Gutenberg "opinionated" styles. This might be the solution we go with, hence this PR, but I would love for us to consider it a "Plan B" while we explore better solutions.
@Jeanneavelo
Copy link

Ok, I have logged a ticket with Hueman support.

So did I : https://wordpress.org/support/topic/display-issue-with-youtube-embedded-video-in-gutenberg-3-9-0-edited-post/

@Pikkals
Copy link
Author

Pikkals commented Sep 24, 2018

I logged my ticket with [email protected] and asked the developers to please come to this thread.

@notnownikki
Copy link
Member

As much as it disappoints me, I think we might have to make this opt in. Or, perhaps we make responsive videos opt-in (or out?) at the document level instead of the block level?

@jasmussen
Copy link
Contributor

jasmussen commented Sep 24, 2018

As much as it disappoints me, I think we might have to make this opt in. Or, perhaps we make responsive videos opt-in (or out?) at the document level instead of the block level?

This PR makes it opt-in through the block styles. But I agree, I would consider that our last option, because I sort of feel a responsibility for Gutenberg to provide responsive features by default, so let's make sure we have a fix in the 4.0, but let's keep thinking of ways to do this without making it opt-in.

Presumably themes which provide their own responsiveness do so with a filter that adds additional wrapping elements and classes, no? Would it be possible/feasible/not break things, if we apply a differently named filter for responsive videos?

@notnownikki
Copy link
Member

the oEmbed provider itself was the one supplying the responsive code that was then doubled by Gutenberg

Oh... wonderful :( so if providers are sometimes doing their own responsive thing, then we need to detect when that's happening and turn of the gutenberg responsive styles for that embed. I guess we could parse the html and see if any of the elements have a relative position applied...?

@gregsullivan thank you for spotting this!

@gregsullivan
Copy link
Contributor

You're welcome, @notnownikki! It occurred to me that this could be happening on three levels on the oEmbed side:

  • Inline CSS being added directly to the HTML, as shown above
  • Inline CSS being added by a JavaScript file included within the oEmbed response
  • An HTML class that is made responsive by a CSS file included within the oEmbed response

I started to wonder whether it might make sense to use JavaScript to check for and remove responsive styling on elements descending from .wp-block-embed__wrapper, but I'm not sure how feasible that is, and it would likely introduce flashes of empty space in the moment before the styles are removed.

Definitely not an easy problem to solve!

@notnownikki
Copy link
Member

Definitely not an easy problem to solve!

Indeed!

So the way I'd probably approach this is when we receive a new bunch of preview HTML, set state that says we're checking it for provider supplied responsiveness, and not render the preview until that check is complete. Then we parse the HTML and walk the DOM looking for provider supplied responsiveness (or maybe we render into an iframe and watch DOM changes to see what's going on... perhaps for the first iteration we just parse the HTML and walk the DOM). Once we've done that, we know if we need to disable responsiveness for that embed block or not, we set the state to say "reponsiveness checked" and then the preview would render without any flashing.

@gregsullivan
Copy link
Contributor

That makes sense to me!

Do you see an approach that would prevent issues on the front-end if oEmbed providers were to change the HTML they provide (either by starting as non-responsive and becoming responsive, or vice versa)?

@jasmussen
Copy link
Contributor

Oh golly.

then we need to detect when that's happening and turn of the gutenberg responsive styles for that embed

So — embed providers sometimes also change their icons, and given we include icons that means we also need to update them. This is one of the challenges we more or less took on our shoulders when deciding to have explicit separate embed providers on a whitelist, rather than be more generic in the approach.

In that vein, if we were able to auto-detect this stuff, that would be swell. But if that's not possible, would it make sense to simply create a list instead of embed providers that provide their own responsiveness, and omit those from the block editor treatment?

@notnownikki
Copy link
Member

Do you see an approach that would prevent issues on the front-end if oEmbed providers were to change the HTML they provide (either by starting as non-responsive and becoming responsive, or vice versa)?

We'd have to include the same detection on the front end that we do on the backend. I'm not sure we want to do that, or even if it's a good idea. We could end up with race conditions and scripts fighting each other, which in the editor is kinda... yucky not not site breaking. On the front end though, unacceptable.

In that vein, if we were able to auto-detect this stuff, that would be swell. But if that's not possible, would it make sense to simply create a list instead of embed providers that provide their own responsiveness, and omit those from the block editor treatment?

There might be some clever CSS way we can override the responsive padding from the provider and only have ours take effect? That's a question for you really @jasmussen but my feeling is that we might end up breaking more things without realising it if we go down that route.

If the providers are starting to include their own responsiveness, then yes this seems the way to go. I'm not sure why some videos get it and some dont when the videos come from the same provider - it could be some kind of A/B test going on? But if that's the way they're moving, then yeah, just register the blocks with responsive: false and let the providers do their own thing.

@jasmussen
Copy link
Contributor

There might be some clever CSS way we can override the responsive padding from the provider and only have ours take effect? That's a question for you really @jasmussen but my feeling is that we might end up breaking more things without realising it if we go down that route.

I had the same thought back when we were hoping to be able to roll this out without opt in. And I did get it to work, for one such method. But the problem is there are a number of different ways the responsiveness can be built, and unfortunately if I were to have to target them all, it would very likely touch selectors and elements it shouldn't. So I'm not so confident in that approach anymore, unfortunately.

I think you may be right, it could be an A/B test, or it could even be a thing that happens only for new embeds or something in that vein. Perhaps an interim solution is to update the help text here to explain cases where you might want to turn it off?

Screenshot 2019-07-11 at 15 29 20

@gregsullivan
Copy link
Contributor

One further note here: Vimeo seems to be breaking the oEmbed spec with these styles. Here's what oEmbed has to say about the html parameter in 2.3.4.2 (the video type):

The HTML required to embed a video player. The HTML should have no padding or margins. Consumers may wish to load the HTML in an off-domain iframe to avoid XSS vulnerabilities.

(Emphasis mine.)

I've asked my client for access to their Vimeo account; maybe I'll be able to get a sense of what is triggering the responsive styles on some videos and not others. I know it's not new versus old, as it happened in very recent videos as well as videos more than three years old.

@gregsullivan
Copy link
Contributor

Between Vimeo's documentation and access to my client's account, I think I have a decent handle on the cause of the variation between videos.

Vimeo's embed code documentation has a "Responsive embeds" section that says the following:

The responsive code may not play nicely with certain webpage themes and layouts. If you are having trouble with the responsive embed code, we recommend switching back to the fixed size embed code.

So if you're getting the embed code manually, responsive code is triggered by choosing between "Responsive" and "Fixed Size" in the "Share this Video" overlay.

The documentation also says that use of Vimeo Cards causes the embed always to be responsive. This isn't a problem I ran into, but I could see it being an issue for others.

Vimeo's oEmbed documentation says the default is for a non-responsive embed, but also says:

You can't override the video's default settings when the video owner's membership level is anything above Vimeo Basic. (Refer to the account_type field in the oEmbed response to determine this information.)

My client has a Vimeo Pro account. In their account, the videos set to "No preset" had the responsive styles added, whereas their custom preset sets a size of 640 × 360. They occasionally forget to select the preset, and it's those videos that had the responsive styles and corresponding empty space above them.

In conclusion: Videos hosted by Vimeo Pro accounts without fixed sizes set for their embed (and videos using Vimeo Cards) will always send responsive styles when requested via oEmbed.

I'm not sure how feasible this is, but perhaps having WP_Embed create a simplified embed code for Vimeo using the width, height and video_id properties would be the cleanest fix. Something like:

<iframe src="https://player.vimeo.com/video/${video_id}" width="${width}" height="${height}" frameborder="0" allow="autoplay; fullscreen" allowfullscreen></iframe>

@jasmussen
Copy link
Contributor

Thanks for the superb investigation, this is exceptionally helpful.

I'm not sure how feasible this is, but perhaps having WP_Embed create a simplified embed code for Vimeo using the width, height and video_id properties would be the cleanest fix.

There is perhaps a larger question around the UX for responsive embeds, given that technically any oembed provider could change their markup at will. The fact that responsive embeds in the block editor are opt-in, and you can opt out on a per embed basis, is a good start. But I'm wondering what the best next steps are.

  • A whitelist of known good responsive embeds is kind of in the vein of what you suggested with an exception for Vimeo. It does sound like it would be a whack-a-mole game to try and detect these things in a generic way.
  • Better descriptions and information in the block settings sidebar. Perhaps even consider moving the "Resize for smaller devices" button onto the selected video block itself rather then have it in the sidebar.

@gregsullivan question: does the embed look broken both in the frontend and in the editor? Or just the frontend? If it's in both places, the block toggle on the block itself might be a good UX change because you'd be able to toggle it on and off easily and see the effect instantly.

@gregsullivan
Copy link
Contributor

@jasmussen The embed looks normal in the editor for all the videos I've tested, so it's purely a front-end problem for me.

On the one hand, I agree 100% with the whack-a-mole characterization. On the other hand, I'm curious whether services implementing oEmbed would tend to see the padding change causing the problem as breaking the spec, making the problem rarer.

@gregsullivan
Copy link
Contributor

One further possibility: Is there a willingness to rewrite the CSS responsible for responsive embeds? I took a go at it and made a version that works for both styles of Vimeo output. If you think this is worth pursuing, I could test it across more embeds to ensure compatibility and then share it with you.

It requires some use of !important—let me know if that's a deal-breaker.

@jasmussen
Copy link
Contributor

Better CSS is absolutely an option, and I would suggest that !important was created for use cases like this. Depending on the solution you have in mind I can't know for sure whether it'll work, but I'd be excited to see your solution! Thanks again.

@gregsullivan
Copy link
Contributor

Sounds good! I'll update you when I have something further along to show you.

A related question: Are the embeds meant to support 9:16, 9:6 or both?

If 9:6, I think there's a typo here:

&.wp-embed-aspect-9-16 .wp-block-embed__wrapper,

If 9:16, I think this would need to be changed:

&.wp-embed-aspect-9-6 .wp-block-embed__wrapper::before {
padding-top: 66.66%; // 6 / 9 * 100
}

Based on the order, it looks like lines 66–68 should actually be:

&.wp-embed-aspect-9-16 .wp-block-embed__wrapper::before {
	padding-top: 177.77%; // 16 / 9 * 100
}

(All of the padding-top values are ascending until that one, and it would potentially make sense to support 16:9 in portrait orientation, so that's my guess as to what's meant to be happening.)

Sorry if this should be a separate issue! Let me know if you think it makes sense to do a pull request just for this.

@jasmussen
Copy link
Contributor

It's very possible there's a typo there. And yes, feel free to open a ticket if you like, you can reference this one and CC me (@jasmussen) and I'll follow it along. I'm going in vacation now, though, so I may not be able to test your code, but I'll certainly be able to ping someone who can.

@gregsullivan
Copy link
Contributor

Great, thanks! I went ahead and did a pull request for the aspect ratio typo: #16573

Enjoy your vacation!

@jessejoeyjames
Copy link

Hi @jasmussen @gregsullivan unfortunately my website seems to also have probems with white spacing.

I'm using Sydney theme along with Visual composer. Everything is up to date, but after the Gutenberg change I just can't seem to remove it.

The CSS code you provided, doesn't seem to work for me. Hope you can help me out:
https://www.jessejoeyjames.com/portfolio/

@gregsullivan
Copy link
Contributor

@jessejoeyjames The CSS above is specific to WordPress's built-in block editor. You appear to be encountering a conflict between Visual Composer and FitVids.js. You'd have to figure out which plugin or theme is including the latter to resolve the conflict.

@jessejoeyjames
Copy link

jessejoeyjames commented Jul 12, 2019

@gregsullivan Thank you for replying so quickly. I have no idea how you found that out so quickly, but I searched for Fitvids in cobination with my SydneyTheme. And the theme indeed seems to be using Fitvids.js

Am I supposed to contact Sydney theme or can i simply remove fitvids myself?

EDIT:
Okay so i fixed it by adding this code to the customise CSS part on wordpress

.vce-yt-video-player-inner::before {
padding-top: 0 !important;
}

Please let me know if this is okay, I'm assumming there should be a proper fix. I'm only a very average coder with just basic php,css,html knowledge. But I just tested this out as a combination of @jasmussen 's code and the conflicting area you pointed out.

@gregsullivan
Copy link
Contributor

@jessejoeyjames I'd suggest contacting your theme's developers and asking if there's a way to disable FitVids.js, either using a setting or a filter. Since you're going to continue using Visual Composer, it would be better to remove FitVids.js as one half of the doubled responsive embed styles. The theme's developers should be able to help, though—good luck!

@gregsullivan
Copy link
Contributor

@jasmussen @notnownikki I have another (potentially final) update on this, as my sense is that this is even more of an edge case than I initially thought.

In order to test potential CSS fixes, I added a hook to oembed_fetch_url to force Vimeo to send responsive embeds if ?responsive was added as a query string to the Vimeo URL.

In doing so, I found that the preexisting checks below successfully prevent the embed from being flagged as responsive:

// If we have a fixed aspect iframe, and it's a responsive embed block.
if ( iframe && iframe.height && iframe.width ) {
const aspectRatio = ( iframe.width / iframe.height ).toFixed( 2 );
// Given the actual aspect ratio, find the widest ratio to support it.
for ( let ratioIndex = 0; ratioIndex < ASPECT_RATIOS.length; ratioIndex++ ) {
const potentialRatio = ASPECT_RATIOS[ ratioIndex ];
if ( aspectRatio >= potentialRatio.ratio ) {
return classnames(
existingClassNames,
{
[ potentialRatio.className ]: allowResponsive,
'wp-has-aspect-ratio': allowResponsive,
}
);
}
}
}

(Vimeo's responsive embed removes the width and height from its iframe and instead gives a padding-top value to the wrapper div to create the desired aspect ratio.)

So in order for the double-responsive issue to occur in this case, the embed needs to change over time, outputting the responsive version after initially outputting the non-responsive version.

I did find that a minor revision to the embed CSS would solve the problem and work both for existing embeds and the responsive Vimeo embed, but I don't know that it warrants a pull request given that it's relatively difficult to trigger the issue. I suppose an argument in favour of the change would be that someone could add the responsive classes to a responsive Vimeo embed that initially loaded without them, and they would work without triggering double-responsiveness.

@TaisaDesigner
Copy link

Hi!
I am having this issue with Vimeo. With Youtube it seems like it doesn't add the space, but the preview shows up cropped oddly, and when I play the video, it shows at its original proportions, but into the shape that the preview had. So on my computer maybe I see a square and then a 16:9 video in. But on my mobile there is a 9:16 preview, and then a small 16:9 video in the middle.

Are there any news on this? I also reported to my theme (DIVI) the issue to see if they should do something.

I add some snapshots as example.

playing youtube video example
vimeo example
Youtube example

@flemmingss
Copy link

This problem still exist, I have an updated WP, and today 07.11.19 I have this issue on my site.
This is my forst youtube embed so i don't know how it worked before.
Anyway, removing the wp-embed-aspect-4-3 from code worked here also.

@notnownikki notnownikki removed their assignment Nov 7, 2019
@jessebw
Copy link

jessebw commented Jan 24, 2020

I had the same problem, from the Youtube Free plugin. Youtube settings > defaults > Unselecting "responsive video sizing" fixed the issue. Its looks like it adds a fluid-width-video-wrapper with a huge amount of padding when ticked. (It might conflict with some theme or plugin styles applied elsewhere.)

@robwent
Copy link

robwent commented Apr 9, 2020

Fitvids has an option to exclude certain css classes.
If you can edit the js without hacking anything (Or the theme/plugin using fitvids allows you to add options) then you can use the options to exclude Gutenberg embeds.
Mine looks like this now:

jQuery(".container").fitVids({
		ignore: '.wp-block-embed'
	});

If you remove it completely then videos embedded elsewhere may no longer be responsive.

@trauring
Copy link

trauring commented Mar 3, 2021

I'm seeing this same problem using OceanWP theme and Vimeo links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet