Prevent distortion of images with a 'sizes' attribute

[pierlon]

Summary

This PR re-adds the disabled sizes attribute to the amp-img component, and disables the inline width being added by adding the disable-inline-width attribute. This should prevent any distortion of the image when viewed with an AMP page.

I'm using the post content @westonruter posted in #2036 to test some of the possible ways an image can be used on a page. This verifies that there are no apparent regressions:

Desktop

Non-AMP	AMP	AMP without fix

Mobile

Non-AMP	AMP	AMP without fix

Fixes #4606.

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[schlessera]

All-in with the constants... :)

[schlessera]

You should use Attribute here.

Suggested change

	if ( isset( $new_attributes['sizes'] ) ) {
	$new_attributes['disable-inline-width'] = '';
	if ( isset( $new_attributes[ Attribute::SIZES ] ) ) {
	$new_attributes[ Attribute::DISABLE_INLINE_WIDTH'] = '';

Note: You'll have to add the DISABLE_INLINE_WIDTH constant to the Layout class as well.

[pierlon]

Constants everywhere 😄!

[pierlon]

Relevant change ⬆️.

[westonruter]

As expected, on a page with images, the changes in this PR result in the i-amphtml-no-boilerplate attribute being removed, with this comment added to the end of the head:

AMP optimization could not be completed due to the following:
 - CannotRemoveBoilerplate: Cannot remove boilerplate as either heights, media or sizes attribute is set: <amp-img noloading="" width="1600" height="1200" src="https://wordpressdev.lndo.site/content/uploads/2011/07/dsc09114.j…>
 - CannotRemoveBoilerplate: Cannot remove boilerplate as either heights, media or sizes attribute is set: <amp-img noloading="" width="1600" height="1200" src="https://wordpressdev.lndo.site/content/uploads/2011/07/dsc09114.j…>

This is expected as this regression will then be addressed as part of #4482.

[westonruter]

I'm testing this with the issue in #1939 regarding inline images. In Twenty Nineteen, adding content like so:

The non-AMP version and the AMP version on develop look like this:

But with this PR there is a regression:

This is due to the amp-img having the disable-inline-width attribute which causes the element to get this AMP shared CSS rule applied:

.i-amphtml-layout-responsive,
[layout=responsive][width][height]:not(.i-amphtml-layout-responsive),
[width][height][sizes]:not(.i-amphtml-layout-responsive)
{
  display: block;
  position: relative;
}

If I'm not mistaken, I think this CSS may actually be not updated for to account for the disable-inline-width attribute. I think that the last selector should perhaps be changed to not apply with the disable-inline-width attribute is present:

[width][height][sizes]:not([disable-inline-width]):not(.i-amphtml-layout-responsive)

I'm checking with the AMP team: ampproject/amphtml#27083 (comment).

[westonruter]

Note this issue did not show up in testing Twenty Twenty because it (erroneously) has a display:block style added to all images.

[westonruter]

The display:block style also has the effect of centering the image on the attachment template (at least on Twenty Seventeen). See #1237.

[westonruter]

Opened a PR to fix the the issue in ampshared.css: ampproject/amphtml#28020

Constants have been applied

[westonruter]

Upstream PR merged: ampproject/amphtml#28020

[westonruter]

@schlessera anything else on this PR to receive your ✅?

[westonruter]

I guess not 😄 I'll merge and you can amend as needed in #4482.