WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 4 weeks ago

#19889 reopened defect (bug)

Image Editor doesn't apply changes to custom image sizes

Reported by: Otto42 Owned by:
Priority: normal Milestone: 3.6
Component: Media Version: 3.3.1
Severity: normal Keywords: has-patch
Cc: toni.viemero@…, studiograsshopper, dreamwhisper, mpretty@…, anatolbroder, dromsey@…, sabreuse, jer@…, rereradu

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 (3)

19889.diff (1.4 KB) - added by Otto42 17 months ago.
Change image-edit.php to account for additional sizes on save
19889.2.diff (1.3 KB) - added by SergeyBiryukov 11 months ago.
19889.3.diff (1.3 KB) - added by SergeyBiryukov 7 months ago.

Download all attachments as: .zip

Change History (32)

Otto4217 months ago

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

comment:1 skithund17 months ago

  • Cc toni.viemero@… added

comment:2 SergeyBiryukov16 months ago

  • Keywords has-patch added

comment:3 studiograsshopper14 months ago

  • Cc studiograsshopper added

comment:4 dreamwhisper14 months ago

  • Cc dreamwhisper added

SergeyBiryukov11 months ago

comment:5 SergeyBiryukov10 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 prettyboymp10 months ago

  • Cc mpretty@… added

comment:7 scribu9 months ago

  • Keywords needs-refresh close added

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

comment:8 scribu7 months ago

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

Duplicate of #22100.

SergeyBiryukov7 months ago

comment:9 SergeyBiryukov7 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 nacin7 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 markoheijnen7 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 nacin7 months ago

  • Milestone changed from 3.5 to Future Release

comment:13 helen6 months ago

#22990 was marked as a duplicate.

comment:14 helen6 months ago

  • Keywords close removed

comment:15 follow-up: anatolbroder6 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: markoheijnen6 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 SergeyBiryukov6 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 anatolbroder6 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 6 months ago by anatolbroder (previous) (diff)

comment:19 markoheijnen6 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 SergeyBiryukov6 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: markoheijnen6 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 SergeyBiryukov6 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 markoheijnen5 months ago

#23145 was marked as a duplicate.

comment:24 goto105 months ago

  • Cc dromsey@… added

comment:25 sabreuse5 months ago

  • Cc sabreuse added

comment:26 markoheijnen5 months ago

#23274 was marked as a duplicate.

comment:27 SergeyBiryukov3 months ago

#23860 was marked as a duplicate.

comment:28 jeremyclarke2 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 rereradu4 weeks ago

  • Cc rereradu added
Note: See TracTickets for help on using tickets.