Make WordPress Core

Opened 6 months ago

Last modified 4 weeks ago

#60354 reviewing defect (bug)

Image filter functions strip query string from image src

Reported by: sanchothefat's profile sanchothefat Owned by: joemcgill's profile joemcgill
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords: reporter-feedback
Focuses: Cc:

Description

The following 2 functions strip the query string from the image src found in the content but it's not clear why this is done.

When using an external image service like Photon, Tachyon, or some other integration that provides resizing via a query string this breaks the generated srcset & sizes calculation, and can make the width and height attributes use the original image dimensions when the same image file name is used up to the start of the query string.

Take the following 2 image URL examples:

The query string means these are 2 different images but those functions will treat them both as the original.

Change History (8)

#1 @joemcgill
6 months ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

#2 @joemcgill
6 months ago

Hey @sanchothefat! These functions were designed to use the image src to try to match the markup with one of the image files stored on the filesystem, by comparing them to file paths stored in attachment metadata. In the case of srcset, this was to ensure that we are only including different dimensions of the same image in the source list, and not any cropped variations that would change the intended display of the image at different breakpoints.

For services that are doing dynamic generation of image sizes, you likely don't need to rely on WP to generate the srcset sizes, since you have the ability to generate your own list of sources on the fly. Jetpack, for example, filters the srcset values calculated by WP in order to serve sizes from their CDN.

I think the available filters would allow you to optimize these values even more effectively than what WP is able to do out of the box, but if there are things we could do to make that easier, feel free to make suggestions. Hope that explanation helps.

#3 @sanchothefat
6 months ago

Cheers @joemcgill, so I think the core functions would just work out of the box if not for stripping the query string off. If the filters provided the full unmodified image src it’d make the kind of integration I’m after much more simple. In general I prefer to avoid rewriting a lot of what core WP already does really well.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 months ago

#5 @antpb
4 months ago

  • Keywords reporter-feedback added

Hi @sanchothefat! This ticket came up in a recent Media Meeting and there was question around where the filter you would suggest would go and when/where it would make the data available. At the time of the lines shared it is parsing markup not the url of the image. If you know of a place that this would be better exposed that isn't available already, I think more folks would be open to a suggested filter.

In current state I'm a bit unsure how to move this forward in the wp_image_add_srcset_and_sizes and wp_img_tag_add_width_and_height_attr functions because we are parsing the html to get the data. It wouldnt be the right place to pull a url and make it filterable.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 weeks ago

#8 @joedolson
4 weeks ago

Hi, @sanchothefat! Right now, we believe that this is already solvable using existing functions, but are open to making it easier. However, we don't really have a clear understanding of your use case, so we're not sure exactly what path would be best.

If you can provide some additional detail on your use case, please do. Otherwise, we'll close this. Thanks!

Note: See TracTickets for help on using tickets.