Make WordPress Core

Opened 5 weeks ago

Last modified 13 days ago

#54308 assigned defect (bug)

Customizer: Can't crop header images

Reported by: alshakero Owned by: alshakero
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Customize Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:


Quoting https://github.com/Automattic/wp-calypso/issues/52182

Steps to reproduce the behavior

  • Activate a theme that uses a custom header image:
  • Go to My Site > Appearance > Customize > Header Image
  • Click on Add new Image
  • Select and image and then click on Select and crop

What I expected to happen

To be able to crop the image to the correct header size and then save it to the header

What actually happened

When you get to the next screen, there is no cropping selection. Dragging across the image to create a selected section results in Drop image. Clicking on Crop results in the error {file name} There has been an error cropping your image. if there is a Skip Cropping option and you select it, it does not update the header image

There is a video of my test site here: d.pr/v/0kpjbn

Here is the diagnosis and proposed fix

imageAreaSelect breaks for non-integer image width

WordPress relies on imageAreaSelect in Customizer.

imageAreaSelect has a function called adjust, this functions is called on every change to the selection box. And it's also called on-load to select the entire image.

adjust has a check to see whether selection is outside the image boundaries, in which case it caps the selection values to image boundaries and re-renders.

This capping is done by using Math.min and Math.max, which return NaN if one of their operands is undefined.

When an image is loaded, we render the selection box, then we call adjust. If the image has a width of 400.6, its width is rounded to 401 and the selection box's width is set to that, when we check if the selection went outside the image if(selection.x2 > imgWidth) we get true, because the rounded value is indeed bigger than the original value. When we come to re-render by calling doResize, Math.min returns NaN and the selection box disappears.

A possible solution is to use floor instead of round, this way the check if(selection.x2 > imgWidth) won't return true for non-integer image width.

Another solution might be to check if any of the coordinates are undefined in the beginning of doResize. I tried both solutions and both work, but I prefer the first one, because the second one might hide bugs we don't know about and make it harder to debug them.

Change History (4)

This ticket was mentioned in PR #1774 on WordPress/wordpress-develop by alshakero.

5 weeks ago

  • Keywords has-patch added; needs-patch removed

#2 @yansern
5 weeks ago

Tested this on zoomed browser for both portrait & landscape images, and unzoomed browser on both portrait & landscape images. Resize handle is showing as expected!

This ticket was mentioned in Slack in #core by yansern. View the logs.

13 days ago

#4 @costdev
13 days ago

  • Keywords needs-testing added
Note: See TracTickets for help on using tickets.