Images on attachment page appear stretched #1237

westonruter · 2018-07-02T19:29:00Z

In Twenty Seventeen at least, an image on the attachment template looks stretched:

<amp-img width="300" height="300" src="https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-300x300.png" class="attachment-medium size-medium amp-wp-enforced-sizes" alt="" srcset="https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-300x300.png 300w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-150x150.png 150w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-100x100.png 100w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-270x270.png 270w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-192x192.png 192w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-180x180.png 180w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-32x32.png 32w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp.png 512w" sizes="100vw" layout="intrinsic"></amp-img>

Non-AMP version:

<img width="300" height="300" src="https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-300x300.png" class="attachment-medium size-medium" alt="" srcset="https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-300x300.png 300w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-150x150.png 150w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-100x100.png 100w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-270x270.png 270w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-192x192.png 192w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-180x180.png 180w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp-32x32.png 32w, https://src.wordpress-develop.test/wp-content/uploads/2018/05/cropped-wordpress-amp.png 512w" sizes="100vw" />

kienstra · 2018-07-24T02:18:03Z

Seems Related To Inline Styling
But That Might Be Expected

This looks to only exist when using the sizes attribute, like in Twenty Seventeen.

It looks like the <amp-img> adds an inline style of the same width as the sizes attribute:

https://paired-ampconfdemo.pantheonsite.io/image-alignment-150x150/?amp

<amp-img ... sizes="100vw" layout="intrinsic" style="width: 100vw;">

In the example above, the sizes and style="width: values are both 100vw. When I tried blocking the sizes via the plugin's sanitizer, this issue didn't exist. Though I'm not suggesting that as a solution.

Still, maybe the inline styling of width: 100vw; is expected.

hellofromtonya · 2018-07-24T22:26:35Z

Bringing @kienstra and my conversation here.

The inline width styling is applied by AMP (not our plugin) from the sizes attribute. WordPress provides both the srcset and sizes while exposing a filter 'wp_calculate_image_sizes' for theme/plugin developers to customize the sizes.

Here's what AMP Docs say about the attributes:

srcset
Same as srcset attribute on the img tag. For browsers that do not support srcset, <amp-img> will default to using src. If only srcset and no src is provided, the first url in the srcset will be selected.
sizes
Same as sizes attribute on the img tag.

Reading deeper into the AMP's docs about sizes:

sizes
You can also use the sizes attribute along with srcset. The sizes attribute describes how to calculate the element size based on any media expression. Based on the element’s calculated size, the user agent selects the most relative source supplied by the srcset attribute.

The sizes attribute defines the element’s width to be 50% the size of the viewport when the viewport is 650px or more. For example, if the viewport is 800px, the element’s width is set to 400px. The browser then selects the srcset resource relative to 400px, assuming the device pixel ratio is 1, which in this instance is hummingbird-narrow.jpg (320px).

When sizes attribute is specified along with width and height, layout defaults to responsive.

The width of the image then is set by the value in the sizes attribute as @kienstra notes above. AMP uses it to add the inline width style.

Compare AMP to non-AMP versions

Let's compare the difference between AMP and non-AMP attachments to see if we can find the solution:

Here is the non-AMP HTML:

<p class="attachment">
    <a href="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg">
        <img width="150" height="150" src="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg" class="attachment-medium size-medium" alt="Image 150x150" srcset="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg 150w, https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150-100x100.jpg 100w" sizes="100vw">
    </a>
</p>

Here is the AMP version of that attachment:

<p class="attachment">
    <a href="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg">
        <amp-img width="150" height="150" src="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg" class="attachment-medium size-medium amp-wp-enforced-sizes i-amphtml-element i-amphtml-layout-intrinsic i-amphtml-layout-size-defined i-amphtml-layout" alt="Image 150x150" srcset="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150.jpg 150w, https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-150x150-100x100.jpg 100w" sizes="100vw" layout="intrinsic" style="width: 100vw;">..</amp-img>
    </a>
</p>

The differences include:

<amp-img>
Extra class attributes that are added for <amp-img>
The inline style style="width: 100vw;" that AMP adds

Let's look at the CSS rules added by those extra class attributes:

element.style {
    width: 100vw;
}

.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;
}

.i-amphtml-layout-size-defined {
    overflow: hidden!important;
}

.i-amphtml-layout-intrinsic {
    display: inline-block;
    position: relative;
    max-width: 100%;
}

Possible Solutions

@kienstra notes some possible solutions above.

Option A: Remove the inline width style.

Removing the inline width style also requires us to set the display to inline-block. While this can work, we'll get into timing issues between when AMP applies the inline style and then we remove it. It could cause flashes on the screen.

Option B: Target the width of the parent container.

AMP's Tips & Tricks section suggests setting the parent container's max-width. Attachments use p.attachment. We could target that and set a max-width equal to the size of the image, i.e. grab the width attribute of the image and use it.

This possible solution would ensure the image does not stretch.

westonruter · 2018-07-24T23:47:08Z

@kienstra @hellofromtonya I'm sure you've seen this already, but just to make clear that the source of the problem is Twenty Seventeen's wp_get_attachment_image_attributes filter:

https://github.com/WordPress/wordpress-develop/blob/ddc8f803c6e99118998191fd2ea24124feb53659/src/wp-content/themes/twentyseventeen/functions.php#L534-L554

That being the case, the solution chosen could perhaps be made part of the AMP_Core_Theme_Sanitizer as a specific fix for Twenty Seventeen.

