Skip to content

Correct srcset to prevent blurry images#1037

Closed
girishpanchal30 wants to merge 4 commits intodevelopmentfrom
bugfix/1030
Closed

Correct srcset to prevent blurry images#1037
girishpanchal30 wants to merge 4 commits intodevelopmentfrom
bugfix/1030

Conversation

@girishpanchal30
Copy link
Copy Markdown
Contributor

@girishpanchal30 girishpanchal30 commented Mar 23, 2026

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 (naturalWidth and naturalHeight) as data attributes on the <img> tag. Since images served via Optimole are dynamically resized, the browser-reported naturalWidth and naturalHeight reflect the optimized image rather than the original source, leading to incorrect dimension calculations.

Closes #1030

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@pirate-bot
Copy link
Copy Markdown
Collaborator

pirate-bot commented Mar 23, 2026

Plugin build for 8aa5536 is ready 🛎️!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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-height onto 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/naturalHeight now come only from data-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 to img.naturalWidth/img.naturalHeight when the data attributes are missing/NaN. Also, this change alone doesn’t address issue #1030 because _generateResponsiveSizes() still caps 1x variants by currentWidth; 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.

Comment on lines +319 to +330
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 );
}
}
}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 );
					}
				}
			}

Comment on lines +319 to +330
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 );
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 );
					}
				}
			}

Comment on lines +186 to +187
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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You modified here that the natural height is not longer needed. Is this also true for this function

const responsiveSizes = this._generateResponsiveSizes(currentWidth, currentHeight, aspectRatio, naturalWidth, naturalHeight);
or just here?

In the idea that we might not need it at all if we go with your fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@selul
Copy link
Copy Markdown
Contributor

selul commented Mar 25, 2026

closing it since it was not addressing the root cause.

@selul selul closed this Mar 25, 2026
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.

srcset capped at profiling container width — images served blurry on viewports wider than profiling screen

5 participants