WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#32171 closed defect (bug) (fixed)

Images not removed upon restore when IMAGE_EDIT_OVERWRITE true

Reported by: bradt Owned by: mikeschroder
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.2.1
Component: Media Keywords: has-patch needs-testing has-unit-tests
Focuses: administration Cc:

Description

With IMAGE_EDIT_OVERWRITE set to true, image edit files are removed when the original image is restored. But in certain cases, leftover image edit files remain.

Steps to reproduce:

  1. Add define( 'IMAGE_EDIT_OVERWRITE', true ); to your wp-config.php
  2. Upload an image
  3. Crop it and save
  4. Crop it and save again

You'll notice that the second crop overwrote the 150x150 intermediate image, created some new intermedia images, and left some intermediate images from the first crop behind. Now if you restore the original image, the images created by the second crop are removed, but the leftover images from the first crop remain.

It should probably remove the images from the first crop when you do the second crop.

Attachments (4)

32171.diff (956 bytes) - added by bradt 5 years ago.
32171.2.diff (2.8 KB) - added by bradt 4 years ago.
Added a unit test.
32171.2.2.diff (3.9 KB) - added by chriscct7 4 years ago.
Adds some checks for safety when trying to access indices of an array coming from post_meta
32171.3.diff (2.8 KB) - added by mikeschroder 4 years ago.
Clean up; use existing stored dirname, rather than extracting for each size.

Download all attachments as: .zip

Change History (17)

This ticket was mentioned in Slack in #feature-respimg by joemcgill. View the logs.


5 years ago

#2 @mtekk
5 years ago

  • Keywords needs-patch added

I was able to reproduce this quite easily. A little more background on this, it appears to only occur when the aspect ratio of the crops are not the same (hence the 150x150 thumbnail getting overwritten but nothing else is). When that happens, the scaled images will have multiple copies for a specific limiting dimension.

Keep the related ticket #32302 in mind when approaching a solution.

@bradt
5 years ago

#3 @bradt
5 years ago

  • Keywords has-patch needs-unit-tests needs-testing added; needs-patch removed

@bradt
4 years ago

Added a unit test.

#4 @chriscct7
4 years ago

  • Keywords needs-unit-tests removed
  • Milestone changed from Awaiting Review to 4.6
  • Owner set to chriscct7
  • Status changed from new to reviewing

#5 @chriscct7
4 years ago

Since the $meta is being pulled via a helper function from a piece of post_meta, I've added checks to ensure that expected indices exist, as plugins could if they wanted to change the expected results.

This looks good otherwise.

@chriscct7
4 years ago

Adds some checks for safety when trying to access indices of an array coming from post_meta

#6 @chriscct7
4 years ago

  • Version changed from 4.2.1 to 4.2

#7 @chriscct7
4 years ago

  • Version changed from 4.2 to 4.2.1

Ignore these. Me and nacin are testing a bug in trac at wceu.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


4 years ago

#9 @swissspidy
4 years ago

  • Keywords has-unit-tests added

#10 @mikeschroder
4 years ago

  • Owner changed from chriscct7 to mikeschroder

I'll handle the last review for commit.

@mikeschroder
4 years ago

Clean up; use existing stored dirname, rather than extracting for each size.

#11 @mikeschroder
4 years ago

In 32171.3.diff:
Small clean up; use existing stored dirname, rather than extracting for each size.

#12 @mikeschroder
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 38113:

Media: Clean up prior image edits if IMAGE_EDIT_OVERWRITE is true.

When IMAGE_EDIT_OVERWRITE is set to true, edited image files are
supposed to be deleted when an image is restored to the original.

However, when an image was edited more than once, and then restored,
files created during previous edits were left behind.

Fixes this behavior by updating wp_save_image() to clean up
leftover images after each edit when IMAGE_EDIT_OVERWRITE is true.

Props bradt, chriscct7, joemcgill.
Fixes #32171.

#13 @ocean90
4 years ago

In 38139:

Docs: Fix minor formatting issue for a comment added in [38113].

See #32171.

Note: See TracTickets for help on using tickets.