Size Change: +21 B (0%)

Total Size: 1.1 MB

_{compressed-size-action}

Perhaps we could do something similar to what we did in the twentytwentyone theme?
We basically added a script to get the width & height of the video, and then set the height using % units to keep the perspective correct. See https://github.com/WordPress/WordPress/blob/5.8/wp-content/themes/twentytwentyone/assets/js/responsive-embeds.js for details

But relying on a script to obtain the width/height on the frontend is not optimal for performance, right? That will result in layout shift when the script runs. I thought the point of browsers implementing a default aspect-ratio based on the width and height would eliminate the need for this, and so it's better if the width/height are added to the video element.

The AMP plugin is obtaining the width and height via wp_get_attachment_metadata(): https://github.com/ampproject/amp-wp/blob/9b483e02e00b880e48e4b943e5c07e9441904bae/includes/embeds/class-amp-core-block-handler.php#L186-L213

In any case, I'm seeing some strange browser behavior for video even when it has a width and height defined up-front, where it seems to have no effect at all. Layout shift is still happening for video but not for img: https://video-layout-shift-despite-aspect-ratio.glitch.me/

I think we could combine the 2... A PHP implementation makes more sense than a JS one as it will be more performant and will avoid layout shifts like you said.
But instead of using width & height, we could maybe add style="width: 100%; height: {x}%" to implement the aspect-ratio 🤔

My understanding is that only setting the width and height attributes will populate a default aspect-ratio. With that populated, setting width:100% (and height:auto?) should then allow the element to resize and preserve its aspect ratio without any initial layout shift. See aspect-ratio on MDN:

The reason I'm a bit hesitant with using aspect-ratio is that Safari only started supporting it in v15 released a couple of months ago... which means a large portion of users on older devices (~14% on both desktop & mobile versions) won't benefit from it.

Hi, @westonruter

Is this PR still relevant?

@Mamaduka It's still relevant, yes, but in reality the issue is larger than this because the VIDEO in a Video block is missing the width and height attributes. See #52185.

In the 4 years since I opened this PR (😊), I think the solution is to not only add height:auto but also add the width and height attributes, as well as the aspect-ratio, as can be seen in this example:

	// Set the width and height to reflect the captured intrinsic dimensions.
	$processor->set_attribute( 'width', (string) $intrinsic_dimensions['width'] );
	$processor->set_attribute( 'height', (string) $intrinsic_dimensions['height'] );
	if ( 'VIDEO' === $processor->get_tag() ) {
		// TODO: It's not clear why the aspect-ratio needs to be specified when the user agent style is already defining `aspect-ratio: auto $width / $height;`.
		// TODO: Also, the Video block has styles the VIDEO with width:100% but it lacks height:auto. Why?
		// TODO: Why does the Image block use width:content-fit?
		$style = sprintf( 'height: auto; width: 100%%; aspect-ratio: %d / %d;', $intrinsic_dimensions['width'], $intrinsic_dimensions['height'] );

		$old_style = $processor->get_attribute( 'style' );
		if ( is_string( $old_style ) ) {
			$style .= $old_style;
		}

		$processor->set_attribute( 'style', $style );
	}

As noted by the TODO comments, I didn't haven't yet have everything figured out. Nevertheless, the result is an impressive improvement in Cumulative Layout Shift:

Before:

After:

See also:

• Visual tool to scan a page and report missing width/height elements performance#10

Thanks for the update, @westonruter! It seems that we could start with #52185 and then move to other related PRs.

Closing in favor of #70293 which includes the full fix.