Responsive images break uploads with full path stored in metadata

[SergeyBiryukov]

Before [8796], we used to store absolute file paths for uploaded images.

On my install, I have a couple files uploaded in a pre-2.7 era, where _wp_attached_file looks like this: H:\home\wordpress\trunk/wp-content/uploads/2008/06/2429111678_2ecb3abb93_b.jpg.

They used to display correctly before 4.4, but now the following markup is generated:

<img width="1024" height="755" src="http://trunk.wordpress/wp-content/uploads/2008/06/2429111678_2ecb3abb93_b.jpg" class="attachment-post-thumbnail size-post-thumbnail wp-post-image" alt="Sticky" srcset="http://trunk.wordpress/wp-content/uploads/H:\home\wordpress\trunk/wp-content/uploads/2008/06/2429111678_2ecb3abb93_b-300x221.jpg 300w, http://trunk.wordpress/wp-content/uploads/H:\home\wordpress\trunk/wp-content/uploads/2008/06/2429111678_2ecb3abb93_b.jpg 1024w" sizes="(max-width: 709px) 85vw, (max-width: 909px) 67vw, (max-width: 984px) 60vw, (max-width: 1362px) 62vw, 840px">

The culprit is wp_calculate_image_srcset()​, which assumes that dirname( $image_meta['file'] ) is always relative.

We could borrow the fix from wp_get_attachment_url()​, where it was added in [10254].

[joemcgill]

Thanks @SergeyBiryukov,

The fix looks like it will work but makes me wonder if we should institutionalize this knowledge in the form of a helper function for calculating the file path of an image, which could be used in both places?

[SergeyBiryukov]

Replying to joemcgill:

The fix looks like it will work but makes me wonder if we should institutionalize this knowledge in the form of a helper function for calculating the file path of an image, which could be used in both places?

Good point, 35106.2.patch​ introduces _wp_get_attachment_relative_path() and uses it in both places.

[azaozz]

35106.2.patch looks good. Unfortunately I don't have any pre-2.7 attachments to test it "live".

[azaozz]

Ran into another similar case on old multisite installs. The full path is stored again but instead of wp-content/uploads, there is wp-content/blogs.dir/[random]/files. This is handled by the previous row in wp_get_attachment_url(): https://core.trac.wordpress.org/browser/tags/4.4/src/wp-includes/post.php?marks=4872#L4870. In our case it should be:

$file = str_replace( $uploads['basedir'], '', $file );

Seems we will have to cache both basedir and baseurl in _wp_upload_dir_baseurl().

[dd32]

35106.diff​ is based off 35106.2.patch​

• Tested with /non-existant/path/wp-content/uploads/2015/12/image.jpg
• Tested with /non-existant/path/wp-content/uploads/image.jpg
• _wp_get_attachment_relative_path() returns a standard output, a foldername only, no . or filename.
• Needed to suffix the image url to _wp_get_attachment_relative_path() in one spot
• Needed to use _wp_get_attachment_relative_path() within the responsive image generation (It wasn't generating srcsets for them)

@azaozz:

Ran into another similar case on old multisite installs.

I can't quite work out the details here on what needs doing, or where you're proposing it needs to be taken into account, or where it's actually broken.. AFAICT multisite is already being handled right?

[azaozz]

35106.diff looks good. The case for old multisite installs can be handled through the (proposed) filter on the image meta for #35045, see https://core.trac.wordpress.org/attachment/ticket/35045/35045.8.diff.

[azaozz]

In 36120:

Responsive images: add compatibility for versions < 2.7 when the full image path was stored in the metadata. Introduces _wp_get_attachment_relative_path() and uses it in wp_get_attachment_url().

Props dd32, SergeyBiryukov.
Fixes #35106 for trunk.

[azaozz]

Reopen for 4.4.

[dd32]

In 36151:

Responsive images: add compatibility for versions < 2.7 when the full image path was stored in the metadata. Introduces _wp_get_attachment_relative_path() and uses it in wp_get_attachment_url().

Merges [36120] to the 4.4 branch.
Props dd32, SergeyBiryukov.
Fixes #35106.