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

W-17609247 - Fix Safari bug where the same image loads several times #2223

Merged
merged 3 commits into from
Jan 29, 2025

Conversation

vcua-mobify
Copy link
Contributor

This PR addresses a known Safari bug seen also in other systems (such as this from Next.js).

The bug in Safari is that if an image has the src attribute before srcSet and sizes, it will fetch the image multiple times as it processes each attribute.

To ensure Safari only loads an image once, we set src to be the last attribute.

Reproducing the original issue:

  1. In Safari, open dev tools
  2. Go to https://pwa-kit.mobify-storefront.com/global/en-GB/product/Spring-look-2M (or deploy a version of the PWA without this fix onto an MRT test environment)
  3. In the network tab, search for PG.10236198.JJ3KRXX.PZ
  4. See that there are multiple requests to fetch this image (the first one without any query parameters and the second one with ?sw={width}&q=60)

You can compare this result with Chrome (which only loads the image once with ?sw={width}&q=60)

Testing the fix:

  1. Deploy this fix onto MRT and in Safari navigate to the same page on your test environment (or go to https://scaffold-pwa-vincentc-test-env.mobify-storefront.com/global/en-GB/product/Spring-look-2M)
  2. In the network tab, search for PG.10236198.JJ3KRXX.PZ
  3. See that there is now only one request to fetch the image (the one with ?sw={width}&q=60)

@vcua-mobify vcua-mobify requested a review from a team as a code owner January 28, 2025 01:10
@vcua-mobify vcua-mobify requested a review from vmarta January 28, 2025 01:10
sizes: mapWidthsToSizes(widths),
srcSet: mapWidthsToSrcSet(widths, src)
srcSet: mapWidthsToSrcSet(widths, src),
src: getSrcWithoutOptionalParams(src)
Copy link
Contributor

@vmarta vmarta Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a code comment to explain why and to make sure that src will be kept last.

@vcua-mobify vcua-mobify requested a review from vmarta January 28, 2025 17:51
Copy link
Contributor

@vmarta vmarta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the test environment, I've verified that the bug fix works as expected on Safari browser 👍

Copy link
Collaborator

@joeluong-sfcc joeluong-sfcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed on the test env that there is no duplicate image requests

@vcua-mobify vcua-mobify merged commit 81697ce into develop Jan 29, 2025
29 checks passed
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

Successfully merging this pull request may close these issues.

3 participants