Correct srcset to prevent blurry images#1037
Correct srcset to prevent blurry images#1037girishpanchal30 wants to merge 4 commits intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent blurry images on larger viewports by ensuring the JS srcset profiler uses the original image dimensions (not the optimized ones) when deciding which responsive sizes to include.
Changes:
- Inject
data-original-width/data-original-heightonto processed<img>tags (derived from attachment metadata). - Update the srcset detector to read “natural” dimensions from those new data attributes instead of
img.naturalWidth/Height.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
inc/tag_replacer.php |
Adds original-dimension data attributes to image tags and introduces a URL→attachment-ID helper. |
assets/js/modules/srcset-detector.js |
Switches “natural” dimension reads to the new HTML data attributes. |
Comments suppressed due to low confidence (1)
assets/js/modules/srcset-detector.js:193
naturalWidth/naturalHeightnow come only fromdata-original-*, so any image where PHP couldn't inject these attributes (non-attachment URLs, offloaded/DAM URLs that don’t resolve, cached content generated before this change) will be skipped entirely (insufficient dimension data). Consider falling back toimg.naturalWidth/img.naturalHeightwhen the data attributes are missing/NaN. Also, this change alone doesn’t address issue #1030 because_generateResponsiveSizes()still caps 1x variants bycurrentWidth; the cap needs to be based on the (original) natural width instead.
const naturalWidth = img.hasAttribute('data-original-width') ? parseInt(img.getAttribute('data-original-width'), 10) : 0;
const naturalHeight = img.hasAttribute('data-original-height') ? parseInt(img.getAttribute('data-original-height'), 10) : 0;
// Skip if we can't determine dimensions
if (!currentWidth || !currentHeight || !naturalWidth || !naturalHeight) {
optmlLogger.warn(`Skipping image ${imageId}: insufficient dimension data`);
return null;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
inc/tag_replacer.php
Outdated
| if ( strpos( $image_tag, 'data-original-width=' ) === false || strpos( $image_tag, 'data-original-height=' ) === false ) { | ||
| $_image_id = $this->get_image_id_from_url( $images['img_url'][ $index ] ); | ||
| $image_metadata = wp_get_attachment_metadata( $_image_id ); | ||
| if ( $image_metadata ) { | ||
| if ( strpos( $image_tag, 'data-original-width=' ) === false ) { | ||
| $image_tag = str_replace( 'data-opt-id=', 'data-original-width="' . $image_metadata['width'] . '" data-opt-id=', $image_tag ); | ||
| } | ||
| if ( strpos( $image_tag, 'data-original-height=' ) === false ) { | ||
| $image_tag = str_replace( 'data-opt-id=', 'data-original-height="' . $image_metadata['height'] . '" data-opt-id=', $image_tag ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
There are existing PHPUnit coverage suites for tag replacement/srcset behavior (e.g., tests/test-replacer.php, tests/test-srcset.php). Please add/update tests to assert process_image_tags() injects data-original-width/data-original-height for Media Library images (including resized filenames like -150x150, @2x, and -scaled) so this doesn’t regress.
There was a problem hiding this comment.
This can be compacted like this:
$missing_original_width = false === strpos( $image_tag, 'data-original-width=' );
$missing_original_height = false === strpos( $image_tag, 'data-original-height=' );
if ( $missing_original_width || $missing_original_height ) {
$_image_id = $this->attachment_url_to_post_id( wp_unslash( $images['img_url'][ $index ] ) );
$image_metadata = wp_get_attachment_metadata( $_image_id );
if ( $image_metadata && is_array( $image_metadata ) ) {
if ($missing_original_width && ! empty( $image_metadata['width'] ) ) {
$original_width = (int) $image_metadata['width'];
$image_tag = str_replace( 'data-opt-id=', 'data-original-width="' . $original_width . '" data-opt-id=', $image_tag );
}
if ( $missing_original_height && ! empty( $image_metadata['height'] ) ) {
$original_height = (int) $image_metadata['height'];
$image_tag = str_replace( 'data-opt-id=', 'data-original-height="' . $original_height . '" data-opt-id=', $image_tag );
}
}
}
inc/tag_replacer.php
Outdated
| if ( strpos( $image_tag, 'data-original-width=' ) === false || strpos( $image_tag, 'data-original-height=' ) === false ) { | ||
| $_image_id = $this->get_image_id_from_url( $images['img_url'][ $index ] ); | ||
| $image_metadata = wp_get_attachment_metadata( $_image_id ); | ||
| if ( $image_metadata ) { | ||
| if ( strpos( $image_tag, 'data-original-width=' ) === false ) { | ||
| $image_tag = str_replace( 'data-opt-id=', 'data-original-width="' . $image_metadata['width'] . '" data-opt-id=', $image_tag ); | ||
| } | ||
| if ( strpos( $image_tag, 'data-original-height=' ) === false ) { | ||
| $image_tag = str_replace( 'data-opt-id=', 'data-original-height="' . $image_metadata['height'] . '" data-opt-id=', $image_tag ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This can be compacted like this:
$missing_original_width = false === strpos( $image_tag, 'data-original-width=' );
$missing_original_height = false === strpos( $image_tag, 'data-original-height=' );
if ( $missing_original_width || $missing_original_height ) {
$_image_id = $this->attachment_url_to_post_id( wp_unslash( $images['img_url'][ $index ] ) );
$image_metadata = wp_get_attachment_metadata( $_image_id );
if ( $image_metadata && is_array( $image_metadata ) ) {
if ($missing_original_width && ! empty( $image_metadata['width'] ) ) {
$original_width = (int) $image_metadata['width'];
$image_tag = str_replace( 'data-opt-id=', 'data-original-width="' . $original_width . '" data-opt-id=', $image_tag );
}
if ( $missing_original_height && ! empty( $image_metadata['height'] ) ) {
$original_height = (int) $image_metadata['height'];
$image_tag = str_replace( 'data-opt-id=', 'data-original-height="' . $original_height . '" data-opt-id=', $image_tag );
}
}
}| const naturalWidth = img.hasAttribute('data-original-width') ? parseInt(img.getAttribute('data-original-width'), 10) : 0; | ||
| const naturalHeight = img.hasAttribute('data-original-height') ? parseInt(img.getAttribute('data-original-height'), 10) : 0; |
There was a problem hiding this comment.
You modified here that the natural height is not longer needed. Is this also true for this function
or just here?In the idea that we might not need it at all if we go with your fix.
There was a problem hiding this comment.
Yes, that height also works with the function, as it is used to verify whether the size is valid for the current rendered image dimensions.
|
closing it since it was not addressing the root cause. |
All Submissions:
There is an open PR with the fix as suggested in the issue description.
Changes proposed in this Pull Request:
Passed the original image dimensions (
naturalWidthandnaturalHeight) as data attributes on the<img>tag. Since images served via Optimole are dynamically resized, the browser-reportednaturalWidthandnaturalHeightreflect the optimized image rather than the original source, leading to incorrect dimension calculations.Closes #1030
Other information: