WordPress image cropping is buggy

[Jossnaz]

see here

​http://wordpress.stackexchange.com/questions/187577/cropping-image-results-in-image-being-too-small?noredirect=1#comment273421_187577

text copied from there:

This is the wordpress version 4.2.2 powell

I crop 1500 x 1000 and then press on crop, then save.

Resulting image size:

1497 x 1000

I don't know, I somehow hoped that wordpress is able to crop correctly.

The original image can be found here:

​http://img.youtube.com/vi/zXgyoDAuaH0/maxresdefault.jpg

What I did is downsize to .... x 1000 then try to crop with selection manually entering 1500 x 1000

You can actually see the bug earlier, when you enter 1500 x 1000 and then click on the selection to drag it around (right, drag the selection, not change size) it results in the number 1500 falling down to 1498.

[rachelbaker]

I successfully reproduced this bug using the latest trunk from 4.5 and the provided image. After manually setting a crop size of 1500x1000 the image size then changed to 1497x998 once I pressed the crop button, or tried to move the selection area.

[Presskopp]

Related: #18275

[johnillo]

• Crop the image in 100% scale to avoid rounding errors.
• Don't recompute selection values when selection is dragged only.
• Fix the issue where the selection values are off after the image is scaled down or restored.
• Keep aspect ratio when holding the shift key.

[joedolson]

Confirmed still an issue.

[swissspidy]

Punting due to lack of activity & new patch.

[shailu25]

Patch Refreshed.

[shailu25]

Refreshed the Previous patch.

[sudipatel007]

Test Report

---

Description

Now, you can crop image using WordPress core functionality

Patch has been tested and working as expected.

Environment

• WordPress: 6.5.5-alpha-58404
• PHP: 8.1.23
• Server: nginx/1.16.0
• Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.23)
• Browser: Chrome 126.0.0.0 (macOS)
• Theme: Twenty Nineteen 2.8

Test Results

Crop image is working as expected.

Video - ​https://app.screencast.com/fIjJ5ziAjCcqg

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

[joedolson]

PR updates the patch to meet coding standards, correct a couple errors, & adjust some code styling for consistency.

Tested and confirmed this resolves the functional issue with cropping.

[joedolson]

In 58456:

Media: Fix admin image cropping calculations.

The admin image editor crop function introduced rounding errors by using a scaled image to calculate values. Fix uses the image at 100% scale for calculations. Also avoid recalculating selection when the selection position is changed, and prevent incorrect values after scaling or restoration.

Props Jossnaz, johnillo, shailu25, rachelbaker, sudipatel007, joedolson.
Fixes #32282.

In r58456

Hi @joedolson!

Recently, after this PR (commit 25e7dc6c10 and 4f175e167e), the e2e tests for image cropping in gutenberg started ​failing. I tested it manually and it seems like a legit bug. Here's a recording of the reproduction:

​https://github.com/WordPress/wordpress-develop/assets/7753001/6163efe2-600b-441f-9ed3-f554eb12eccc

Testing instructions:

1. Add an image block to a post and upload an image.
2. Select "Crop" in the block toolbar.
3. Select "Aspect ratio" in the block toolbar, then select an option from the dropdown list.
4. Hit "Apply" in the block toolbar.
5. Expect the image to be cropped, but it doesn't.

[andrewserong]

I'm just reopening this ticket as a bug has been flagged in the block editor's image cropping tools. @joedolson would it be worth reverting this change for 6.6, or is there another fix that could be done? Thanks!

[hmbashar]

Test Report

Patch tested: 32282.patch

Environment

• WordPress: 6.6-beta1-58337-src
• PHP: 8.3.8
• Server: nginx/1.25.4
• Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.8)
• Browser: Firefox 127.0
• OS: macOS
• Theme: Twenty Twenty-Four 1.1
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.1.0

Actual Results

1. ✅ Issue resolved with patch.

Check Video ​https://www.berrycast.com/conversations/ae908bc4-8b0c-5814-b85a-089851a11bee

[oglekler]

@audrasjb can you take a look at this one, please?

[oglekler]

This ticket is staying in the milestone due to the commit that most likely needs to be reverted.

[jorbin]

In 58571:

Media: unfix admin image cropping calculations.

[58456] introduced some failures to the automated test system that indicate this fix is incomplete.

