Opened 5 months ago

Last modified 3 months ago

#22985 assigned defect (bug)

Edit thumbnail image only - loses all sub sizes in attachment meta

Reported by: lewismcarey Owned by: markoheijnen
Priority: high Milestone: 3.5.1
Component: Media Version: 3.5
Severity: critical Keywords: has-patch
Cc: bananastalktome@…

Description (last modified by SergeyBiryukov)

I have several additional image sizes in my theme. When I crop an image and save over only the thumbnail, all references to the various sub-sizes are lost and only the new thumbnail is referenced in _wp_attachment_metadata.

  1. Add an image to the media library.
  2. Output _wp_attachment_metadata.
  3. Edit the image (crop).
  4. Save only the thumbnail.
  5. Output the relevant _wp_attachment_metadata.

This may be an issue with wp_save_image() not re-genarating subsizes from the original.
I believe this to be different from Ticket #19889 as it appears to remove references to default sizes from media settings screen.

Attachments (2)

22985.diff (529 bytes) - added by markoheijnen 5 months ago.
using array_merge() inside wp_save_image()
media_tests.patch (2.9 KB) - added by bananastalktome 4 months ago.

Download all attachments as: .zip

Change History (22)

Whilst testing for this issue I have noticed that when using the 'Apply to all image sizes' option that custom image sizes are excluded here as well - so this issue doesn't seem to be limited to just the thumbnail only option.

The main diff I can see between image-edit.php 3.4 & 3.5 is

695     $meta['sizes'] = $img->multi_resize( $_sizes );

replaces

596    if ( $resized )
597        $meta['sizes'][$size] = $resized;
598    else
599        unset($meta['sizes'][$size]);

before wp_update_attachment_metadata($post_id, $meta); is called on success.

  • Milestone changed from Awaiting Review to 3.5.1
  • Owner set to markoheijnen
  • Status changed from new to assigned

I will look into this today. I can see the issue with the code lewis send but I need to look into the context of that since for what I think that get called when a new image getting uploaded. So if that is true that code is unrelated

#22983 was marked as a duplicate.

  • Keywords needs-patch added

Testing it and yes the multi_resize() call in wp_save_image() is wrong. It will then only put the changed images as meta sizes.

using array_merge() inside wp_save_image()

  • Keywords has-patch added; needs-patch removed
  • Priority changed from normal to high
  • Severity changed from major to critical
  • Description modified (diff)

For fixing broken metadata I think can use wp_restore_image() on all images that are missing thumbnail, medium or large on upgrade. Will going to play with it this or next week.

Let's get the fix in now, and then when we have code to go back and fix the issue on upgrade, put that in too.

Was playing to get a script done for hotfixing this issue. The result I have as a gist: https://gist.github.com/4349227

In 23246:

Don't stomp existing sizes inside wp_save_image().

props markoheijnen.
see #22985.
for trunk.

Went through a bunch of random ideas when talking to westi, regarding an upgrade routine. One is that _edit_lock gets set when entering post.php. We can look for all attachments with an _edit_lock greater than December 11, then individually run over those to fix their metadata. It's not foolproof, but it'll help a decent amount.

_wp_attached_file also gets updated with a suffix on edit, -e followed by 13 digits. The first 10 digits are time() and the last three are rand(100, 999). So we can use the time instead (which would also last through an export/import).

Next, we need to figure out how to fix these. The gist is nice to prevent saves as they occur. But what about once the sizes are missing? What is the best way to re-calculate and recover this data?

If I'm correct all the data still exists in get_post_meta( $post_id, '_wp_attachment_backup_sizes', true );
So then it looks like we still can do some kind of merge with the attachment meta data.

Anyone working on fixing the lost data? I was checking if we could used backup sizes but that's a no.

So it seems we need to check if an attachment has a '_edit_lock' and then loop check if the meta data sizes doesn't have an image from get_intermediate_image_sizes(). And this should be done as late as possible since themes do need to add their sizes.

A little new to phpunit tests with WordPress, but I attempted at a test to cover this case. Thoughts?

In 23342:

Don't stomp existing sizes inside wp_save_image().

Merges [23246] to the 3.5 branch.

props markoheijnen.
see #22985.

comment:18 follow-up: ↓ 19   markoheijnen4 months ago

Is this fixed and if not what is there to be fixed to close this ticket.

comment:19 in reply to: ↑ 18   SergeyBiryukov3 months ago

Replying to markoheijnen:

Is this fixed and if not what is there to be fixed to close this ticket.

media_tests.patch probably needs to be reviewed and committed.

  • Cc bananastalktome@… added
Note: See TracTickets for help on using tickets.