Images unexpectedly expand to fill containers

[kienstra]

As @westonruter pointed out, images can sometimes appear horizontally stretched.

This issue sometimes appears when images have less width than their container.

Appears on

• Paired Mode child themes of Twenty Ten, Twenty Fifteen, Twenty Sixteen, and Twenty Seventeen
• Native AMP child theme of Twenty Fifteen
• Legacy Templating (screenshot)

Steps To Reproduce

1. Checkout the 0.7 branch of this plugin
2. Activate a Paired Mode child theme of Twenty Fifteen (update: or simply use any core theme, not a child theme)
3. Create a new post
4. Add an image that's less wide than 600px, like this image (update: and center-align the image)
5. Save the post, and visit the front-end on a desktop display (appending /amp or ?amp=1 to the URL)
   Expected: it appears at its natural dimensions (300x232 for the example above)
   Actual: it appears stretched (see image above)

This may be related to #937.

[kienstra]

Initial Findings

So far, this looks to be related to the aligncenter class.

That has the style rule display: block;

.aligncenter {
    clear: both;
    display: block;
    margin-left: auto;
    margin-right: auto;
}

And that seems to override the rule that AMP has for layout="intrinsic"

.i-amphtml-layout-intrinsic {
    display: inline-block;
    ...
}

[postphotos]

Hi @kienstra, Thanks for hopping in and thanks @westonruter for reporting! I added a user story/AC here.

[westonruter]

The margin on the .amp-wp-enforced-sizes also seems to be a problem, since it is blowing away the auto margins:

The issue can be seen in 1.0-alpha as well on https://amp-make-xwp.pantheonsite.io/2016/07/24/dependently-contextual-customizer-controls/

[kienstra]

Simple Way To See Bug

Another simple way to see this bug is to add the class aligncenter to any <amp-img> on a "legacy template" post using version 0.7, like this.

[kienstra]

Legacy Templating: Images Used To Be Centered

A related issue is that in the previous version, 0.6 (see branch 0.6), the "legacy template" images were all centered unless they had a class of alignleft or alignright.

We should probably try to keep it that way in version 0.7.

Background
This is a "legacy template" in version 0.6:

https://ryankienstra.wordpress.com/2018/05/01/test-page/amp/

This style rule used to center all of the images without the classes alignleft or alignright. regardless of whether they were aligned left, right, or center in the editor:

.amp-wp-article-content amp-img {
    margin: 0 auto;
}

In 0.7, the <amp-img> has a layout of intrinsic, which seems to be a much better layout. But it seems to have a side effect of overriding the style rule above with:

.i-amphtml-layout-intrinsic {
    display: inline-block;
    ...
}

As I understand it, applying left and right margins of auto doesn't center elements that have a display of inline-block.

For example, this post uses the plugin version 0.7, and the images aren't centered.

However, this could have been caused by my removal of the sizes attribute in #937.

We could output different layouts for "Legacy Templating," by using current_theme_supports( 'amp' ). Here's one place where this would be possible.

[westonruter]

But it seems to have a side effect of overriding the style rule

And it is illegal to add our own style rules that target i-amphtml-* classes.

[delawski]

As I've looked into the issue, I've noticed that there's no problem with images having captions:

It turns out that when you add a wrapper element that has a defined width and the aligncenter class applied, the inner amp-img with layout="intrinsic" is centered properly and not stretched.

We've investigated the issue with @kienstra and came up to such solution:

1. Remove aligncenter class from the amp-img.
2. Wrap the amp-img with a block element (e.g. figure) having the width defined and alingcenter class applied, i.e.:

<figure class="aligncenter" style="width: ...px; max-width: 100%"><amp-img ...></amp-img></div>

Thanks to that the image should not get stretched horizontally, it should be centered and in case it's wider than the container, it won't overflow.

[westonruter]

Fixed by #1109. Thanks @delawski!

[mustafauysal]

Hi,

Unfortunately, inline styles are not supported by AMP. Could we reconsider this?

[westonruter]

@mustafauysal You're right that inline styles aren't allowed by AMP, but the AMP plugin does support them by converting them into CSS style rules via selectors with generated CSS class names.

[mustafauysal]

@westonruter Oh! I see now. We are using custom sanitizers, that's why I've looked to the wrong place at first. Thanks for the clarification.

[westonruter]

This is being revisited in #2036. Please test.