Amphtml: AMP's srcset/sizes is different from browser's native implementation

Created on 6 Oct 2017 · 24Comments · Source: ampproject/amphtml

What's the issue?

AMP's sizes attribute doesn't work in the same way as a browser's native implementation. AMP-IMG tag doesn't seem to be respecting the width of the element set by SIZES attribute and ends up loading the image based on the entire viewport width.

This was pointed out by our client. The test was done on a desktop browser. This does not happen normally as users will generally only land on AMP pages from a mobile device and these devices generally have a width below 600px. I'am mentioning the issue here just in case it's possible to address this difference.

How do we reproduce the issue?

Please disable caching through 'Developer Tools' > 'Network' tab > 'Disable Cache' then try out the following links on desktop to observe the difference.
_Note : sizes="300px" in both the following cases._
Native Implementation : https://codepen.io/KraftPixel/pen/JrMQaJ
AMP Implementation : https://codepen.io/KraftPixel/pen/JrMQpN
For reference : Here is a screenshot of the output on a 1920x1080 -> https://i.imgur.com/g2jFaxu.png

What browsers are affected?

Tested on Google Chrome (Desktop)

Which AMP version is affected?

1506977814714

High Priority Bug

Source

kraftpixel

👍2

Most helpful comment

Just an update on this. We're going to ramp up this experiment on a weekly basis: 1%, 10%, 30%, 50%, 100%. Assuming no P0/rollbacks, we'll have this 100% in production in roughly 4 weeks and I'll cleanup the experiment in week 5. If anyone wants to opt in for testing, this is currently 100% in canary (dev channel), and under the experiment 'amp-img-native-srcset'.

One thing missing here is I think that we need to generate a sizes attribute if it is not present. This generated sizes attribute should select a src from the srcset based on the screen width.

^ I'm tracking this, and will start ramping that up when we're a bit more certain that 'amp-img-native-srcset' isn't breaking anyone.

cathyxz on 24 May 2018

👍2

All 24 comments

/cc @dvoytenko

aghassemi on 6 Oct 2017

@cramforce is this the way you wanted this in the last PR?

dvoytenko on 6 Oct 2017

I did not realize there was the action-at-a-distance of handling the sizes attribute in framework land and srcset on the image itself.

I suppose we could revert to old behavior if the sizes attribute is present on the amp-img element as a quick fix?

I still think that going forward we should just use the native implementation here.

cramforce on 6 Oct 2017

@cramforce I suppose either way. Maybe we should just copy srcset/sizes from amp-img to img and leave the current implementation in place as a polyfill for those that do not support it?

dvoytenko on 6 Oct 2017

cramforce on 7 Oct 2017

This is a high priority issue but it hasn't been updated in awhile. Do you have any updates?

ampprojectbot on 30 Oct 2017

This is a high priority issue but it hasn't been updated in awhile. Do you have any updates?

ampprojectbot on 20 Nov 2017

Since is a high priority I'm assigning it to you @dvoytenko Feel free to re-assign.

adelinamart on 20 Nov 2017

This is a high priority issue but it hasn't been updated in awhile. @dvoytenko Do you have any updates?

ampprojectbot on 4 Dec 2017

This is a high priority issue but it hasn't been updated in awhile. @dvoytenko Do you have any updates?

ampprojectbot on 25 Dec 2017

This is a high priority issue but it hasn't been updated in awhile. @dvoytenko Do you have any updates?

ampprojectbot on 15 Jan 2018

Here is an example:

<!doctype html>

