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

<amp-img> gets converted into <img class="i-amphtml-fill-content i-amphtml-replaced-content">​ #6032

Closed
flixwatchsupport opened this issue Apr 1, 2021 · 7 comments
Labels

Comments

@flixwatchsupport
Copy link

flixwatchsupport commented Apr 1, 2021

Bug Description

In the template, I am adding:

<amp-img alt="<?php echo get_the_title(); echo " "; the_field('copyrightyear'); ?>"
  width="320"
  height="180"
  layout="fixed"
  src="<?php echo $imgwebp; ?>">
  <amp-img alt="<?php echo get_the_title(); echo " "; the_field('copyrightyear'); ?>"
    fallback
    width="320"
    height="180"
    layout="fixed"
    src="<?php echo $img; ?>"></amp-img>
</amp-img>

On the Source Code it shows:

<amp-img alt="Hush 2016" width="320" height="180" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg.webp" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:320px;height:180px;" i-amphtml-layout="fixed">
    <amp-img alt="Hush 2016" fallback width="320" height="180" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:320px;height:180px;" i-amphtml-layout="fixed"></amp-img>
</amp-img>

But the PageSpeed Insights shows:

<img decoding="async" alt="Hush 2016" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg.webp" class="i-amphtml-fill-content i-amphtml-replaced-content">

And the loading time is very high.

Expected Behaviour

<amp-img alt="Hush 2016" width="320" height="180" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg.webp" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:320px;height:180px;" i-amphtml-layout="fixed">
    <amp-img alt="Hush 2016" fallback width="320" height="180" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:320px;height:180px;" i-amphtml-layout="fixed"></amp-img>
</amp-img>

Steps to reproduce

It's automatically getting converted into that format. I am not sure what are the steps to reproduce it other than installing the APM plugin.
Check: https://www.flixwatch.co/movies/hush/

Screenshots

Not Applicable, looks the same on the frontend.

Additional context

  • WordPress version: 5.7
  • Plugin version: 2.0.11
  • Gutenberg plugin version (if applicable):
  • AMP plugin template mode: Standard
  • PHP version: 7.3.27 (Supports 64bit values)
  • OS:
  • Browser: [e.g. chrome, safari]
  • Device: [e.g. iPhone6]
AMP mode enabled standard
AMP Reader theme legacy
Templates enabled movies, tvshows, lists, country, newonnetflix, is_singular, is_front_page, is_tax[actor], is_tax[director], is_tax[creator]
Serve all templates as AMP? false
Transient caching for stylesheets disabled n/a
Threshold for monitoring stylesheet caching 5000 transients per day
Sampling range for monitoring stylesheet caching 14 days
Number of stylesheet transient cache entries 0
Calculated time series for monitoring the stylesheet caching  
libxml Version 2.9.7

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

@flixwatchsupport flixwatchsupport added the Bug Something isn't working label Apr 1, 2021
@westonruter
Copy link
Member

The img that PageSpeed Insights shows is added to the page by the AMP runtime. So that is expected. I think the problem you're noticing is that the image is getting lazy-loaded when it should be getting prerendered. I suggest you try upgrading to 2.1 Beta 1 which includes hero image prerendering:

To reduce the Largest Contentful Paint (LCP) metric, the AMP Optimizer now prerenders hero image(s). It will automatically detect the first image in the page to prerender, but you can manually designate which images should be prerendered by adding the data-hero attribute. For core themes there is a transformer which is tuned to automatically identify hero candidates among the header image, custom logo, featured image, initial cover block, and initial image block. This transformer will likely be enabled by default in the future once we've tested with more themes. #5055, #5350, #5824, #5923, #5934

With that, I suggest that you add the data-hero attribute to the site logo image and then to that first content image you identified.

@westonruter
Copy link
Member

You also may not need to manually add data-hero to your images since the plugin will add them to the first hero candidate image(s). But if you add data-hero then you will have explicit control.

@flixwatchsupport
Copy link
Author

flixwatchsupport commented Apr 1, 2021

Understood. Thanks @westonruter . I will implement it and let you know the outcome.

Edit: Do I need to add data-hero attribute to the fallback image as well?

@flixwatchsupport
Copy link
Author

flixwatchsupport commented Apr 2, 2021

Now I am getting this:

<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" alt="Hush 2016" src="https://www.flixwatch.co/wp-content/uploads/80091879-320x180.jpg.webp" data-hero="">

in PageSpeed Insights but the LCP hasn't changed as such.

For the logo:

<amp-img class="custom-logo i-amphtml-layout-fixed i-amphtml-layout-size-defined" alt="FlixWatch Logo" width="214" height="50" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/Flixwatch-214x50.jpg.webp" style="width:214px;height:50px" i-amphtml-layout="fixed" i-amphtml-ssr data-hero>
  <amp-img alt="FlixWatch Logo" fallback width="214" height="50" layout="fixed" src="https://www.flixwatch.co/wp-content/uploads/Flixwatch-214x50.jpg"></amp-img>
    <img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" alt="FlixWatch Logo" src="https://www.flixwatch.co/wp-content/uploads/Flixwatch-214x50.jpg.webp">
  </amp-img>

Also there are two warnings in the console:
The resource https://d-30907998692382382773.ampproject.net/2103240330001/nameframe.html was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.
The resource https://af67b97f3e807f0fe60da07cf6928f0c.safeframe.googlesyndication.com/safeframe/1-0-38/html/container.html was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.

@westonruter
Copy link
Member

I don't immediately see how it can be further optimized.

I do see that your logo seems to be a WebP which was derived from a heavily-compressed JPEG. The compression artifacts are clearly visible:

image

I suggest creating a PNG for the custom logo instead. You may also want to look into using a vector graphic instead, and then inlining it as SVG. We've done this on https://amp-wp.org/

Also there are two warnings in the console:
The resource https://d-30907998692382382773.ampproject.net/2103240330001/nameframe.html was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.
The resource https://af67b97f3e807f0fe60da07cf6928f0c.safeframe.googlesyndication.com/safeframe/1-0-38/html/container.html was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally.

These seem to be related to ads on the page, namely AdSense. It's not something the AMP plugin is causing. You can ignore these errors per ampproject/amphtml#10431 (comment).

@westonruter
Copy link
Member

I'm also noticing there is a layout shift caused by amp-accordion, which appears actually to be a bug with AMP itself (not in the AMP plugin). Nevertheless, using amp-accordion is not the best way to implement a menu. You can find an example of how to implement a similar menu when the Twenty Sixteen theme is active. You'll see that it uses amp-bind to toggle the state.

@flixwatchsupport
Copy link
Author

Ok. Will change the logo and change the amp-accordion to amp-bind.

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

No branches or pull requests

2 participants