Make WordPress Core

Opened 18 months ago

Closed 12 months ago

Last modified 11 months ago

#57685 closed defect (bug) (fixed)

Deprecate the `edit_custom_thumbnail_sizes` filter and remove the "Apply changes to..." UI in the image editor

Reported by: azaozz's profile azaozz Owned by:
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.0
Component: Media Keywords: has-patch needs-dev-note add-to-field-guide
Focuses: Cc:

Description

Follow-up to #28277.

Been able to edit only some image sub-sizes may have been somewhat useful 9 years ago but is pretty pointless since WordPress started using responsive images (srcset and sizes). It means that the users will be able to "break" responsive images at will, probably without realizing what they are doing. At best, this is a plugin material and doesn't belong in core.

In addition the "Apply changes to..." selection in the image editor doesn't make sense any more. I think it was added 12 years ago to help users switch from proportional to cropped thumbnails, but never worked well.

Thinking that ideally the edit_custom_thumbnail_sizes filter should be deprecated and maybe output a doing_it_wrong error, and the UI for selecting to edit only the thumbnail should be removed.

Attachments (1)

edit-custom-thumbnail-sizes-separately.zip (725 bytes) - added by costdev 12 months ago.
Single-file plugin to re-enable the disabled UI.

Download all attachments as: .zip

Change History (19)

#1 @azaozz
18 months ago

Currently there seems to be 1 plugin and 0 themes that mention the edit_custom_thumbnail_sizes filter. The System Dashboard plugin doesn't really use it, it only mentions it as an existing hook.

Frankly I'm really tempted to recommend to just remove it. This will not result in any errors (removing a filter doesn't usually trigger any errors), and seems there won't be any plugins or themes that will be affected.

Last edited 18 months ago by azaozz (previous) (diff)

#2 @peterwilsoncc
17 months ago

There are two ways responsive images can be broken:

  • (major) different crops are included in the srcset, for example this close up crop in included in the srcset for a zoomed out crop.
  • (minor) editing an image results in no srcset being added. This means small screen devices download the full size image.

The first item is a major break as srcset is speced that the crop is identical in all image. The second item is a minor break as the image displays correctly but the download is not optimal.

Testing notes

Major breaks can not happen as the srcset due to the use of edit hashes, eg duck-e1676350656850-150x150.jpg. The functionality adding srcset attributes ignores images with a different edit hash.

Minor breaks can happen if a single size is regenerated as none of the other images will include an edit hash. For thumbnails this isn't much of an issue but for larger images it can result in unnecessarily large downloads.

I tend to agree that editing sizes individually can be enough of a problem that it should probably be removed from core.

As allowing users to edit thumbnails only can be of use for certain images, if it were to be removed from core it would be good to move it to a canonical plugin.

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


15 months ago
#3

  • Keywords has-patch added

Also:

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

@azaozz commented on PR #4392:


15 months ago
#4

@costdev Thanks for the review. Yep, the tests look great too!

@azaozz commented on PR #4392:


15 months ago
#5

Just a nitpick but wondering if it would be good to take the tests for the UI image editor out of tests/image/functions.php and put them in their own file?

The names are unfortunately "messed up" as the wp_image_editor() function is part of the UI image editor (outputs the UI), but there is also WP_Image_Editor class that is a "wrapper" for GD or ImageMagick (was added later and ideally should have been named WP_Image_File_Editor or something...).

@costdev commented on PR #4392:


15 months ago
#6

That makes sense, and yeah the naming clash is unfortunate.

However, I think we can safely move these tests to tests/phpunit/image/wpImageEditor.php at least for now, as the proposed test suite reorganising would put tests for the WP_Image_Editor class in their own folder anyway, with one file per method.

@azaozz commented on PR #4392:


13 months ago
#7

I think we can safely move these tests to tests/phpunit/tests/image/wpImageEditor.php

Yea, that makes sense but maybe better to leave these in Tests_Image_Functions in image/functions.php for now, then move/rearrange/reshuffle them when applying that proposal. The wpImageEditor.php file name may be used for something else then.

#8 @azaozz
13 months ago

Thinking this is ready for commit. Leaving the ticket open as a reminder to make a one-line plugin that can be used to re-enable the disabled UI.

#9 @azaozz
13 months ago

In 55935:

Media: Deprecate the 'edit_custom_thumbnail_sizes' filter and disable the "Apply changes to [Thumbnail|All|All except thumbnail]" UI in the image editor. Add a (boolean) filter to reenable that UI.

Props peterwilsoncc, costdev, azaozz.
See: #57685.

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


13 months ago

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


12 months ago

#13 @oglekler
12 months ago

Comment from above for visibility: "Leaving the ticket open as a reminder to make a one-line plugin that can be used to re-enable the disabled UI."

This ticket was discussed during bug scrub.

Props @mukesh27

Last edited 12 months ago by oglekler (previous) (diff)

#14 @audrasjb
12 months ago

  • Keywords needs-dev-note added

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


12 months ago

@costdev
12 months ago

Single-file plugin to re-enable the disabled UI.

#16 @costdev
12 months ago

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

This ticket was discussed during the bug scrub. As we're now at 6.3 RC1 and this ticket's work on trunk is complete, we agreed to close this ticket.

I've added a single-file plugin that calls add_filter( 'image_edit_thumbnails_separately', '__return_true' );. This, the name and the plugin headers can be reviewed and iterated as needed.

Additional props: @oglekler @chaion07 @mukesh27

#17 @stevenlinx
12 months ago

  • Keywords add-to-field-guide added

#18 @sjouteux
11 months ago

This is really annoying and has broken my web site as I cannot edit the thumbnail anymore. Please keep the

add_filter( 'image_edit_thumbnails_separately', 'return_true' );

in future releases.

Last edited 11 months ago by sjouteux (previous) (diff)
Note: See TracTickets for help on using tickets.