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.

Sign up for GitHub

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

Jump to bottom

amp-img - duplicate image downloads on iOS Safari 14.4 #32629

Closed
andydavies opened this issue Feb 12, 2021 · 4 comments
Closed

amp-img - duplicate image downloads on iOS Safari 14.4 #32629

andydavies opened this issue Feb 12, 2021 · 4 comments
Assignees
@kristoferbaxter
Labels
P2: Soon Type: Bug WG: components

Comments

@andydavies
Copy link

andydavies commented Feb 12, 2021 •
edited

What's the issue?

amp-img makes duplicate image requests when both img and srcset attributes are defined.

How do we reproduce the issue?

  1. Connect iPhone to Mac, open Inspector etc.
  2. Load page below on iPhone Safari
  3. View image requests in Inspector
  4. Observe two requests are made by the amp-img component – seems to be both the src, and one defined in srcset

Have observed issue on multiple real-world sites.

Test case - https://output.jsbin.com/cisawet

<!doctype html>
<html amp lang="en">

<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">

    <script async src="https://cdn.ampproject.org/v0.js"></script>

</head>
<body>
  <amp-img src="https://amp.belfasttelegraph.co.uk/news/northern-ireland/41a24/40057065.ece/AUTOCROP/w620h350/bpanews_63ec4cf7-b63f-4f8a-bc1b-dba90afca0c1_1" width="375" height="210" layout="responsive" alt="More than 300,000 people have now been vaccinated in Northern Ireland (PA)" srcset="https://m.belfasttelegraph.co.uk/news/northern-ireland/41a24/40057065.ece/AUTOCROP/w620h350/bpanews_63ec4cf7-b63f-4f8a-bc1b-dba90afca0c1_1 620w,https://m.belfasttelegraph.co.uk/news/northern-ireland/41a24/40057065.ece/AUTOCROP/w840h475/bpanews_63ec4cf7-b63f-4f8a-bc1b-dba90afca0c1_1 840w,https://m.belfasttelegraph.co.uk/news/northern-ireland/41a24/40057065.ece/AUTOCROP/w1240h700/bpanews_63ec4cf7-b63f-4f8a-bc1b-dba90afca0c1_1 1240w,https://m.belfasttelegraph.co.uk/news/northern-ireland/41a24/40057065.ece/AUTOCROP/w1680h950/bpanews_63ec4cf7-b63f-4f8a-bc1b-dba90afca0c1_1 1680w"></amp-img>
</body>
</html>

What browsers are affected?

  • iOS Safari 14.4 on iPhone SE 2020
  • Desktop Safari 14.0.3 (15610.4.3.1.6, 15610) – in both normal and responsive design mode

Which AMP version is affected?

Powered by AMP ⚡ HTML – Version 2101300534005

Unknown whether it previously worked

Screenshot 2021-02-12 at 13 18 40
@andydavies andydavies changed the title Duplicate image downloads on iOS Safari 14.4 amp-img - duplicate image downloads on iOS Safari 14.4 Feb 12, 2021
@kristoferbaxter kristoferbaxter self-assigned this Feb 12, 2021
@kristoferbaxter
Copy link
Contributor

Taking a look, have to download the iOS 14.4 simulator first so will report back afterwards.

@kristoferbaxter
Copy link
Contributor

Looks like this is an issue with propagating src before srcset. Making a change now, working on tests.

@kristoferbaxter kristoferbaxter mentioned this issue Feb 12, 2021
Merged
@kristoferbaxter
Copy link
Contributor

Network requests for different sources is fixed with #32634. Taking a look at the double request for the same resource is going to take a bit longer, it appears at first glance to be an issue with Webkit (see final comment here from 2018, https://bugs.webkit.org/show_bug.cgi?id=6656)

@kristoferbaxter
Copy link
Contributor

kristoferbaxter commented Feb 12, 2021 •
edited

Unable to reproduce the double network request issue with a direct example: https://thrilling-workable-flyingfish.glitch.me/image-element-propagation.html

Turns out debugging the issue causes a double network request with this fix applied. Once I removed breakpoints the network request was only made once.

Closing this issue as resolved, and it should be available in the next release.

Edit: Added comment about debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kristoferbaxter kristoferbaxter

Labels
P2: Soon Type: Bug WG: components
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kristoferbaxter @andydavies