Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#24518 closed defect (bug) (fixed)

wp_delete_attachment() does not delete all the intermediate image sizes on some conditions

Reported by: joshuaabenazer's profile JoshuaAbenazer Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.5.1
Component: Media Keywords: has-patch
Focuses: Cc:


The wp_delete_attachment() function relies on the get_intermediate_image_sizes() function which in turn depends on the global $_wp_additional_image_sizes to delete the intermediate image sizes. The global $_wp_additional_image_sizes is filled with additional images sizes through the add_image_size() function, either through themes or plugins.

Steps to reproduce the bug
Now consider the following scenario. We have installed a plugin or theme that makes use of add_image_size() to add an intermediate image. Suppose after using the plugin or theme after few days or months we decide to stop using it for some reason. Now what happens is the particular add_image_size() that was called by the plugin or theme does not exist anymore. Now I delete a particular media image that was uploaded when the particular theme or plugin was activated. The attachment gets deleted but that one particular size remains in the file system since it wasn't recognized by the global $_wp_additional_image_sizes

Instead of relying on the get_intermediate_image_sizes() function or global $_wp_additional_image_sizes we could just loop through sizes available in the _wp_attachment_metadata postmeta ( wp_get_attachment_metadata() )

Attachments (3)

wp-includes-post.php.patch (1.3 KB) - added by JoshuaAbenazer 11 years ago.
wp_delete_attachment() patch
post.php.patch (1.3 KB) - added by desrosj 11 years ago.
Refreshed version of patch with some spacing adjustments to follow WordPress coding standards.
24518.diff (1.4 KB) - added by markoheijnen 10 years ago.

Download all attachments as: .zip

Change History (18)

11 years ago

wp_delete_attachment() patch

#1 @SergeyBiryukov
11 years ago

  • Keywords 3.7-early added
  • Milestone changed from Awaiting Review to Future Release

#2 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#3 @desrosj
11 years ago

I can reproduce the issue. JoshuaAbenazer's patch works great fixing the issue.

11 years ago

Refreshed version of patch with some spacing adjustments to follow WordPress coding standards.

#4 @ocean90
11 years ago

  • Keywords commit added

Looks good in my eyes. Any objections?

#5 @nacin
11 years ago

The only issue I can think of is #22985. Might we want to do both?

#7 @johnbillion
11 years ago

Patch still applies cleanly. Tested and working with an image size that's removed before the attachment is deleted. Anything else to consider?

#8 @nacin
11 years ago

As suggested in my comment above and as communicated to johnbillion at WordCamp Europe, the final consideration is that not everything always exists in meta. (The only situation where this may have occurred is in #22985.)

So, I think we should do both methods.

I also think this is puntable.

#9 @johnbillion
11 years ago

  • Keywords needs-patch added; has-patch 3.7-early commit removed
  • Milestone changed from 3.7 to 3.8

#10 @samuelsidler
11 years ago

  • Keywords 3.9-early added
  • Milestone changed from 3.8 to Future Release

#12 @SergeyBiryukov
10 years ago

#28076 was marked as a duplicate.

10 years ago

#13 @markoheijnen
10 years ago

  • Keywords 3.9-early removed
  • Milestone changed from Future Release to 4.1

Moving to 4.1. For now I only refreshed the patch. Will work on a patch to combine both in the afternoon. Adding the $meta['sizes'] to the intermediate_sizes array.

Last edited 10 years ago by SergeyBiryukov (previous) (diff)

#14 @wonderboymusic
10 years ago

  • Keywords has-patch added; needs-patch removed

#22985 is fixed, so let's do this

#15 @wonderboymusic
10 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 29816:

In wp_delete_attachment(): account for orphan sizes by looping over the sizes stored in metadata, instead of relying on the current sizes stored in $_wp_additional_image_sizes.

Props JoshuaAbenazer, desrosj, markoheijnen.
Fixes #24518.

Note: See TracTickets for help on using tickets.