Image Compare Block: AMP Errors

[kraftbj]

Steps to reproduce the issue

1. Add an image compare block when using AMP in Reader mode (e.g. /amp/ URLs used with a different layout).
2. Validate via Google.

What I expected

No errors.

What happened instead

"AMP HTML Tag has an invalid layout specified by its attributes."

<amp-image-slider layout="responsive"> <amp-img id="62096" src="https://i1.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606170005430_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img> <amp-img id="62097" src="https://i0.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606173109566_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img></amp-image-slider>

From https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md, it mentions for fill:
The element takes the space available to it—both width and height. In other words, the layout and size of a fill element matches its parent. For an element to fill its parent container, specify the "fill" layout, and ensure the parent container specifies position:relative or position:absolute.

I don't see position:relative or position:absolute.

The parent element of the image slider does have layout=responsive, but from the docs:

Note: Elements with "layout=responsive" have no intrinsic size. The size of the element is determined from its container element. To ensure your AMP element displays, you must specify a width and height for the containing element. Do not specify "display:table" on the containing element as this overrides the display of the AMP element, rendering the AMP element invisible.

I don't see the containing element with a width/height when looking at the source of https://kraft.blog/2020/06/covid-cut/amp/

Screenshots

HTML:

<div class="e-content">
<p>...</p>
<p>...</p>

<amp-image-slider layout="responsive"> <amp-img id="62096" src="https://i1.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606170005430_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img> <amp-img id="62097" src="https://i0.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606173109566_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img></amp-image-slider>

<amp-image-slider layout="responsive"> <amp-img id="62098" src="https://i2.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606170021564_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img> <amp-img id="62099" src="https://i0.wp.com/kraft.blog/uploads/00100sPORTRAIT_00100_BURST20200606173301092_COVER-scaled.jpg?fit=1920%2C2560&amp;ssl=1" alt="" layout="fill"></amp-img></amp-image-slider>


<p>...</p>
</div>

[westonruter]

PR to fix: #17253

[westonruter]

In looking at \Automattic\Jetpack\Extensions\ImageCompare\render_amp(), the issue seems to be that it is supplying no width or height when there is no width or height supplied:

jetpack/extensions/blocks/image-compare/image-compare.php

Lines 61 to 63 in 3a551cb

	'<amp-image-slider layout="responsive"%1$s%2$s> <amp-img id="%3$d" src="%4$s" alt="%5$s" layout="fill"></amp-img> <amp-img id="%6$d" src="%7$s" alt="%8$s" layout="fill"></amp-img></amp-image-slider>',
	! empty( $img_before['width'] ) ? ' width="' . absint( $img_before['width'] ) . '"' : '',
	! empty( $img_before['height'] ) ? ' height="' . absint( $img_before['height'] ) . '"' : '',

This is not valid AMP, as all elements need to have sizing. I suggest a fallback width/height of 1. Since since the layout is responsive, this will have the effect of making the element sized to fit its container as a square (1:1 aspect ratio).