JS error when uploading small image as Site Icon and electing to Skip Cropping

[truonght]

I select a _small_ image in Customize > Site identity > Site icon. After selecting the image, I choose “Skip Cropping” and then the modal changes to blank. I see an error in browse console:

Uncaught TypeError: Cannot read property 'url' of undefined

[westonruter]

@truonght Thanks for the report. I tried reproducing your steps, but I do not even see there being a “Skip Cropping” option after having selected an image.

[truonght]

Oh sorry. Now it work. I think problem is i didn't update 2015 theme :)

[truonght]

Oh. It still happens when I select a small image (about 32px)

[westonruter]

Thank you. I was able to reproduce the issue once I tried a _small_ image.

The error is happening in the SiteIconControl.setImageFromAttachment() method on this line:

$( 'link[sizes="32x32"]' ).attr( 'href', icon.url );

[tyxla]

With the current implementation, if there is an image with the size site_icon-32, it is used. If there is no image with that size, the thumbnail size is used. In case the image is smaller than thumbnail, it has only full size, which we should use in this case. So I suggest that we check for all of these sizes and finally the full size for this edge case. Patch coming up soon.

[tyxla]

Customizer Site Icon: SiteIconControl.setImageFromAttachment() - properly handling smaller image sizes when cropping step is skipped.

[obenland]

I'd like to propose a different approach, requiring users to always crop the icon.

We need the icon to be square anyway, otherwise they're displayed distorted which is not a good user experience either.

[westonruter]

@tyxla @truonght Thoughts on 33417.diff​?

[tyxla]

My two cents: I believe we should allow the user to skip the cropping step. There is no reason to require the cropping IMHO - why not allow the image to remain untouched? There are many cases when the user has prepared a special image for the site icon, and they would want it to remain uncropped.

Also, concerning 33417.diff​, it does not fix the issue for me. I haven't made a research, but I guess that mustBeCropped is not fully implemented yet.

I've just tested my patch again, it fixes this issue and the "Skip Cropping" button works exactly as it should.

[ocean90]

In 34056:

Site Icon: For preview fall back to full size URL when thumbnail size doesn't exist.

Prevents a JavaScript error for rare cases when cropping is skipped and the image is smaller than thumbnail.

Props tyxla.
Fixes #33417.

[ocean90]

In 34057:

Site Icon: For preview fall back to full size URL when thumbnail size doesn't exist.

Prevents a JavaScript error for rare cases when cropping is skipped and the image is smaller than thumbnail.

Merge of [34056] to the 4.3 branch.

Props tyxla.
See #33417.

[obenland]

Replying to tyxla:

There is no reason to require the cropping IMHO - why not allow the image to remain untouched? There are many cases when the user has prepared a special image for the site icon, and they would want it to remain uncropped.

There are two reasons to force cropping, being able to generate the correct image sizes for app icons and making sure the image is actually square.

If 33417.diff​ didn't fix your issue we should see what the problem is and fix it rather than just patching over an error.

[ocean90]

#33865 was marked as a duplicate.