incubator-pagespeed-mod icon indicating copy to clipboard operation
incubator-pagespeed-mod copied to clipboard

Image dimensions are erroneously supplied to amp-img>img elements

Open westonruter opened this issue 4 years ago • 2 comments

A user of the AMP plugin for WordPress reported an issue with images on an AMP page where the server-side rendered img in the amp-img light shadow DOM was getting a width and height attribute supplied:

<amp-img width="150" height="150" src="https://www.example.com/150x150.jpeg" class="attachment-thumbnail size-thumbnail wp-post-image amp-wp-enforced-sizes i-amphtml-layout-intrinsic i-amphtml-layout-size-defined i-amphtml-element i-amphtml-built i-amphtml-layout" alt="" data-hero-candidate="" layout="intrinsic" disable-inline-width="" i-amphtml-layout="intrinsic" data-hero="" i-amphtml-ssr="" i-amphtml-auto-lightbox-visited="">
    <i-amphtml-sizer class="i-amphtml-sizer" slot="i-amphtml-svc"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;base64,PHN2ZyBoZWlnaHQ9IjE1MCIgd2lkdGg9IjE1MCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB2ZXJzaW9uPSIxLjEiLz4="></i-amphtml-sizer>
    <img width="150" height="150" class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" loading="lazy" alt="" src="https://www.example.com/150x150.jpeg" data-hero-candidate="" data-hero="">
</amp-img>

The img inside of an amp-img gets position:absolute and stretches to fill its container element, so the width and height are irrelevant. Since the attributes are not expected, they cause AMP validation errors and the AMP base stylesheet inadvertently gives the img a position:relative styling, which causes the image to be hidden by being positioned outside of the parent's overflow.

All this to say, when an img is the child of amp-img it should be skipped from having its width and height supplied.

westonruter avatar May 12 '21 06:05 westonruter

Hi @westonruter As far as I know only the filters resize_images and resize_rendered_image_dimensions can be related to this issue. Pagespeed by default has a rewrite level core that enables the resize_images.

There are some ways to disable these filters:

pagespeed DisableFilters filtera,filterb; (nginx)
ModPagespeedDisableFilters filtera,filterb (apache)

But these disables the filters for all request, not only for amp pages. Maybe some type of logic can disable these filters only for amp request. If I remember the AMP Plugin put amp as an arg to the url, so some like this may work:

if ($args = amp) {
             set $disable_filters "resize_images,resize_rendered_image_dimensions";
      }

 pagespeed DisableFilters "$disable_filters";

That´s nginx syntax, I don´t know if in apache can be done.

Lofesa avatar May 12 '21 18:05 Lofesa

The easiest approach may be to disable the insert_image_dimensions https://www.modpagespeed.com/doc/reference-image-optimize#insert_image_dimensions filter:

ModPagespeedDisableFilters insert_image_dimensions

you can also do this in an amp-specific way using fancier config-file constructs (the syntax of which I have now forgotten :), but it's easiest if AMP pages are under an "/amp/" subdirectory in the URL path.

-Josh

On Wed, May 12, 2021 at 2:49 PM Lofesa @.***> wrote:

Hi @westonruter https://github.com/westonruter As far as I know only the filters resize_images and resize_rendered_image_dimensions can be related to this issue. Pagespeed by default has a rewrite level core https://www.modpagespeed.com/doc/config_filters#level that enables the resize_images.

There are some ways to disable these filters:

pagespeed DisableFilters filtera,filterb; (nginx)

ModPagespeedDisableFilters filtera,filterb (apache)

But these disables the filters for all request, not only for amp pages. Maybe some type of logic can disable these filters only for amp request. If I remember the AMP Plugin put amp as an arg to the url, so some like this may work:

if ($args = amp) {

         set $disable_filters "resize_images,resize_rendered_image_dimensions";

  }

pagespeed DisableFilters "$disable_filters";

That´s nginx syntax, I don´t know if in apache can be done.

pagespeed UrlValuedAttribute amp-img src image

in their config

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apache/incubator-pagespeed-mod/issues/2067#issuecomment-840017441, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAO2IPKQIGGIEPFNVHCGCADTNLESRANCNFSM44X4ILYA .

jmarantz avatar May 12 '21 20:05 jmarantz