vip-go-mu-plugins icon indicating copy to clipboard operation
vip-go-mu-plugins copied to clipboard

Filter `sizes` value for intermediate images

Open t-wright opened this issue 5 years ago • 12 comments

Desired Behavior

Responsive images should have a value on the sizes attribute which reflects the width of the chosen intermediate size.

Actual Behavior

When images are added to posts using the block editor, the value of the sizes attribute currently reflects the width of the original/full-size image. This can cause these images to display at the incorrect dimensions.

For more information:

P2: p9zhOE-Qw-p2 Ticket: #102087-zd-wordpressvip

t-wright avatar Dec 31 '19 04:12 t-wright

Noted in JIRA as GH-3353

simonwheatley avatar Dec 31 '19 04:12 simonwheatley

This issue persists and has been reported by Greg from Alley as well.

Ticket: #119159-zd-wordpressvip

yolih avatar Nov 19 '20 21:11 yolih

👋 Hi all. It looks like what is most likely happening here is that something in the a8c-files plugin is causing the dimension matching that would normally happen in wp_image_src_get_dimensions to fail. Unfortunately, there isn't currently a filter available to fix this issue when it occurs, but I've opened a Trac ticket to add this as a possible fix.

Ideally, this is something that could be fixed in the VIP plugin repo more quickly since this bug currently causes almost all images in content to be rendered with incorrect height, width, and sizes attributes—causing browsers to request much larger file sizes than necessary.

joemcgill avatar Nov 24 '20 15:11 joemcgill

This issue mentioned in ticket #123565-zd-wordpressvip Any blockers for committing the proposed fix? https://github.com/Automattic/vip-go-mu-plugins/pull/1900

lschuyler avatar Feb 24 '21 22:02 lschuyler

Hey @lschuyler - we discussed internally and want to explore other ways of addressing this that ideally don't need to filter the_content.

nickdaugherty avatar Feb 24 '21 23:02 nickdaugherty

This has been addressed in WP 5.7 with https://core.trac.wordpress.org/ticket/51865 Once that's released we can close this issue out.

mikeyarce avatar Mar 01 '21 16:03 mikeyarce

Ok, turns out we can't close this quite yet even with the new filter.

The main issue right now is that by the time wp_image_src_get_dimensions runs, our image path has been stripped of the requested dimensions that are found in the URL query string (e.g. ?w=400). WordPress then assumes it's a full size image and acts accordingly.

Where the parameters are stripped: list( $image_src ) = explode( '?', $image_src );

From: https://github.com/WordPress/WordPress/blob/0c5a3fa2ed4a6c773363984f61abd6b6d9641a69/wp-includes/media.php#L1915

Jetpack's Photon doesn't have the same issue because it does the interpolation really late on the_content whereas VIP does it really early, when the image is added in the editor.

@joemcgill - do you have any insight into why WordPress wants to remove these parameters? I wonder if we could add a filter here that could allow us to bypass this when we need these arguments to determine image dimensions.

mikeyarce avatar Mar 15 '21 16:03 mikeyarce

I don't know exactly why that line was added to this function, except that WordPress uses the file name to determine what the expected dimensions should be, by comparing against the sizes array for that attachment in post meta. Since you're overriding the URL earlier, we may need to add another filter here or pass the original src to the wp_image_src_get_dimensions function so you can override things there. It's worth opening a Trac ticket for, because you're not the only plugin that is filtering the src value before these calculations are made.

joemcgill avatar Mar 16 '21 01:03 joemcgill

This issue has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep issues manageable and actionable and is not a comment on the quality of this issue nor on the work done so far. Closed issues are still valuable to the project and are available to be searched.

github-actions[bot] avatar Sep 18 '22 00:09 github-actions[bot]

@mikeyarce @joemcgill has there been any movement on this issue? We're currently coding our way around it in themes, but it would be great to have it work out of the box. Is there anything I can do to help?

kevinfodness avatar Sep 21 '22 15:09 kevinfodness

@kevinfodness We've attempted to polyfill this solution into our own projects, but it didn't seem to work in the couple of places we tested it, but I've not revisited the implementation in several months. I assume we would need a new solution at this point.

joemcgill avatar Sep 21 '22 15:09 joemcgill

This issue has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep issues manageable and actionable and is not a comment on the quality of this issue nor on the work done so far. Closed issues are still valuable to the project and are available to be searched.

github-actions[bot] avatar Nov 22 '22 00:11 github-actions[bot]