Cropping custom logo should preserve attachment properties

[flixos90]

I just noticed that, when uploading and cropping a custom logo, the original attachment's properties won't be migrated to the cropped image. I specifically added an "alt" description of the image, but on the website it only showed the file name for the alt tag, since the new cropped image does not preserve any of the data provided into the uploaded image. I think this is unexpected behavior and should be fixed.

I think the data manually changed by the user should be migrated to the cropped image to prevent this.

[Clorith]

I'm not sure I agree (just to give a counterpoint), if I manually crop an image, it is to put emphasis on a part of it, making it no longer relevant to the meta data added to the full image.

Scenario:

• I upload a picture of a bunch of football players, I give it an alt label of "The whole football team in action"
• I crop the image, I only want the part of the image where a player kicks the ball, I now have "Player McGee in action", as such it's expected that a new Alt title be put in, if we auto-populate the field then this will just be ignored.

I think a much better approach would be if the cropping tool actually provided the meta sidebar like the other media management windows does.
This would allow the meta data to be manipulated there and then, without running the risk of being forgotten or ignored, thoughts?

[flixos90]

Good point. Maybe a combination of both would do it. I think we should cover both scenarios - both of our proposals will ignore either one or the other. In my opinion a good solution would be to show the meta sidebar on the cropped image like you said, but fill in the values the user entered for the original image (as defaults). Many times these will apply to the cropped image as well, and if not, they could easily change it.

[afercia]

Related: #38768

[afercia]

#36868 was marked as a duplicate.

[afercia]

See also #30155

[afercia]

Agreed to re-assign ownership.

[dlh]

#46185 was marked as a duplicate.

[jarledb]

Why is this being ignored? It is an actual accessibility issue - since it takes away the ability to set proper alt text on the site logo.

Its also a very frustrating bug to come across when trying to set alt text for the site image (when its been cropped).

The very least it should be documented in the WordPress Codex so people can find it and understand that alt text (and other attachment data) is not available for the image after being cropped in the customizer.

[antonvlasenko]

I'm able to reproduce this bug.
I will try to work on a patch to see if I can move this issue forward.

This PR aims to implement preserving attributes when cropping a custom logo image.

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

[antonvlasenko]

Steps to test this PR:

1. Activate a non-block theme like Twenty Twenty-One.
2. Go to the Media Library.
3. Upload a new image to the media library.
4. Set attachment properties like Alternative Text, Title, Caption, Description.
5. Go to the Appearance -> Customize page.
6. Go to the Site Identity, click on the Select Logo.
7. Set a new cropped logo for the theme using the image you uploaded.
8. Go to the Media Library.
9. Open the last (cropped) image.
10. Make sure that the original attachment properties (i.e. Alternative Text, Title, Caption, Description) have been copied over to the cropped image.
11. Repeat steps 2-3.
12. Reset all attachment properties (leave the fields empty).
13. Repeat steps 5-9.
14. Make sure that the Title field contains name of the cropped file.
15. Make sure that the Description field contains url of the cropped file.
16. Alternative Text and Caption properties have to be empty.

[ironprogrammer]

Thank you for the patch, @antonvlasenko!

Test Report

For ​PR 2441. Sincere apologies for stomach grumbles caused by these test images 😋

Env

• WordPress 6.0-alpha-52448-src
• Chrome 99.0.4844.74
• macOS 12.3 (Monterey)
• Theme: Twenty Twenty-One
• Gutenberg DISABLED 🔴

Steps to Test

1. Activate a non-block theme like Twenty Twenty-One.
2. Navigate to Media > Library.
3. Upload a new image to the media library, at least 512x512 pixels in size.
4. Set attachment properties for Alternative Text, Title, Caption, and Description.
5. Go to the Appearance > Customize.
6. Select Site Identity, and under the Logo section, click "Select Logo".
7. Select the image just uploaded.
8. Click the "Select" button to open the crop tool.
9. Select the region to crop, and click the "Crop image" button.
10. Click "Publish"* and close the Customizer.
11. Navigate to Media > Library.
12. Select the latest image added (the cropped logo).
13. Observe that the Alt Text, Title, Caption, and Description previously set are populated in the Attachment Details sidebar. ✅
14. Repeat steps 2-3.
15. Reset all attachment properties (clear all fields).
16. Go to the Appearance > Customize.
17. Select Site Identity, and under the Logo section, click "Change Logo".
18. Repeat steps 7-12.
19. Observe that Title contains the name of the cropped file, and that Description contains the URL of the cropped file. The other two fields remain empty. ✅

(* NB: you do not need to publish for this test, but it avoids popups and revision notices in subsequent steps.)

Expected Results (✅)

• Attachment Details attributes are preserved when set on the source image.✅
• When attributes are blank, the Title and Description attributes are automatically set to the cropped file name and URL, respectively.✅

Screenshots

Original uploaded image (Step 3).

Cropped logo image retains attributes (Step 13).

Image with cleared attributes (Step 15).

Cropped logo image defaults Title and Description to name and URL (Step 19).

Additional Notes

This PR preserves attachment details for the Customizer's Logo crops, but crops to the Site Icon do not retain those attributes -- the system exhibits the same behavior as in Step 19 above. This is out of scope for the reported issue, but worth noting here.

Crop for Site Icon does not retain attributes from original image (Additional Notes).

[antonvlasenko]

Thank you so much for the test reports, @ironprogrammer!
I appreciate it.

