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

Lighthouse report on AMP page - Offscreen Images #5494

Closed
antoinebisch opened this issue Jun 13, 2018 · 4 comments · Fixed by #5504
Closed

Lighthouse report on AMP page - Offscreen Images #5494

antoinebisch opened this issue Jun 13, 2018 · 4 comments · Fixed by #5504
Labels

Comments

@antoinebisch
Copy link

Bug report

Provide the steps to reproduce

  1. Run LH on https://storage.googleapis.com/cs-emea-assets/antoine/Safestore/AMP%20POC/index.html

What is the current behavior?

The report highlighted "Offscreen images" as the top opportunity.

What is the expected behavior?

My tested URL being an AMP page, lazy load of background images is a feature coming out of the box and is a major USP of AMP, which Lighthouse reports against here. Therefore this could send the wrong signals to users who have built similar AMP pages and would not expect this feedback from a performance analysis.

Comments from Malte Ubl (AMP PM) on this issue:

AMP's algorithm is:
Prioritize resources (like images) based on distance to viewport
Eventually (when there is nothing better to do) download resources within 3 viewports of the current viewport.
This was chosen as a trade-off between bandwidth usage and avoiding users having to wait for resources to download as they scroll.
Basically, it seems like lighthouse would prefer a more conservative strategy (less bandwidth used, more waiting for users). I'd appreciate filing a lighthouse bug for them to evaluate whether they shouldn't accept AMP's strategy. My suspicion is that they don't use a large enough doc to notice that AMP stops are 3 viewports.

Environment Information

  • Affected Channels:
  • Lighthouse version: Latest from Chrome DevTools
  • Node.js version: N/A
  • Operating System: Mac OS
@patrickhulce
Copy link
Collaborator

Thanks for reporting @antoinebisch! This is indeed a bug in our performance simulation.

Chrome sometimes seems to think the images are being regularly discovered by parsing the main document instead of being added via script. In this case, we have no way of knowing that they weren't already lazily loaded. We might need to investigate a little more.

@paulirish
Copy link
Member

will be fixed by #5504

@aaron-renovationbrands
Copy link

4.0.0-alpha.1 also returns for amp-carousel : Best Practices - Consider marking your touch and wheel event listeners as passive to improve your page's scroll performance.

URL Location
/v0/amp-carousel-0.1.js(cdn.ampproject.org) line: 28

Does this result cover this ticket as well?

@patrickhulce
Copy link
Collaborator

@aaron-renovationbrands nope that would be different. In advance though, I'd say we'd need clarification from the AMP if they really need to have the listener be non-passive. That violation comes directly from Chrome.

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

Successfully merging a pull request may close this issue.

4 participants