Make WordPress Core

Opened 19 months ago

Last modified 8 months ago

#58137 assigned defect (bug)

WP creates unnecessary "cropped-"[filename] icon file

Reported by: thomask's profile thomask Owned by: antpb's profile antpb
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch changes-requested
Focuses: Cc:

Description

When you setup web icon, it always creates "cropped-"[filename] icon file, even when you upload precisely 512×512 png file, so that the new file is exact copy.

(and btw it is not optimal that for icon you have to go to customizer and for logo you have to go to site editor. IMO both should be logically defined on options-general.php admin screen, where is the site title and description and then as a block in site editor you could change the logo as well).

Attachments (1)

favicon.jpg (4.1 KB) - added by tb1909 17 months ago.
Example favicon which is not working

Download all attachments as: .zip

Change History (22)

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


19 months ago

#3 @antpb
19 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.3

Thanks for the report!

I found some further weirdness in my testing. If you remove the icon using the remove button and select the source 512x512 image again, it makes ANOTHER copy. You can repeat this process a bunch of times and each will result in another cropped copy made.

@joedolson mentioned :
Two things: 1) should be able to skip cropping if the image has the right dimensions, 2) should only create a cropped version once.

Moving to 6.3 to keep visibility on this and work toward a resolution.

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


17 months ago
#4

  • Keywords has-patch added; needs-patch removed

#5 @sstoqnov
17 months ago

I have opened a pull request that addresses the cropping issue when uploading an image with the correct dimensions. However, I am unable to replicate the problem of multiple cropping. Is it possible that the issue has already been resolved?

@antpb Are there any steps other than removing/uploading a new icon?

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


17 months ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


17 months ago

@tb1909
17 months ago

Example favicon which is not working

#8 @tb1909
17 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/4557/commits/e7d678d84b7969840cb220c8bc3d043d444691fe

Environment

  • OS: macOS 13.4
  • Web Server: Nginx
  • PHP: 8.1.13
  • WordPress: 6.3-alpha-55505-src
  • Browser: Google Chrome 114.0.5735.106
  • Theme: Twenty Twenty-Three
  • Plugins: none

Actual Results

  • Both issues still exist

Additional Notes

  • After uploading an favicon with dimensions 512x512px the favicon still gets a cropped version (favicon attached).
  • The favicon also gets cropped again if it is manually removed by the user and the other image is set as the favicon

Steps to reproduce:

  • Upload the attached favicon and press select
  • Open the image select again to see the cropped favicon show up
  • Delete the cropped favicon and select the previously uploaded favicon again
  • See the cropped favicon appear again

Supplemental Artifacts

Favicon which is still being cropped

  • https://core.trac.wordpress.org/raw-attachment/ticket/58137/favicon.jpg

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


17 months ago

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


16 months ago

#11 @oglekler
16 months ago

  • Keywords needs-refresh added
  • Milestone changed from 6.3 to 6.4

This ticket was discussed during the bug scrub and because the patch needs additional work, it was decided to move it into 6.4.

Props @mukesh27

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


15 months ago

#13 @nicolefurlan
13 months ago

@sstoqnov are you able to update the patch for this ticket? We're looking to include it in the release candidate for 6.4 in ~2 weeks.

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


13 months ago

#15 @antpb
13 months ago

  • Owner set to antpb
  • Status changed from new to assigned

#16 @oglekler
13 months ago

  • Keywords changes-requested added; needs-refresh removed

Comment has no sense because we know for sure that this is $context === 'site-icon':

// Skip creating a new attachment if the attachment is a Site Icon.

This line is not working because _wp_attachment_context doesn't exist at this point:

get_post_meta( $attachment_id, '_wp_attachment_context', true ) == $context

And this condition is now working either became there is no $metadata array:

$data['width'] === $metadata['width'] && $data['height'] === $metadata['height']

So, it needs to check something else to make this work.

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


13 months ago

#18 @oglekler
13 months ago

  • Milestone changed from 6.4 to 6.5

It looks like we cannot make it until RC1, so I am moving it into the next milestone.

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


9 months ago

#20 @rajinsharwar
9 months ago

  • Milestone changed from 6.5 to Future Release

This ticket was discussed during a test bug scrub session, and punted to a Future Release due to a low activity.

ghost-of-the-mind commented on PR #4557:


8 months ago
#21

Hey, I am kinda new to this whole "contributing thing". So, I have no idea if this is the best way to contribute, yet, I was struggling with a similar issue with SVG instead of ICO files and wanted to share it here.

  1. I added SVG support and wanted to upload a .svg file as the Site Icon (Appearance >> Customize >> Site Identity).
  2. I uploaded the .svg first. That went well since I had enabled SVG support inside functions.php.
  3. I wanted to select the uploaded .svg as a Site Icon. I selected the image, but WP asked me to crop the image. I could not do that since WP is not set up to crop vector images. I clicked on "Crop" anyway and WP gave me an error, that the image can't be cropped. Suprise, surprise...

To fix the issue, I added the following code in the same file /wp-admin/includes/ajax-actions.php

function wp_ajax_crop_image() {

//...rest of the code

if ( empty( $attachment_id ) || ! current_user_can( 'edit_post', $attachment_id ) ) {
  wp_send_json_error();
}

// THE CODE I ADDED BELLOW

// Check if the attachment is an SVG file
    $mime_type = get_post_mime_type( $attachment_id );
    if ( 'image/svg+xml' === $mime_type ) {
        // For SVG files, send the original image data without cropping
        wp_send_json_success( wp_prepare_attachment_for_js( $attachment_id ) );
    }

// THE CODE I ADDED ABOVE

$context = str_replace( '_', '-', $_POST['context'] );
$data    = array_map( 'absint', $_POST['cropDetails'] );
$cropped = wp_crop_image( $attachment_id, $data['x1'], $data['y1'], $data['width'], $data['height'], $data['dst_width'], $data['dst_height'] );

//...rest of the code
}

What it does, is it checks if the image that is going to be cropped is an SVG, if it is and you click on "Crop" it does not bother to crop it at all and returns the image as it is.

This way I was able to designate the SVG file I uploaded as a Site Icon.

I am not saying that this is a perfect solution, but I do not see the point in cropping an SVG at all. So it works for me...

Note: See TracTickets for help on using tickets.