See #32282.
Props hellofromtonya, audrasjb, andrewserong, kevin940726, oglekler.

[sabernhardt]

Moving to next milestone after commit was reverted

[andrewserong]

Now that 6.6 RC 1 is released, I've retested image cropping in the block editor, and it appears to still be failing. I've opened an issue in Github with reproduction steps: ​https://github.com/WordPress/gutenberg/issues/62855

As far as I can tell, 6.6 RC 1 included the above revert, and unfortunately I haven't been able to find anything obvious that could be the culprit. The values being sent to the media/edit REST API endpoint appear to be unchanged from the block editor side.

Are there any other recent server-side image editing changes that could have affected this?

[andrewserong]

Update: after testing locally, I think reverting a different (but similar) change from #59782 (​https://github.com/WordPress/wordpress-develop/pull/6876) seems to get the cropping working in the block editor again. I'll comment over on that ticket, too.

## What

Fixes cropping tools in the block editor. Related Gutenberg issue: ​https://github.com/WordPress/gutenberg/issues/62855

## Why

As of ​https://github.com/WordPress/wordpress-develop/commit/4f175e167ebd33d68ebe347fcc376f1342d9e873 / ​https://github.com/WordPress/wordpress-develop/pull/6876, the width and height values are rounded and cast to an int before the comparison to see if the target width and height differ from the original width and height.

Since they are now ints, it exposes a bug where the && of the if conditional meant that if you were only cropping in one dimension, the check wouldn't pass, and cropping would not occur.

In the block editor, the cropping tools are aspect ratio based, so one of the dimensions will always match that of the source image. Therefore, now that the values are cast as ints, we need to update the condition that allows a cropping to occur. In this case, I believe it should be || instead of &&. If either width or height is different from the source image, then we should allow a crop.

## How to test

1. Add an image block to a post in the block editor
2. Click on the Crop tool in the block toolbar
3. Select a Crop from the aspect ratio dropdown
4. Click Apply
5. The aspect ratio should be applied in the final image
6. Save and load the post on the site frontend and double check that the crop is correct

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

[joedolson]

Given the further research since the revert, it sounds like this change is actually fine, and can go back in at any time. If that's not how you see it, @andrewserong, let me know. Otherwise, I'm marking this for commit again.

[hmbashar]

Replying to jorbin:

In 58571:

Media: unfix admin image cropping calculations.

[58456] introduced some failures to the automated test system that indicate this fix is incomplete.

See #32282.
Props hellofromtonya, audrasjb, andrewserong, kevin940726, oglekler.

Hello @jorbin
I'm new to contributing, so I may have made a mistake. Can you explain why I didn't receive the props? If you clarify, I can ensure and follow the instructions in the future.

Thanks for the feedback @jrfnl! I've added a unit test, copying the existing one for image cropping, but updating it to include cropping on only one axis.

@jrfnl or @joedolson this PR is ready for another review and commit if it's all looking okay. It'd be great to get this fix in for the next 6.6 RC if we can.

Thanks for the PR! Merged in r58612.

[antpb]

Just leaving a note here, this ticket was reverted because it was thought that it was causing test failures. It actually wasn't but there was not enough time to commit again. This is clear for commit at any time for 6.7. Also if you commit, please ensure that @hmbashar gets props :)

[jorbin]

Replying to hmbashar:

Hello @jorbin
I'm new to contributing, so I may have made a mistake. Can you explain why I didn't receive the props? If you clarify, I can ensure and follow the instructions in the future.

It's my bad, sorry about that. SVN commits can't be edited after the fact, but I have made sure that you received credit on ​your WordPress.org profile.

[joedolson]

Updated @since reference

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

[joedolson]

In 58915:

Media: Fix admin image cropping calculations.

The admin image editor crop function introduced rounding errors by using a scaled image to calculate values. Fix uses the image at 100% scale for calculations. Also avoid recalculating selection when the selection position is changed, and prevent incorrect values after scaling or restoration.

Previously committed in [58456] and reverted in [58571]. The revert was due to a misattributed test failure.

Props Jossnaz, johnillo, shailu25, rachelbaker, sudipatel007, joedolson, kevin940726 , andrewserong, hmbashar.
Fixes #32282.

In r58915