Skip to content

Fix srcsetWidth methods to properly handle retina variants#432

Open
lukastransom wants to merge 1 commit intonystudio107:develop-v5from
lukastransom:fix/srcset-retina-variants
Open

Fix srcsetWidth methods to properly handle retina variants#432
lukastransom wants to merge 1 commit intonystudio107:develop-v5from
lukastransom:fix/srcset-retina-variants

Conversation

@lukastransom
Copy link
Copy Markdown

Description

The getSrcsetSubsetArray() method fails when retina variants (2x, 3x) are configured, causing srcsetWidth(), srcsetWidthWebP(), and related methods to return wrong variants.

Index-based mapping between variantSourceWidths (duplicated for retina) and sorted optimizedImageUrls keys breaks when looking up variants.

Implemented Fix

Replace fragile index mapping with direct width analysis:

  1. Iterate through actual widths in the URL set
  2. Detect source width by checking retina multipliers (1x/2x/3x)
  3. Filter based on source width comparisons
  4. Add DPR parameter to control retina inclusion

What's Been Tested

  • ✅ All 6 methods: srcsetWidth*(), srcsetMinWidth*(), srcsetMaxWidth*()
  • ✅ Both DPR modes: width descriptors (w) vs density descriptors (x)

Configuration: 5 base widths (360, 442, 466, 559, 573) × 2 retina variants = 10 total

Before Fix (Broken)

{{ optimizedImages.srcsetWidthWebP(559, true) }}
→ .../884px/...webp 1x, .../932px/...webp 1x  {# Wrong variants! #}

After Fix (Correct)

{{ optimizedImages.srcsetWidthWebP(559, true) }}
→ .../559px/...webp 1x, .../1118px/...webp 2x  {# Correct! #}

{{ optimizedImages.srcsetWidthWebP(559, false) }}
→ .../559px/...webp 559w                       {# Only 1x variant #}

{{ optimizedImages.srcsetMaxWidth(466, true) }}
→ 360px@1x, 720px@2x, 442px@1x, 884px@2x, 466px@1x, 932px@2x
Related Issues

@khalwat
Copy link
Copy Markdown
Contributor

khalwat commented Apr 29, 2026

I'm so sorry, this fell off of my radar. I'm planning to merge it in and backport it this week.

Are there any changes since your initial PR?

@lukastransom
Copy link
Copy Markdown
Author

This is the same patch I've been using for my 2 latest projects w/o issue. No additional changes on my end!

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.

2 participants