WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 5 weeks ago

#19889 closed defect (bug) (fixed)

Image Editor doesn't apply changes to custom image sizes

Reported by: Otto42 Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.3
Component: Media Keywords: has-patch 3.9-early
Focuses: Cc:

Description

The built in image editor doesn't apply changes like cropping, rotation, etc. to image sizes added using add_image_size or similar methods.

This is because the code in image-edit.php is directly looking for the image sizes in the options table, which only valid for the built-in sizes, not for custom image sizes.

Specifically, in the wp_save_image function in image-edit.php, this code is incorrect:

$crop = $nocrop ? false : get_option("{$size}_crop");
$resized = image_make_intermediate_size($new_path, get_option("{$size}_size_w"), get_option("{$size}_size_h"), $crop );

The fix to this is to make the wp_save_image function aware of custom sizes in the $_wp_additional_image_sizes global, and to use the options data as a fallback (for the built in sizes).

Patch for trunk attached. Tested on trunk install with no obvious side effects. Patched version correctly creates the custom image sizes with the applied editing changes, and deletes them properly when the image is deleted in the admin. Meta data thus created looks correct as well.

Attachments (4)

19889.diff (1.4 KB) - added by Otto42 2 years ago.
Change image-edit.php to account for additional sizes on save
19889.2.diff (1.3 KB) - added by SergeyBiryukov 21 months ago.
19889.3.diff (1.3 KB) - added by SergeyBiryukov 18 months ago.
19889.4.diff (1.4 KB) - added by SergeyBiryukov 8 months ago.

Download all attachments as: .zip

Change History (55)

Otto422 years ago

Change image-edit.php to account for additional sizes on save

comment:1 skithund2 years ago

  • Cc toni.viemero@… added

comment:2 SergeyBiryukov2 years ago

  • Keywords has-patch added

comment:3 studiograsshopper2 years ago

  • Cc studiograsshopper added

comment:4 dreamwhisper2 years ago

  • Cc dreamwhisper added

SergeyBiryukov21 months ago

comment:5 SergeyBiryukov21 months ago

  • Milestone changed from Awaiting Review to 3.5

