Image size extraction can fail on bad SSL configuration

[schlessera]

Bug Description

The images in a Core Gallery block are not sized correctly if they don't have a perfect fit for their container.

In the example below, you can see that the images in the AMP version are smaller than the non-AMP versions.

The main issue seems to be that the dimensions that get added to the <amp-img> are miscalculated. In the example screenshot, the very first image has the following size:

However, the corresponding <amp-img> element ends up with the following calculated dimensions:

When scaled, this produces an image of the dimensions 304x209.64, instead of the expected 304x243.13.

Note: The visual result is further corrupted because the code in #2793 erroneously adds the object-fit=contain style to the images, even though they should be object-fit=cover.

Expected Behaviour

AMP galleries should have the same sizing as non-AMP galleries.

Steps to reproduce

1. Import the theme unit test data
2. Visit the AMP version of the "Block: Gallery" post

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[schlessera]

I found the cause of this.

The image dimensions were set to $content-width x FALLBACK_HEIGHT because the plugin failed to fetch the images via FasterImage on my system due to an SSL certificate verification failure.

The work-around on my system right now: Disable SSL verification when fetching the images.

This is currently not filterable, so that might be a valuable filter to add: https://github.com/ampproject/amp-wp/blob/3119cb275c69ca579610a9992bb4681c62bd0b5f/includes/utils/class-amp-image-dimension-extractor.php#L237-L238

Moreover, we should consider:

• Leaving the library's defaults for sslVerifyHost & sslVerifyPeer at false
• Examining the URL and detecting local files and retrieving them from the filesystem instead
• Using a more WordPress-integrated way of fetching the files (slower, but might be a good fallback for cases where SSL verification failed?)

[westonruter]

This is closely related to #2287. We should use the same value as returned by the https_local_ssl_verify filter, both here and when doing loopback requests.

It would also be good to use filesystem for obtaining the image dimensions whenever possible. We do this already when fetching the stylesheets, but we the FasterImage library doesn't support obtaining dimensions from the filesystem.

[rsmith4321]

I was about to come on and report what I believe is the same issue, images in wordpress galleries are not always sized correctly. However I don't have any ssl issues. Sometimes they will have space above and below the image or will not fill the full width of the container. You can see an example in the very first image on this page https://www.ryansmithphotography.com/real-estate-photography-gallery/. I disabled amp on this page and the images size correctly.

[westonruter]

@rsmith4321 Do you mean that the first image has some white margins on the top and bottom?

I don't think it's the same issue here as in the case of @schlessera the issue is the images fail to get the right dimensions from the underlying image. But in the case here that doesn't seem to be the case since 1500x1000 are the dimensions of the amp-img and the filename being referenced:

<amp-img width="1500" height="1000" src="https://www.ryansmithphotography.com/wp-content/uploads/2019/06/dusk-at-the-ocean-club-2019-1500x1000.jpg" ...>

I think the actual issue is the srcset… notice the currentSrc when examining the img:

So the actual image being displayed here for me is https://www.ryansmithphotography.com/wp-content/uploads/2019/06/dusk-at-the-ocean-club-2019.jpg

This image is 2048x1120 which is a different aspect ratio than the 1500x1000 image which was actually embedded on the page. This is why the image appears with the borders.

A quick way for you to fix this is to override the default object-fit:contain style for the img, using a CSS rule like:

.gallery-item amp-img.amp-wp-enforced-sizes > img {
   object-fit: cover;
}

This will have the effect of then cropping off either side for that image.

[rsmith4321]

Thanks for the help. It happens on a lot of images, if your scroll down some images don't expand to the full width and others appear to have padding on the top and bottom. It's like the width and height inside the image tag is set to 1500x1000 no matter what the actual image aspect ratio. I have no idea why. But that does seem like it would have to be an issue outside of the amp plugin. I guess without amp the browser ignores the width and height in the img tag but with amp it doesn't? Because if amp is switched off the images display correctly. I really have no idea what is causing this issue but I've noticed it a lot. I really don't want to crop them so I will see if I can figure out the root cause unless you have any suggestions.

On Wed, Apr 29, 2020 at 9:03 PM Weston Ruter notifications@github.com wrote:

@rsmith4321 https://github.com/rsmith4321 Do you mean that the first image has some white margins on the top and bottom?

[image: image] https://user-images.githubusercontent.com/134745/80661064-69756f80-8a42-11ea-962c-cf1f51814c0e.png

I don't think it's the same issue here as in the case of @schlessera https://github.com/schlessera the issue is the images fail to get the right dimensions from the underlying image. But in the case here that doesn't seem to be the case since 1500x1000 are the dimensions of the amp-img and the filename being referenced:

<amp-img width="1500" height="1000" src="https://www.ryansmithphotography.com/wp-content/uploads/2019/06/dusk-at-the-ocean-club-2019-1500x1000.jpg" ...>

I think the actual issue is the srcset… notice the currentSrc when examining the img:

[image: image] https://user-images.githubusercontent.com/134745/80661257-ffa99580-8a42-11ea-8015-7dce1aa82b34.png

So the actual image being displayed here for me is https://www.ryansmithphotography.com/wp-content/uploads/2019/06/dusk-at-the-ocean-club-2019.jpg

This image is 2048x1120 which is a different aspect ratio than the 1500x1000 image which was actually embedded on the page. This is why the image appears with the borders.

A quick way for you to fix this is to override the default object-fit:contain style for the img, using a CSS rule like:

.gallery-item amp-img.amp-wp-enforced-sizes > img {

object-fit: cover;

}

This will have the effect of then cropping off either side for that image.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amp-wp/issues/4634#issuecomment-621551008, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H4TP6YWBFA5AMHJHSTRPDE5XANCNFSM4MT33W5Q .

[westonruter]

It's because you probably have a certain image size that is defined with cropping, and you've embedded a gallery with this image size. If you remove the cropping on that image size, that should fix it. Alternatively you could embed the gallery with the full size, but then on smaller viewports you'd get the same padding showing up.

[rsmith4321]

Actually I think I figured out what it is. I notice I don't see the issue in Safari. I have a plugin that generates webp images, I think there were a few images that I had changed the aspect ratio of the jpeg, but the webp wasn't regenerated. So wordpress saw the jpeg dimensions but was serving the old webp image to Chrome. That makes sense to me now! Thanks for the help.

[westonruter]

Aside: The cause may also be that the Gallery block doesn't add width or height attributes for the images it outputs. This defers the browser to set the element's size until it picks the image from the srcset. This isn't allowed in AMP, however, as all elements need to be sized up-front. So if you have a srcset that is populated with images that have varying aspect ratios, then the result for images that have object-fit:contain is for there to be padding present.

[deverman]

Hello is this the same problem I was experiencing in this screen shot? See below the tiny less than postage stamp images. I installed this plugin as part of news pack. When I changed the theme back to my previous theme I still have the same problem. After disabling this plugin the problem went away. I am currently using https but on a very very slow VPS so I'm wondering if there was some connection issue to get the size because there were some images that did display ok. Please confirm here is the link to the page in my screen shot if you need to check the source: https://www.nowshenzhen.com/about/our-team/

[westonruter]

@deverman Please capture the HTML page output for the AMP version and put it into a Gist or on Glitch so we can see what is being generated.