WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#24518 closed defect (bug) (fixed)

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

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

Description

Functionality
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

Solution
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 6 years ago.
wp_delete_attachment() patch
post.php.patch (1.3 KB) - added by desrosj 6 years ago.
Refreshed version of patch with some spacing adjustments to follow WordPress coding standards.
24518.diff (1.4 KB) - added by markoheijnen 5 years ago.

Download all attachments as: .zip

Change History (18)

@JoshuaAbenazer
6 years ago

wp_delete_attachment() patch

#1 @SergeyBiryukov
6 years ago

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

#2 @wonderboymusic
6 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#3 @desrosj
6 years ago

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

@desrosj
6 years ago

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

#4 @ocean90
6 years ago

  • Keywords commit added

Looks good in my eyes. Any objections?

#5 @nacin
6 years ago

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

#7 @johnbillion
6 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
6 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
6 years ago

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

#10 @samuelsidler
6 years ago

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

#12 @SergeyBiryukov
5 years ago

#28076 was marked as a duplicate.

@markoheijnen
5 years ago

#13 @markoheijnen
5 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 5 years ago by SergeyBiryukov (previous) (diff)

#14 @wonderboymusic
5 years ago

  • Keywords has-patch added; needs-patch removed

#22985 is fixed, so let's do this

#15 @wonderboymusic
5 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.