adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
747 stars 753 forks source link

dpr for Dynamic Media =off in srcset #2829

Closed HitmanInWis closed 1 month ago

HitmanInWis commented 3 months ago

Bug Present as of Version: 2.25.5-SNAPSHOT

When using Image v3 with dynamic media images (i.e. anything other than a directly specified smart crop rendition) the dpr parameter is set to off. Javascript in imageDynamicMedia.js updates dpr=off to dpr=on,<value> per the changes on https://github.com/adobe/aem-core-wcm-components/pull/2450 but nothing modifies the values on srcset As such, if the browser picks a rendition from srcset we lose the dpr setting.

HitmanInWis commented 2 months ago

Thinking on this more, is dpr=off actually correct for use in srcset? i.e. does the browser already figure out on its own which image resolution to use, with device pixel ratio already being figured into the equation, and thus sending a dpr value to Dynamic Media will actually potentially fetch an image larger than necessary?

HitmanInWis commented 1 month ago

After further testing in Chrome Dev tools, it appears that when dpr=2 the browser is already requesting the rendition from srcset that is 2x the width required by the component width, thus using dpr in tandem with srcset is resulting in double applying the device pixel ratio (i.e. I'm getting an image that is 4x the needed size instead of 2x).

So using dpr=off for all URLs in srcset appears to be the appropriate functionality.

Closing this ticket as invalid.