crops to the Site Icon do not retain those attributes

I'd implement this fix in the scope of another trac issue as it will require implementing new unit tests and test reports. Thank you for reporting.

[antonvlasenko]

Just FYI: this PR has some breaking changes.
Currently, WordPress always overwrites Title and Description properties (with name and url of the image file respectively) when we crop an image.
If this PR gets merged, these properties will only be populated (overwritten) if Title and Description properties of the original image are empty.
I'm not sure if that's desired behavior, but I can't think of a better implementation.

[hellofromTonya]

Replying to antonvlasenko:

Just FYI: this PR has some breaking changes.
Currently, WordPress always overwrites Title and Description properties (with name and url of the image file respectively) when we crop an image.
If this PR gets merged, these properties will only be populated (overwritten) if Title and Description properties of the original image are empty.
I'm not sure if that's desired behavior, but I can't think of a better implementation.

​PR 2441 does pre-populate a new cropped image's properties from the original image's properties - including the Title and Description. The Title though doesn't compare if the user edited it though - proposing that change in the code review.

The PR seems to align to the intent proposed by @flixos90

fill in the values the user entered for the original image (as defaults)

But yes, both the Title and Description properties existing behavior / values are changed by the PR.

What about the original intent? Looking at the existing data and [33280] / #29211 that added this functionality:

• there's no discussion about the properties.
• and thus, both Title and Description seem to be generic placeholders to create the new cropped image.

To me, as a user, I'd expect to retain the original image's properties and be given the ability to modify them when cropping. If the properties sidebar is also exposed as @Clorith proposed, then the user is given that ability.

Hey @flixos90 @Clorith what do you think today (as this one is an oldie)? Any concerns about changing the existing behavior for the Title and Description? Or does it still make sense today to pre-populate with the original image's properties and show the properties sidebar?

[hellofromTonya]

Test Report

Patch tested: ​PR 2441

Env:

• OS: macOS Big Sur
• Localhost: wp-env
• WordPress: trunk
• Browser: Firefox and Edge
• Plugins: none
• Theme: Twenty Twenty

Testing Steps

1. Activate Twenty Twenty or any other non-block theme.
2. Go to Media > Library.
3. Upload at least one image. (In my case, I uploaded 4 different images with different metadata to pre-populate the properties.)
4. For certain tests, set the attachment properties for alt test, title, caption, and/or description.
5. Go to the Appearance > Customize > Site Identity.
6. In the Logo section, click "Select Logo".
7. Select an image.
8. Click the "Select" button. Expected behavior: Crop modal opens.
9. Select a region to crop.
10. Click the "Crop image" button.
11. Click "Publish"* and close the Customizer.
12. Go to Media > Library.
13. Click on the cropped logo image to open its properties screen. Test: Check that the properties are properly populated. Expected behavior:
    • Title:
      • Is cropped-[filename] when the original image's title is the filename (meaning the image did not have a title in its metadata when uploaded or the title property wasn't manually changed in step 4).
      • Is the same as the original image's title when the original title is not the filename (meaning the image either has a title in its metadata or the title property was manually changed in step 4).
    • Alt text: matches the original image's alt text property.
    • Caption: matches the original image's caption property.
    • Description:
      • Cropped image's URL when the original image's description property is empty.
      • Else, matches the original image's description property.

Test Results

Scenarios:

Original Title	Original Alt	Original Caption	Original Description	Cropped Correct?
filename				✅
filename	alt			✅
filename		caption		✅
filename			description	✅
filename	alt	caption		✅
filename	alt		description	✅
filename	alt	caption	description	✅
title				✅
title	alt			✅
title		caption		✅
title			description	✅
title	alt	caption		✅
title	alt		description	✅
title	alt	caption	description	✅

Summary

• Able to reproduce the reported issue ✅
• After applying the patch and tested each of the scenarios, patch works as expected ✅

[hellofromTonya]

​PR 2441 LGTM for pre-populating cropped file properties from the original image. Approved the PR.

commit is pending feedback from @flixos90 and @Clorith about changing the title and description's existing behavior (see comment 29 above).

[flixos90]

Replying to hellofromTonya:

Hey @flixos90 @Clorith what do you think today (as this one is an oldie)? Any concerns about changing the existing behavior for the Title and Description? Or does it still make sense today to pre-populate with the original image's properties and show the properties sidebar?

This still makes sense to me today, I think it's a UX flaw that this information isn't taken over when cropping the image. I also just reviewed the PR and left a few minor comments, but it's pretty much good to go from my end.

Thank you @antonvlasenko and everyone involved in getting this fixed after all these years! :)

[Clorith]

This makes sense from my end as well, once you are done cropping the interface also now shows the cropped version and its data so that covers my original concerns as well, so looks good!

(And can i say thank you for those amazing test scenarios and the details they provide, I am loving them!)

[joedolson]

Awesome! Let's get this moving! I'm marking for commit; I'll do a final review and take care of this soon.

[joedolson]

In 53027:

Media: Preserve attachment properties on cropping custom logo.

Migrate the alternative text, title, description, and caption of an image over to the cropped copy of the image after cropping. Ensure that characteristics added to an image prior to cropping are not lost.

Props flixos90, Clorith, afercia, antonvlasenko, ironprogrammer, hellofromTonya.
Fixes #37750.

Committed via changeset https://core.trac.wordpress.org/changeset/53027.