Make WordPress Core

Opened 9 years ago

Closed 4 months ago

#36441 closed defect (bug) (fixed)

Customizer: when setting header image, site icon and logo, do not force the user to crop the image if cropping is not required

Reported by: azaozz's profile azaozz Owned by: jorbin's profile jorbin
Milestone: 6.7 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: needs-testing has-patch
Focuses: template Cc:

Description

This is a long time standing UX "weirdness". When using "the Cropper" in the Customizer, it forces the user to crop the selected image even when it only requires scaling. To reproduce:

  • Open the Customizer and go to setting a site icon.
  • Upload an image that is square, I tested with 800 x 800px. We require 512 x 512px image for site icon, so the uploaded image only needs to be scaled. However on the next screen there is no "Skip cropping" button. If the users wants to use the whole image, they will have to "crop" it at 100% x 100% to get to the next step.

Attachments (1)

36441.0.diff (1.3 KB) - added by westonruter 8 years ago.

Download all attachments as: .zip

Change History (31)

#1 @azaozz
9 years ago

  • Summary changed from Customizer: when setting heared image, site icon and logo, do not force the user to crop the image if cropping is not required to Customizer: when setting header image, site icon and logo, do not force the user to crop the image if cropping is not required

#2 @westonruter
9 years ago

For missing condition to allow cropping, see 36412.mustBeCropped.diff.

#3 @melchoyce
8 years ago

@westonruter Was this fixed?

#4 @westonruter
8 years ago

  • Keywords needs-patch added
  • Milestone changed from Future Release to 4.7.4

@melchoyce it doesn't appear so. I just tried selecting a 800x800 image for the site icon and I wasn't able to skip cropping.

This would seem like quick win territory.

@westonruter
8 years ago

#5 @westonruter
8 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

See 36441.0.diff.

The key change here is that if the aspect ratio of the selected image is the same as the required aspect ratio, then it allows cropping to be skipped.

What I'm not certain about now is, if you skip cropping, whether the theme will be able to get the best image size when displaying. For example, the_custom_logo() will currently use the full size, which here is wrong. Instead it should be asking for the array( $width, $height ) as defined when you added custom-logo theme support. This assumes that a theme has done an add_image_size() call with the same dimensions as the custom logo is defined to need. Presumably this would then allow wp_get_attachment_image() to get the cropped image size as opposed to getting the full size. We wouldn't want the full high res image to always be used if the user skipped cropping.

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


8 years ago

#7 @westonruter
8 years ago

  • Keywords needs-patch added; has-patch removed

I think 36441.0.diff will ultimately need to be augmented further to ensure that an image size variation is created at the requested size, even if cropping was skipped due to the aspect ratio being a match. With that I think this would need to be worked alongside #21819.

@joemcgill thoughts?

#8 @westonruter
8 years ago

  • Milestone changed from 4.7.4 to 4.8

Scope seems larger than initially thought. Milestoning for 4.8 alongside #21819.

This ticket was mentioned in Slack in #core-media by melchoyce. View the logs.


8 years ago

#10 @westonruter
8 years ago

  • Milestone changed from 4.8 to Future Release

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


8 years ago

#13 @joemcgill
7 years ago

  • Milestone changed from Future Release to 4.9

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


7 years ago

#15 @westonruter
7 years ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

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


7 years ago

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


7 years ago

#18 @melchoyce
7 years ago

  • Milestone changed from 4.9 to 4.9.1

#19 @johnbillion
7 years ago

  • Milestone changed from 4.9.1 to 5.0

#20 @pento
6 years ago

  • Milestone changed from 5.0 to 5.1

#21 @joe12345
6 years ago

my theme is not showing my post like it always did in the past, what is worng

#22 @pento
6 years ago

  • Milestone changed from 5.1 to Future Release

This patch needs testing and further work.

#23 @celloexpressions
4 years ago

  • Focuses template added
  • Version set to 4.3