[18996] (for #17475) introduced a bug causing custom image sizes to be removed from metadata when saving the edited image (as described in #21457).

19889.2.diff is a patch combined with the one from #21457.

comment:6 prettyboymp21 months ago

  • Cc mpretty@… added

comment:7 scribu19 months ago

  • Keywords needs-refresh close added

Patch is stale. Also, #22100 provides a more holistic approach.

comment:8 scribu18 months ago

  • Milestone 3.5 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #22100.

SergeyBiryukov18 months ago

comment:9 SergeyBiryukov18 months ago

  • Keywords needs-refresh close removed
  • Milestone set to 3.5
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#22100 is an enhancement, so not exactly a duplicate.

Given the attention to the media revamp, I guess this should be fixed in 3.5. Refreshed the patch.

comment:10 nacin18 months ago

  • Keywords close added

I don't know if this is a good thing to fix at this point. Image editing is something that largely missed the boat for 3.5, both from an API perspective (WP_Image_Editor aside) and from the UI perspective. Fixing this with #22100 — and a load of other tickets relating to cropping/editing abilities — makes sense.

comment:11 markoheijnen18 months ago

I think it's better to fix all of these issues in 3.6. I would love to see a better workflow in 3.6 where you can select the images it should apply too.

comment:12 nacin18 months ago

  • Milestone changed from 3.5 to Future Release

comment:13 helen16 months ago

#22990 was marked as a duplicate.

comment:14 helen16 months ago

  • Keywords close removed

comment:15 follow-up: anatolbroder16 months ago

Let’s talk about the responsive challenge. Imagine we need 3 different image ratios.

  1. Thumbnail (1:1)
  2. Medium (1.5:1)
  3. Large (3:1)

The most common transformation is the crop. I fact we must crop the original image manually, because the automatic crop function would just cut out the middle part of the original. This is an artistic decision. So our edit list would look like:

  • All sizes
  • Thumbnail
  • Medium
  • Large

But for every ratio we need multiple versions. Think on different screen widths and resolutions (mdpi, hdpi, xhdpi, …). So if i. e. we crop the medium ratio, we want a 480 × 320, a 720 × 480, and a 960 × 640 version of it. In fact, we register all the medium versions, but we need to crop every ratio only once.

  • Medium
    • 480 × 320
    • 720 × 480
    • 960 × 640

There are two solutions.

Sizes

The naive one is a check list (☒) with all the registered sizes.

  • All sizes
  • Thumbnail
    • 160 × 160
    • 240 × 240
    • 320 × 320
  • Medium
    • 480 × 320
    • 720 × 480
    • 960 × 640
  • Large
    • 1440 × 480
    • 2160 × 720
    • 2880 × 960

I think you can implement that one fast, till 3.6. Me, the editors of my magazine and Jon Kristian would be happy with that.

Shapes

The smart one is to go with ratios or shapes. We should not define sizes but shapes. (Call it whatever you want, my English is too bad.) The medium shape would be a group of sizes.

So we can introduce an add_image_shape function you can define the shape ratio, minimal size, etc.

add_image_shape( <name>, <ratio>, <min_width>, <max_width>, <crop>);

Then if we add a size with new_add_image_size, we could pass the defined shape through the parameter.

new_add_image_size( <name>, <shape>, <width>);

The edit list would look like.

  • All shapes
  • Thumbnail
  • Medium
  • Large

This is a different way to think. Please read the latest posts of Responsive Images Community Group if you don’t understand what I’m talking about. Maybe a new ticket is a better place to discuss that way.

comment:16 follow-up: markoheijnen16 months ago

The part for every ratio we need multiple versions isn't inside the scope of this ticket and is also something WordPress core shouldn't do. There still a lot of discussions still going and almost no one is using it.

comment:17 in reply to: ↑ 15 SergeyBiryukov16 months ago

Replying to anatolbroder:

I fact we must crop the original image manually, because the automatic crop function would just cut out the middle part of the original.

Related: #19393, #21810

Maybe a new ticket is a better place to discuss that way.

Indeed.

comment:18 in reply to: ↑ 16 anatolbroder16 months ago

  • Cc anatolbroder added

Replying to markoheijnen:

The part for every ratio we need multiple versions isn't inside the scope of this ticket and is also something WordPress core shouldn't do.

So is there another way for creating multiple resolutions of the same size right after the image manipulation? Can you provide a hook I can run to create multiple descendants from the manually edited image?

If I can only crop manually the mdpi version, but the hdpi and xhdpi versions still auto-cropped, the solution is not acceptable.

Last edited 16 months ago by anatolbroder (previous) (diff)

comment:19 markoheijnen16 months ago

This isn't something to discuss on this ticket or on trac. You should discuss this on the hackers mailinglist.
If you are creative you should read http://make.wordpress.org/core/2012/12/06/wp_image_editor-is-incoming/

comment:20 SergeyBiryukov16 months ago

  • Milestone changed from Future Release to 3.6

The issue here is not just that custom image sizes are not updated when editing, it's that they disappear from metadata when saving the edited image.

This causes the full size to be used everywhere instead of the custom size, as mentioned in #21457.

I guess #22985 is a similar issue, exacerbated by the fact that default image sizes disappear from metadata as well in 3.5.

comment:21 follow-up: markoheijnen16 months ago

It did cause disappearance in 3.4 and also in 3.5 because of the bug described in #22985. If that is fixed the data wil exists since we don't unset data anymore at that part of code.

comment:22 in reply to: ↑ 21 SergeyBiryukov16 months ago

Replying to markoheijnen:

If that is fixed the data wil exists since we don't unset data anymore at that part of code.

Confirmed that ticket:22985:22985.diff fixes the disappearance.

So, what's left is actually applying the changes to all sizes.

comment:23 markoheijnen16 months ago

#23145 was marked as a duplicate.

comment:24 goto1015 months ago

  • Cc dromsey@… added

comment:25 sabreuse15 months ago

  • Cc sabreuse added

comment:26 markoheijnen15 months ago

#23274 was marked as a duplicate.

comment:27 SergeyBiryukov13 months ago

#23860 was marked as a duplicate.

comment:28 follow-up: jeremyclarke12 months ago

  • Cc jer@… added

+1 to make this a priority. What are we supposed to do in the meantime? Does someone have a plugin hack that makes the add_image_size sizes be re-rendered when an image is cropped?

comment:29 rereradu11 months ago

  • Cc rereradu added

comment:30 SergeyBiryukov10 months ago

#24632 was marked as a duplicate.

comment:31 TigerDE210 months ago

  • Cc tigerde2@… added

comment:32 markoheijnen9 months ago

#24717 was marked as a duplicate.

comment:33 nacin9 months ago

  • Milestone changed from 3.6 to Future Release

Not new — maybe 3.7.

comment:34 SergeyBiryukov9 months ago

  • Version changed from 3.3.1 to 3.3

comment:35 mau9 months ago

  • Cc ngomau@… added

comment:36 tollmanz8 months ago

  • Cc tollmanz@… added

SergeyBiryukov8 months ago

comment:37 SergeyBiryukov8 months ago

  • Milestone changed from Future Release to 3.7

Refreshed for 3.7.

comment:38 nacin7 months ago

  • Milestone changed from 3.7 to 3.8

I like this in general, but this needs to happen at the beginning of a cycle.

comment:39 nacin7 months ago

Also, does this change UI expectations?

comment:40 Otto427 months ago

I think the latest patch for this echoes the original intent of the ticket, and doesn't break anything in any obvious enough ways to matter. The goal of this ticket was to have the image editor create the expected image sizes when the image editor is used, even when image sizes are added by the theme or plugins. If the theme or plugins are adding sizes, then editing an image should produce correct and expected results with those added sizes.

This is still a valid bugfix, as opposed to some new feature. Later additions may improve or enhance this, but it's still in the realm of a problem with how the image editor does not recognize additional sizes.

comment:41 richardmtl5 months ago

  • Cc richard@… added

comment:42 wonderboymusic5 months ago

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

comment:43 in reply to: ↑ 28 ejdanderson4 months ago

Replying to jeremyclarke:

+1 to make this a priority. What are we supposed to do in the meantime? Does someone have a plugin hack that makes the add_image_size sizes be re-rendered when an image is cropped?

The current workaround is to just filter the height/width options as they're requested. It attempts to process the image sizes but the values for their height and width are empty.

comment:44 markoheijnen4 months ago

#24997 was marked as a duplicate.

comment:45 markoheijnen4 months ago

#16203 was marked as a duplicate.

comment:46 SergeyBiryukov4 months ago

  • Milestone changed from Future Release to 3.9

comment:47 lkraav4 months ago

  • Cc leho@… added

comment:48 kraftner4 months ago

  • Cc thomas@… added

comment:49 ircbot5 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by DH-Shredder. View the logs.

comment:50 DH-Shredder5 weeks ago

If we're going to do this, I'd argue we should land it right away.
If it waits until beta 2+, we should likely leave this alone until next time around.

comment:51 wonderboymusic5 weeks ago

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

In 27522:

The Image Editor should apply changes to custom image sizes by checking $_wp_additional_image_sizes for $size in wp_save_image() before arbitrarily going to the options table.

Props Otto42, SergeyBiryukov.
Fixes #19889.

Note: See TracTickets for help on using tickets.