PHP deprecation error in WP_Image_Editor_Imagick

[skithund]

When cropping and saving an image it throws an deprecation error when WP_Image_Editor_Imagick is ​converting floats to ints.

Implicit conversion from float 1112.7466666666667 to int loses precision

Backtrace originally in JSON format from our central logging, hence funny formatting:

{
  "file": "/srv/www/site/public_html/wp-admin/admin-ajax.php",
  "line": 188,
  "function": "do_action",
  "args": [
    "wp_ajax_image-editor"
  ]
},
{
  "file": "/srv/www/site/public_html/wp-includes/plugin.php",
  "line": 517,
  "function": "do_action",
  "class": "WP_Hook",
  "object": {
    "callbacks": {
      "1": {
        "wp_ajax_image_editor": {
          "function": "wp_ajax_image_editor",
          "accepted_args": 1
        }
      }
    }
  },
  "type": "->",
  "args": [
    [
      ""
    ]
  ]
},
{
  "file": "/srv/www/site/public_html/wp-includes/class-wp-hook.php",
  "line": 334,
  "function": "apply_filters",
  "class": "WP_Hook",
  "object": {
    "callbacks": {
      "1": {
        "wp_ajax_image_editor": {
          "function": "wp_ajax_image_editor",
          "accepted_args": 1
        }
      }
    }
  },
  "type": "->",
  "args": [
    "",
    [
      ""
    ]
  ]
},
{
  "file": "/srv/www/site/public_html/wp-includes/class-wp-hook.php",
  "line": 310,
  "function": "wp_ajax_image_editor",
  "args": [
    ""
  ]
},
{
  "file": "/srv/www/site/public_html/wp-admin/includes/ajax-actions.php",
  "line": 2683,
  "function": "wp_save_image",
  "args": [
    4281
  ]
},
{
  "file": "/srv/www/site/public_html/wp-admin/includes/image-edit.php",
  "line": 930,
  "function": "image_edit_apply_changes",
  "args": [
    {},
    [
      {
        "type": "crop",
        "sel": {
          "x": 0,
          "y": 45,
          "w": 475,
          "h": 326
        }
      }
    ]
  ]
},
{
  "file": "/srv/www/site/public_html/wp-admin/includes/image-edit.php",
  "line": 722,
  "function": "crop",
  "class": "WP_Image_Editor_Imagick",
  "object": {},
  "type": "->",
  "args": [
    0,
    153.60000000000002,
    1621.3333333333335,
    1112.7466666666667
  ]
},
{
  "file": "/srv/www/site/public_html/wp-includes/class-wp-image-editor-imagick.php",
  "line": 606,
  "function": "setImagePage",
  "class": "Imagick",
  "object": {},
  "type": "->",
  "args": [
    1621.3333333333335,
    1112.7466666666667,
    0,
    0
  ]
}

[skithund]

Couldn't add php81 keyword

This pull request removes the PHP deprecated errors (like Implicit conversion from float 1112.7466666666667 to int loses precision) in WP_Image_Editor_Imagick/WP_Image_Editor crop method.

While crop expects integers, integers are not always passed to it.
So the goal is to cast the params to cast (int) before passing them to the method.

Tested on:

• PHP 8.1
• PHP 8.0

How to reproduce the error:

1. Go to /wp-admin/media-new.php and upload an image
2. "Edit" the image
3. Then do a crop with the mouse and "Apply crop"
4. See the Implicit conversion from float in the debug log

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

[mai21]

• Reproducible using the steps mentioned here https://core.trac.wordpress.org/ticket/59782#comment:3

• Tested the patch on WP 6.4.3 and 6.5-alpha-57529, with PHP 8.3.1, and can confirm that it's fixed.

Steps
1- Go to /wp-admin/media-new.php and upload an image
2- "Edit" the image
3- Then do a crop with the mouse and "Apply crop"
4- See the Implicit conversion from float in the debug log

With Patch
No 'PHP Deprecated: Implicit conversion from float xxx.xx' in debug.log

General note
After cropping the image, the size displayed on the image edit page is still that of the original image, not the newly generated image. However, if the image was used on a page, the new size will be used. => Not related to the patch, not sure if we have a ticket about this.