To summarize the comments above, the custom logo frontend functions currently only work with media attachments that have been specifically cropped to the 512px size.

The most likely solution that I can come up with is to still "crop" the image behind the scenes (ie, create a copy of the attachment with 512px as the "full" size and then generate all of the site-icon-specific sizes) but to allow users to skip the step of selecting a cropzone. It should also be possible to "skip cropping" on non-square images and let WordPress auto-crop to square similar to the default thumbnail image size.

#24 @bbceg
3 years ago

Just to add, this forum post: [Site Icon cropped image created when uploaded image is 512×512]https://wordpress.org/support/topic/site-icon-cropped-image-created-when-uploaded-image-is-512x512/ also seems relevant here.

To reiterate, if I upload a 512x512 site icon WordPress still creates a cropped version. This results in two virtually identical images in the image library (one prefixed with 'cropped-') which causes confusion and unnecessary clutter.

As a side note (almost certainly off-topic) WordPress also creates the "cropped" image with more than twice the file size (39KB instead of 15KB) of the original image. This appears to be a more general problem with image resizing in WordPress but it seemed worth mentioning. Not sure if there's a separate issue somewhere that picks this up.

Hope that's helpful.

#25 @scabuster
3 years ago

"To reiterate, if I upload a 512x512 site icon WordPress still creates a cropped version. This results in two virtually identical images in the image library (one prefixed with 'cropped-') which causes confusion and unnecessary clutter."

"As a side note (almost certainly off-topic) WordPress also creates the "cropped" image with more than twice the file size (39KB instead of 15KB) of the original image."

A 6 year old bug of any nature is not acceptable. Reading through the suggested 'fixes' here gives the impression of a lazy cush and non-competitive wordpress culture. Lets go lets go - As if your integrity depended on it, as if your discretion were razor sharp, as if you were a completionist.

Yes it affects SEO. No it should not add multiple new files. No it should not add the prefix 'cropped' to any files. No it should not increase the file size. Even with 512px square image upload, it still crops them and adds multiple new files. Yes it should give the option to not crop. If not cropped then yes it should pull the uploaded image first as it is likely to be known by the site owner that it is already the smallest file and best quality over any of the wordpress generated duplicates. No it doesn't matter if it pulls the full 512px square image for a mobile device if that is smaller in kb than the 32x32px wordpress generated copy.

*

#26 @pressthemes1
10 months ago

I agree, why no just add a parameter to disable cropping all together

This ticket was mentioned in PR #7089 on WordPress/wordpress-develop by @nirajgirixd.


7 months ago
#27

  • Keywords has-patch added; needs-patch removed

## Description

This PR refactors the cropping logic in the image cropping tool to enable the Skip cropping option for images that have the required aspect ratio.

## Screencast

https://github.com/user-attachments/assets/4249afbf-54fd-44e3-a0e6-c37b1ed4187d

## Trac ticket: https://core.trac.wordpress.org/ticket/36441

#28 @jorbin
7 months ago

  • Milestone changed from Future Release to 6.7
  • Owner set to jorbin
  • Priority changed from high to normal
  • Status changed from new to assigned

Lowering the priority as this is now a long-standing issue; milestoning it so that it doesn't stay an issue.

#29 @nirajgirixd
5 months ago

Could someone please review this PR and either approve it or provide feedback?

cc: @azaozz, @westonruter, @melchoyce, @joemcgill, @pento, @jorbin

#30 @jorbin
4 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 59197:

Customizer: Do not force users to go through the cropping flow if the image is the correct ratio.

If a user uploads an 800x800 image and a 512x512 image is required, then they should be allowed to skip cropping. This still creates the correct crop behind the scenes but simplifies the flow a bit for users.

Props nirajgirixd, celloexpressions, westonruter, azaozz, jorbin.
Fixes #36441.

Note: See TracTickets for help on using tickets.