Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wide images are overflowing with <amp-img> but not with non-AMP <img> #17084

Closed
hellofromtonya opened this issue Jul 25, 2018 · 6 comments
Closed

Comments

@hellofromtonya
Copy link

What's the issue?

Wide <amp-img> images overflow the content area when its parent element has a width of fit-content. Behavior is consistent with fixed, intrinsic, and responsive layouts.

The same images in non-AMP <img> do not overflow the content area.

How do we reproduce the issue?

I've set up a demonstration of the AMP behavior compared to the same non-AMP images

  1. Set the primary content area's width to less than target image's width.
  2. Add a <figure> parent around the <amp-img> and applying fit-content as its width.
<figure class="wp-block-image">

where the CSS ruleset is:

.wp-block-image {
    width: -webkit-fit-content;
    width: -moz-fit-content;
    width: fit-content;
}
  1. Set the image's src, width, height, srcset, and sizes attributes to a size wider than the content area:

For example, here is the <img> HTML:

<figure class="wp-block-image">
    <img src="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg" alt="Image Alignment 580x300" width="580" height="300" class="alignleft size-full wp-image-423" data-amp-layout="intrinsic" srcset="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg 580w, https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300-300x155.jpg 300w" sizes="(max-width: 580px) 100vw, 580px">
</figure>

Here is the AMP HTML:

<figure class="wp-block-image">
    <amp-img src="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg" alt="Image Alignment 580x300" width="580" height="300" class="alignleft size-full wp-image-423 amp-wp-enforced-sizes i-amphtml-element i-amphtml-layout-intrinsic i-amphtml-layout-size-defined i-amphtml-layout" layout="intrinsic" srcset="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg 580w, https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300-300x155.jpg 300w" sizes="(max-width: 580px) 100vw, 580px" style="width: 580px;">
        <i-amphtml-sizer class="i-amphtml-sizer">
            <img class="i-amphtml-intrinsic-sizer" src="data:image/svg+xml;charset=utf-8,&lt;svg height=&quot;300px&quot; width=&quot;580px&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/&gt;">
        </i-amphtml-sizer>
        <img decoding="async" alt="Image Alignment 580x300" srcset="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg 580w, https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300-300x155.jpg 300w" src="https://paired-ampconfdemo.pantheonsite.io/wp-content/uploads/2018/07/image-alignment-580x300.jpg" sizes="(max-width: 580px) 100vw, 580px" class="i-amphtml-fill-content i-amphtml-replaced-content">
    </amp-img>
</figure>
  1. Expected: The image should proportionally scale to fit within the content area as demonstrated here

screen shot 2018-07-25 at 11 16 11 am

Please note, the content space is highlighted by the striped background to provide a quick glance of the image's relationship to the space.

  1. Actual: The image overflows the content area as demonstrated here:

screen shot 2018-07-25 at 11 15 33 am

This issue may be related to Issue #17053.

What browsers are affected?

Chrome - Version 67.0.3396.99 (Official Build) (64-bit)
Firefox Quantum - 61.0.1 (64-bit)
Firefox Developer Edition - 62.0b11 (64-bit)
Safari Version 11.1.2 (13605.3.8)

MacOS Version 10.13.5
MacOS Version 10.13.6
Did not test on Windows.

Which AMP version is affected?

1531800879103

We noticed this issue on April 19, 2018 per amp-wp/1086.

@dreamofabear
Copy link

/to @aghassemi

@aghassemi aghassemi added this to the Pending Triage milestone Jul 30, 2018
@aghassemi
Copy link
Contributor

related to #17053
NOT a regression of srcset change. (I don't think #17053 is either)
/cc @cathyxz FYI for now. will create a mini project for this and related issues to find the best solution and collaborate with @hellofromtonya and WP to ensure the solution is correct based on expectations.

@ampprojectbot
Copy link
Member

This issue seems to be in Pending Triage for awhile. @aghassemi Please triage this to an appropriate milestone.

@cathyxz
Copy link
Contributor

cathyxz commented Apr 29, 2019

I think we triaged this along with #17053 to be due to the sizes attribute having unexpected side effects. I've tried to enable sizes without side-effects by autogenerating sizes in #20968, and am now updating the <amp-img> documentation to better reflect our sizes behavior in AMP. I think we should be able to close this issue with #22053.

@cathyxz cathyxz self-assigned this Apr 29, 2019
@westonruter
Copy link
Member

Yes, I think this is resolved with amp-img-auto-sizes.

@hellofromtonya
Copy link
Author

Yes, it is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants