Image Editing Cropping is not working properly

[andy786]

If any image uploaded in Media is being edited and cropped and then saved. It does get saved. But the final image which gets displayed after the process is the original image itself. But the correct part should be that the final image should always appear as the edited image.

These issues appear when we crop any image two or three times.

[andy786]

Screen recorded video of the issue of image cropping

[antpb]

confirmed seeing this behavior on my end. Happens when you crop an image and get to the attach details page. It uses the old crop. Even closing the modal and opening the image again had no luck.

[sanketchodavadiya]

@antpb I have debugged this issue and below is my finding.

This issue happens if we crop below 1024px x 1024px(Large image size).
So, below large images are being regenerated with the crop but bigger than that images are untouched.

In core file media-template.php ​https://github.com/WordPress/WordPress/blob/ca76abd7e838b5760bc010d4ad3a38e693696667/wp-includes/media-template.php#L360-L361
Large size image is being shown when image is open in Attachment details popup that never cropped.

Also, while cropping when make_subsize(...) > _resize(...) or resize(...) function run it throws WP_Error error_getting_dimensions.

[antonvlasenko]

I'm able to reproduce this bug.
I'm working on a patch and I will submit it as soon as it's ready.

Trac ticket: https://core.trac.wordpress.org/ticket/55070

[antonvlasenko]

Steps to test ​this PR:

1. Upload a relatively large image to the Media Gallery (at least 2000x 1500 pixels). It's just easier to test this issue with a bigger image.
2. Click on the image.
3. Click on the "Edit Image" button below the image.
4. Crop the image (using the "Crop" button above the image).
5. A cropped image should be about 400 x 400 pixels in size.
6. Save the cropped image (click on the "Save" button).
7. You should be able to see the cropped image. Before the fix, WordPress would display the original image and not the cropped one.
8. [Additional step] You can crop the image again and observe that it's displayed correctly.

Steps to test this PR: https://core.trac.wordpress.org/ticket/55070#comment:6

[ironprogrammer]

Test Report

​PR 2489 LGTM! 👍🏻

Env

• WordPress 6.0-alpha-52448-src
• Safari 15.4
• Chrome 100.0.4896.60
• macOS 12.3.1 (Monterey)
• Gutenberg DISABLED 🔴

Steps to Test

1. Under Settings > Media Settings, check the currently configured "Large size" image dimensions (default is 1024x1024).
2. Upload a very large image to Media > Media Library (anything over the configured "Large size" above should work).
3. After upload, with the selected image, click the Edit Image button under the preview.
4. Click the Crop button and either adjust the crop using the handles overlay on the image, or by entering new dimensions in the IMAGE CROP > Selection sidebar. Set the crop to smaller than the site's configured "Large size", and click Crop again to apply the change. Observe that the image displayed refreshes to the cropped version. ✅
5. Click the Save button. Observe that the image refreshes and shows the original un-cropped version. ❌
6. 🛠 Apply the patch, then refresh the Media Library page.
7. Restore the original un-cropped image by clicking Edit Image, then expanding the RESTORE ORIGINAL IMAGE sidebar and clicking Restore image. (Alternatively, upload a new very large image to work with.)
8. ℹ️ Close the image viewer and re-select the image to re-open it (this is necessary because the crop tool does not reset to the original dimensions after being restored.)
9. Click the Crop button and either adjust the crop using the handles overlay on the image, or by entering new dimensions in the IMAGE CROP > Selection sidebar. Set the crop to smaller than the site's configured "Large size", and click Crop again to apply the change. Observe that the image displayed refreshes to the cropped version. ✅
10. Click the Save button. Observe that the image refreshes and shows the cropped version. ✅

Expected Results (✅)

• After cropping a large image to be smaller than the site's configured "Large size", the image preview displays the cropped image. ✅

Additional Notes

The above tests were performed with the "Large size" configured to both 1024x1024 and 1200x1200.

Screenshots

Crop applied to large image (Step 9).

Cropped image displayed in preview (Step 10).

[costdev]

With 6.0 RC1 starting soon, I'm moving this to the 6.1 milestone.

[adamsilverstein]

@antonvlasenko / @andy786 - thanks for the bug report and patch...

to clarify, this is (only) about the image that is displayed in the media library in the "edit image" screen, right?

when you insert the cropped image into a post for example, does the correct cropped version get used?

worth noting that WordPress keeps the original image when making these crops, so displaying the original un-cropped image might be considered expected behavior (if consistent).

[ironprogrammer]

Replying to adamsilverstein:

to clarify, this is (only) about the image that is displayed in the media library in the "edit image" screen, right?

👍🏻 Yes, after the crop has been applied and saved.

when you insert the cropped image into a post for example, does the correct cropped version get used?

👍🏻 Yes, the cropped image is what is ultimately inserted into the post.

worth noting that WordPress keeps the original image when making these crops, so displaying the original un-cropped image might be considered expected behavior (if consistent).

The issue was raised due to an inconsistency that was discovered. When a user crops an image that is larger than the configured "Large image" media size, the preview of the image accurately reflects the crop applied -- but under the reported conditions, the preview does not reflect the crop.

In terms of expected behavior, I believe we're following that when a user is ready to insert the image, that the preview displayed is an accurate representation of what will be inserted into the post.

Thank you for highlighting the fact that this is a display issue, but does not affect the final image or post. 🙌🏻

[antonvlasenko]

@adamsilverstein

to clarify, this is (only) about the image that is displayed in the media library in the "edit image" screen, right?

Yes, that's correct. It's only about the image that's displayed in the media library.

when you insert the cropped image into a post for example, does the correct cropped version get used?

Honestly, I don't know which version of the image is used when a user inserts an image into a post.
I've never investigated this because it's out of the scope of this trac ticket.
IMO If there is an issue with post images, there must be a separate trac issue and a new patch.

[antonvlasenko]

@ironprogrammer

Yes, the cropped image is what is ultimately inserted into the post.

I'd like to clarify this, so there is no misunderstanding.
My PR doesn't affect which version of the image gets inserted into the post.
That logic has remained unchanged as this trac ticket doesn't report any issues with displaying post images.

[antpb]

The PR for this ticket seems to still be in progress. With Beta 1 in an hour I am going to move this to 6.2 to keep it on our radar for the next release. Feel free to move it back to 6.1 if it is feasible for this release!

[antonvlasenko]

@antpb Thanks for the triage.
In terms of development, this PR is complete.
It should be decided whether this approach to fixing the bug suits the community.

Trac ticket: https://core.trac.wordpress.org/ticket/55070

[adamsilverstein]

Hey @antonvlasenko - thanks for your patience here.

I reviewed your PR again and while it does fix the display issue, it feels overly complex.

The underlying issue is the large image isn't cropped in this scenario and the code currently uses the large image. I came up with an alternate approach in 55070.diff​ (also at ​https://github.com/WordPress/wordpress-develop/pull/3985) - switch to using the full sized image for the edit modal preview (instead of large).

In my testing this resolves the issue you raised and the full size always gets the crop. I also tested rotating and after saving the preview was consistently correct. This does cause a potentially larger image to load, however this is still capped at the large image threshold and is probably fine in the edit context regardless - the image will actually show with higher fidelity on high dpi screens if we use the full size.

Could you please review and verify this resolves the issue you reported. Also welcome any feedback on the approach of using the full size image here instead of large.

[antonvlasenko]

Thanks for the review, @adamsilverstein.

I reviewed your PR again and while it does fix the display issue, it feels overly complex.

I'm afraid I fail to understand how this is overly complex.
My solution parses "crop ID" from both "large" and "full" images.
If these crop IDs don't match (meaning that the Large image is outdated), it replaces the Large image with the Full image.
The advantage of such an approach is that it allows WordPress to still use the Large image (where applicable).

Honestly, I'm not entirely happy with either of the proposed PRs, as both sweep the underlying issue under the carpet.
I think the proper solution will be to delete the Large image if the Full image has lesser dimensions than the Large image.
However, I've no idea if implementing it will break backward compatibility.
I think I will be able to submit a new PR sometime this week or at least investigate if it's possible to fix it this way.
What is your opinion on that?

The underlying issue is the large image isn't cropped in this scenario and the code currently uses the large image.

Yes, ​I understand it.

Could you please review and verify this resolves the issue you reported.

I've reviewed your PR. Thanks.

[antonvlasenko]

So, this issue can be fixed by replacing
$meta['sizes'] = array_merge( $meta['sizes'], $img->multi_resize( $_sizes ) );
with
$meta['sizes'] = $img->multi_resize( $_sizes ) );
in src/wp-admin/includes/image-edit.php.
But it's impossible to fix it this way because of https://core.trac.wordpress.org/ticket/22985.
It will break BC.
I've approved your PR, @adamsilverstein.
I will add my test report soon.

@sanketchodavadiya

Also, while cropping when make_subsize(...) > _resize(...) or resize(...) function run it throws WP_Error error_getting_dimensions.

IMO this is fine, as image_resize_dimensions() cannot calculate dimensions for a resized image if it's larger than the original image.

[antonvlasenko]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3985

Environment

• OS: macOS 13.1
• Web Server: Apache 2
• PHP: 8.1.14
• WordPress: 6.2-alpha-54642-src
• Browser: Safari 16.2
• Theme: Twenty Twenty-Three
• No Active Plugins

Steps

1. Upload a relatively large image to the Media Gallery (at least 2000x 1500 pixels). It's just easier to test this issue with a bigger image.
2. Click on the image.
3. Click on the "Edit Image" button below the image.
4. Crop the image (using the "Crop" button above the image).
5. A cropped image should be about 400 x 400 pixels in size.
6. Save the cropped image (click on the "Save" button).

Actual Results

7. ✅ WordPress displays the cropped image (and not the old version).

[costdev]

@adamsilverstein As we're approaching 6.2 Beta 4, can you take a look at the latest comments in the next couple of days and see if this one is landable soon or if it should be moved to Future Release? Thanks!

---

• Changing Severity from critical to normal, as this is an issue with which image is displayed, not with the cropping functionality itself.

[adamsilverstein]

We discussed this in the media chat today and reviewed the latest patch. I'll get this committed.

[hellofromTonya]

@adamsilverstein Beta 4, the last scheduled beta for this release, just happened. Is this fix essential for 6.2?

[costdev]

With Beta 4 released, and @adamsilverstein confirming via Slack that this isn't a critical fix for 6.2, moving this ticket to 6.3.

[adamsilverstein]

Since we are doing a beta 5 tomorrow I'm going to go ahead and commit this bug fix.

[adamsilverstein]

In 55470:

Media: improve display of cropped image in media editor.

Correctly display edits after you crop an image and return to the attachments page.

Prefer the full over the large size image on the edit image screen.

Props andy786, antpb, sanketchodavadiya, antonvlasenko, ironprogrammer.
Fixes #55070.

Closed in favor of ​https://github.com/WordPress/wordpress-develop/pull/3985.