It could be that the issue here is a fundamental incompatibility between img and amp-img. In particular, if you set object-fit:contain on img it works, whereas it does not work with amp-img. This is the same problem we faced with getting the header image to work in Twenty Seventeen (see #1074), and the solution for that was to add CSS that targets the img inside of the amp-img:

https://github.com/Automattic/amp-wp/blob/e3d036c2db60b215636a2b665414b0e599e9ad97/includes/sanitizers/class-amp-core-theme-sanitizer.php#L498-L514

So a similar approach may be needed here. In fact, the existing add_twentyseventeen_masthead_styles method there could be renamed add_twentyseventeen_styles and this attachment-specific style rule could be put there potentially.

kienstra · 2018-07-25T02:05:53Z

Hi @westonruter,
Thanks, good point that this is coming from the wp_get_attachment_image_attributes filter.

Could we wait a little on this issue, to see what happens with amphtml/17053? There's discussion there about possibly changing the <amp-img> behavior for sizes.

westonruter · 2018-07-25T02:14:58Z

Excellent. I'm glad you raised that.

…hment page. Per issue #1237, the Twenty Seventeen theme is adding `$attr['sizes'] = '100vw';` which stretches images on the attachment page when in AMP mode. The attachment image's width This commit filters 'wp_get_attachment_image_attributes' to change the layout to responsive and sizes to the image sizes per `wp_get_attachment_image_sizes()`. AMP uses these attributes to render the <amp-img>.

…hment page. Per issue #1237, the Twenty Seventeen theme is adding `$attr['sizes'] = '100vw';` which stretches images on the attachment page when in AMP mode. This commit filters 'wp_get_attachment_image_attributes' to change the layout to responsive and sizes to the image sizes per `wp_get_attachment_image_sizes()`. AMP uses these attributes to render the <amp-img>.

hellofromtonya · 2018-08-06T22:27:16Z

@westonruter and @kienstra Upon further investigation, we can remedy the Twenty Seventeen theme's image stretching issue by filtering the attachment image's attributes and changing the following:

Set the layout attribute to responsive.
Set the sizes attribute to the returned sizes from wp_get_attachment_image_sizes().

IMO these attribute fixes are not a bandaid but rather a fix for how the theme is setting the sizes for attachment pages.

hellofromtonya · 2018-08-06T22:47:39Z

For reference, PR #1321 fixes this specific issue. However, since we have an open issue with the AMPHTML project, let's keep this issue open until that one is resolved. Once resolved, we can come back and validate this issue to ensure there are no regression problems.

kienstra · 2018-09-24T16:46:44Z

Moving To 'Ready For Merging'

It looks like the Issue for the AMP project probably won't be fixed by this plugin's stable 1.0 release.

So I'm moving this to 'Ready For Merging' if that's alright, as this already has a merged PR that fixes the issue.

kienstra · 2018-12-04T17:57:27Z

Moving Out Of Milestone

As this is only tracking ampproject/amphtml#17053, I'm moving this out of the v1.0 milestone. There's no action needed, other than remove this issue's workaround if that issue is ever resolved.

westonruter · 2019-03-29T04:04:44Z

This is being revisited in #2036. Please test.

westonruter added the Bug Something isn't working label Jul 2, 2018

westonruter added this to the v1.0 milestone Jul 2, 2018

postphotos added the Release label Jul 9, 2018

kienstra self-assigned this Jul 23, 2018

kienstra mentioned this issue Jul 24, 2018

<amp-img> sizes attribute behavior seems different from non-AMP <img> ampproject/amphtml#17053

Closed

westonruter mentioned this issue Jul 24, 2018

Revisit aligned images with inline width larger than max getting displayed off-center #1086

Closed

2 tasks

westonruter mentioned this issue Aug 1, 2018

amp-img receives an unexpected inline style of width:100vw #1305

Closed

hellofromtonya mentioned this issue Aug 6, 2018

Filter the attachment image attributes for the Twenty Seventeen theme. #1321

Merged

kienstra removed this from the v1.0 milestone Dec 4, 2018

kienstra closed this as completed Dec 4, 2018

westonruter mentioned this issue Mar 7, 2019

Try: Experiment with amp-img-auto-sizes #1939

Closed

westonruter mentioned this issue Mar 29, 2019

Definitively fix issues with images in AMP not visually matching non-AMP version #2036

Merged

This was referenced Apr 20, 2020

Discontinue removing sizes attribute when converting img to amp-img #4606

Closed

Prevent distortion of images with a 'sizes' attribute #4622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Images on attachment page appear stretched #1237

Images on attachment page appear stretched #1237

westonruter commented Jul 2, 2018

kienstra commented Jul 24, 2018

hellofromtonya commented Jul 24, 2018 •

edited

Loading

westonruter commented Jul 24, 2018

kienstra commented Jul 25, 2018 •

edited

Loading

westonruter commented Jul 25, 2018

hellofromtonya commented Aug 6, 2018

hellofromtonya commented Aug 6, 2018

kienstra commented Sep 24, 2018

kienstra commented Dec 4, 2018

westonruter commented Mar 29, 2019

Images on attachment page appear stretched #1237

Images on attachment page appear stretched #1237

Comments

westonruter commented Jul 2, 2018

kienstra commented Jul 24, 2018

hellofromtonya commented Jul 24, 2018 • edited Loading

Compare AMP to non-AMP versions

Possible Solutions

westonruter commented Jul 24, 2018

kienstra commented Jul 25, 2018 • edited Loading

westonruter commented Jul 25, 2018

hellofromtonya commented Aug 6, 2018

hellofromtonya commented Aug 6, 2018

kienstra commented Sep 24, 2018

kienstra commented Dec 4, 2018

westonruter commented Mar 29, 2019

hellofromtonya commented Jul 24, 2018 •

edited

Loading

kienstra commented Jul 25, 2018 •

edited

Loading