<html amp lang="en">
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
    <script async src="https://cdn.ampproject.org/v0.js"></script>
    <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
    <style amp-custom>
      img, amp-img {
        width: 10vw;
        height: auto;
      }
    </style>
  </head>
  <body>
    <amp-img src='http://placehold.it/720x440'
      width='720' height='480'
      sizes="10vw"
      srcset="http://placehold.it/360x240 360w, http://placehold.it/720x480 720w, http://placehold.it/1080x720 1080w,  http://placehold.it/1440x960 1440w">
    </amp-img>
    <img src='http://placehold.it/720x440'
      width='720' height='480'
      sizes="10vw"
      srcset="http://placehold.it/360x240 360w, http://placehold.it/720x480 720w, http://placehold.it/1080x720 1080w,  http://placehold.it/1440x960 1440w">
  </body>
</html>

tbll on 27 Jan 2018

👍1

If I understand correctly, <amp-img> assumes sizes=100vw not matter what. This totally defeat the purpose of <amp-img>. All my amp pages download the highest resolution of my images :(

tbll on 27 Jan 2018

Yeah. Clearly wrong. We'll fix asap.

On Jan 27, 2018 11:47 AM, "Almen Munsel" notifications@github.com wrote:

If I understand correctly, assumes sizes=100vw not matter what.
This totally defeat the purpose of . All my amp pages download
the highest resolution of my images :(

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ampproject/amphtml/issues/11575#issuecomment-361010110,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANd2kBK2uB7tTeA-KxxDrbkxlXkVtQ8Pks5tO31egaJpZM4PwHNU
.

amphtml-team on 27 Jan 2018

Sizes should not be ignored. My first change was just bringing it towards native srcset. Lets fix, but primarily lets just use native srcset and be done with it. There are no longer any browser issues. We only need to dynamically create a missing sizes when needed.

cramforce on 28 Jan 2018

👍1

Logging some offline discussions:

1- We will move to full native support and remove AMP-specific logic from processing srcset
2- We may add a new opt-in to evaluate srcset based on container width rather than viewport width ( useful for cases such as thumbnail grids). This might endup being a "best-effort" and ignored in certain situations (e.g. hero images).

aghassemi on 29 Jan 2018

/cc @ericlindley-g

aghassemi on 30 Jan 2018

This is a high priority issue but it hasn't been updated in awhile. @dvoytenko Do you have any updates?

ampprojectbot on 19 Feb 2018

Taking notes from office hours today, we're proposing a solution as following:

Propagate srcset and sizes from <amp-img> to <img>.
Add a new attribute cancel-sizes-layout (or better naming) applicable to all components but applicable primarily to <amp-img>. If specified, this attribute will cancel the current layout logic we do based on sizes.
Add a new attribute img-sizes to <amp-img>, representing the native sizes attribute on <img>. Basically the end result of the sizes attribute propagated to <img> from <amp-img> should be sizes = img-sizes || sizes.

Does this sound right, @aghassemi @dvoytenko?

Clarification: go with either 2 or 3, not both.

cathyxz on 22 Feb 2018

👍1

@cathyxz I think this is cool, but I have a few questions:

This also means that we can fully rely on native srcset, right?
One thing missing here is I think that we need to generate a sizes attribute if it is not present. This generated sizes attribute should select a src from the srcset based on the screen width.
Why do we need cancel-sizes-layout instead of just img-sizes?

cramforce on 22 Feb 2018

so we will either do 2 or 3. Alternative name suggestions:
for (2): sizes-behavior=<native, layout>, use-native-sizes , ??
for (3): native-sizes

aghassemi on 22 Feb 2018

Any update here?

Not having this is actually a problem for our webpackaging project.

Eventually @twifkak's preload generation needs to mimick the same algorithm.

@adelinamart Lets track this as a dependency for WPK.

cramforce on 27 Apr 2018

@cathyxz has started working on a fix. Thanks Cathy!

aghassemi on 2 May 2018

One thing missing here is I think that we need to generate a sizes attribute if it is not present. This generated sizes attribute should select a src from the srcset based on the screen width.

^ I'm tracking this, and will start ramping that up when we're a bit more certain that 'amp-img-native-srcset' isn't breaking anyone.

cathyxz on 24 May 2018

👍2

Was this page helpful?

0 / 5 - 0 ratings