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

SSR all amp-img tags to img (not just the hero images) #465

Open
twifkak opened this issue Aug 20, 2020 · 6 comments
Open

SSR all amp-img tags to img (not just the hero images) #465

twifkak opened this issue Aug 20, 2020 · 6 comments
Assignees

Comments

@twifkak
Copy link
Member

twifkak commented Aug 20, 2020

@jridgewell's http://cl/327496988 (c/o #466) converts hero images from amp-img to amp-img > img and adds link rel=preload.

In addition, for SXG, we should convert all amp-imgs on the page to amp-img > img[loading=lazy] (but not preload them). Thus, all images above the fold can be fetched and decoded before full layout, but at a lower priority than hero images.

Example: https://jsfiddle.net/Ln9yejr3/show

This is particular to SXG-optimized AMP, because loading=lazy isn't implemented in all browsers that AMP targets, but it is implemented in almost all browsers that support SXG. (~3% of Chromium browsers are 73-75.)

As a precondition to this change, SXG-supporting AMP caches should remove the non-hero img tags when retransforming for unsigned use (which includes a higher fraction of non-loading=lazy-supporting browsers), so that the prerender network usage is the same as non-SXG AMP.

Moving discussion from ampproject/amphtml#29025 (comment). /cc @sebastianbenz @cramforce @jridgewell

@jridgewell
Copy link
Contributor

This sounds good. I think we can add loading=lazy to the SSR validator rules, then use that as the signal that this <img> is a low priority one (high priority ones should not lazy load).

@jridgewell
Copy link
Contributor

/tryassign @jridgewell

@amp-invite-bot
Copy link

I've assigned this issue to @jridgewell.

@jridgewell
Copy link
Contributor

jridgewell commented Oct 8, 2020

The C++ transformer already removes SSR'd amp-img > img, so it's covered.

honeybadgerdontcare pushed a commit that referenced this issue Oct 20, 2020
97% (and growing) of Chrome browsers that support SXG also support loading=lazy. We still prioritize (via preload and loading=eager img) hero images. Requires transforming with Version 5.

Re: #465
PiperOrigin-RevId: 336760832
twifkak pushed a commit that referenced this issue Oct 21, 2020
97% (and growing) of Chrome browsers that support SXG also support loading=lazy. We still prioritize (via preload and loading=eager img) hero images. Requires transforming with Version 5.

Re: #465
PiperOrigin-RevId: 336760832
@twifkak
Copy link
Member Author

twifkak commented Oct 21, 2020

Is there anything left to do on this (besides e2e testing)?

@jridgewell
Copy link
Contributor

No, I think that's it.

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

No branches or pull requests

2 participants