General note => Steps:
1- Upload an image
2- Open media library -> edit image
3- Notice the file size displayed on image data at the most right
4- Crop the image and click apply crop then save edits
5- Click the update button
6- Notice the file size => didn't change while the dimensions changed. (if we downloaded the new file, the size is different than that displayed on the edit image page)

Expected:
File size is updated as same as dimensions since the size is already changed

[antpb]

Moving this into 6.6 to get attention on this for the next release. patch and testing are looking good!

[joedolson]

#61375 was marked as a duplicate.

[joedolson]

#61375 was marked as a duplicate.

[joedolson]

Refreshed patch

[joedolson]

Patch is straightforward, has unit tests, and has been tested on PHP8, 8.1, and 8.3.

I tested on PHP 8.2, and confirmed that as well; just to be thorough.

Marking for commit.

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

[joedolson]

In 58457:

Media: Fix implicit conversion from float to int in image cropping.

Cast crop values to integers to prevent PHP error caused by implicit conversion from float to int values when cropping images using ImageMagick.

Props skithund, mai21, nicomollet, amanias1977, joedolson.
Fixes #59782.

In r58457

In r58457.

[andrewserong]

Hi all, I'm just reopening this ticket as the fix appears to have caused a regression in WP 6.6 RC 1 where the image cropping tools in the block editor are no longer working. I've shared reproduction steps over in ​https://github.com/WordPress/gutenberg/issues/62855

While testing locally, reverting the change in ​https://github.com/WordPress/wordpress-develop/pull/6876 appears to get the cropping tools working again. Is it possible to either revert or update this fix for 6.6 to fix the block editor cropping tools?

## 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/59782

[jrf]

Replying to andrewserong:

Hi all, I'm just reopening this ticket as the fix appears to have caused a regression in WP 6.6 RC 1 where the image cropping tools in the block editor are no longer working. I've shared reproduction steps over in ​https://github.com/WordPress/gutenberg/issues/62855

While testing locally, reverting the change in ​https://github.com/WordPress/wordpress-develop/pull/6876 appears to get the cropping tools working again. Is it possible to either revert or update this fix for 6.6 to fix the block editor cropping tools?

Just looked over the tickets. The fix here is correct and should stay in. The fix exposed an unrelated, pre-existing logic bug in the REST API, which affects GB, which highlights a need for better automated tests for that area of the REST API as that bug should have been caught long ago.

I suggest closing this ticket again as fixed and treating the bug in the REST API as a stand-alone bug as it could have been reproduced prior to this fix going in if passed the right test data.

[andrewserong]

Thanks for the feedback @jrf, I have opened a separate ticket in #61514

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.

[SergeyBiryukov]

In 58612:

REST API: Correct image cropping tools in the block editor.

As of [58457], the width and height cropping values are cast to an integer before the comparison to see if the target width and height differ from the original width and height.

Since they are now integers, 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 integers, the condition that allows a cropping to occur needs to be updated. If either width or height is different from the source image, then a crop should be allowed.

Follow-up to [50124], [58457].

Props andrewserong, jrf, kevin940726.
Fixes #61514. See #59782.

Thanks for the PR! Merged in r58612.

[davidbaumwald]

In 58692:

REST API: Correct image cropping tools in the block editor.

As of [58457], the width and height cropping values are cast to an integer before the comparison to see if the target width and height differ from the original width and height.
Since they are now integers, 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 integers, the condition that allows a cropping to occur needs to be updated. If either width or height is different from the source image, then a crop should be allowed.

Follow-up to [50124], [58457].

Reviewed by davidbaumwald.
Merges [58612] to the 6.6 branch.

Props andrewserong, jrf, kevin940726.
Fixes #61514. See #59782.

[hellofromTonya]

Just looked over the tickets. The fix here is correct and should stay in. The fix exposed an unrelated, pre-existing logic bug in the REST API, which affects GB, which highlights a need for better automated tests for that area of the REST API as that bug should have been caught long ago.

I suggest closing this ticket again as fixed and treating the bug in the REST API as a stand-alone bug as it could have been reproduced prior to this fix going in if passed the right test data.

This ticket was reopened due to a regression. But as @jrf noted, the fix in this ticket did not cause the regression. The regression itself is now fixed in #61514 via changesets [58612] (on trunk) and [ ] (on the 6.6 branch).

Thus, I'm reclosing this ticket as fixed via [58457].