Make WordPress Core

Opened 3 years ago

Last modified 11 months ago

#55840 assigned enhancement

Internationalization support for sizes added by 'edit_custom_thumbnail_sizes' filter

Reported by: tmatsuur's profile tmatsuur Owned by: antpb's profile antpb
Milestone: Future Release Priority: normal
Severity: normal Version: 6.0
Component: Media Keywords: 2nd-opinion
Focuses: Cc:

Description

The image size labels added by the edit_custom_thumbnail_sizes filter are not internationalized.

<label for="imgedit-target-custom<?php echo esc_attr( $key ); ?>"><?php echo esc_html( $size ); ?></label>

In a Japanese environment, there will be a mixture of translated and untranslated text.

To solve this problem, the following size of translated text is prepared before the loop process.

$size_names = array(
	'thumbnail'    => __( 'Thumbnail' ),
	'medium'       => __( 'Medium' ),
	'medium_large' => __( 'Medium Large' ),
	'large'        => __( 'Large' ),
	'full'         => __( 'Full Size' ),
);

Change the following where labels are output.

<label for="imgedit-target-custom<?php echo esc_attr( $key ); ?>"><?php echo esc_html( isset( $size_names[$size] )? $size_names[$size] : $size ); ?></label>

Since image size labels are also used in the "image_size_input_fields" function, etc., it may be better to make them a function rather than an array.

In addition, the translated text for "Medium-Large size image height" and "Medium-Large size image width" exists, but the translated text for "Medium Large (or Medium-Large)" does not.

If you are conscious of word commonality, "Medium-Large" may be better than "Medium Large".

Attachments (2)

55840.3.diff (1.3 KB) - added by antpb 22 months ago.
patch_55840_3.png (186.2 KB) - added by petitphp 22 months ago.
Edit image UI after applying patch 55840.3

Download all attachments as: .zip

Change History (40)

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 6.0.1

Hi there, thanks for the ticket!

Moving to 6.0.1, this appears to be a follow-up to [53161] / #28277.

#2 @mukesh27
3 years ago

  • Type changed from feature request to enhancement

Hi there!

I checked the issue and sent some feedback.

For the custom image name system use get_intermediate_image_sizes() function here so it will not add Full Size image type.

Instead of a fixed image size and name array can we use the image_size_names_choose filter with some default array?

<?php
$image_size_names = apply_filters(
        'image_size_names_choose',
        array(
                'medium'       => __( 'Medium' ),
                'medium_large' => __( 'Medium Large' ),
                'large'        => __( 'Large' ),
        )
);

I will upload patch soon.

This ticket was mentioned in PR #2754 on WordPress/wordpress-develop by mukeshpanchal27.


3 years ago
#3

  • Keywords has-patch added

#4 follow-up: @audrasjb
2 years ago

Hey there, thanks for the PR @mukesh27!

Since get_intermediate_image_sizes() also calls wp_get_additional_image_sizes() which return custom sizes with their label, shouldn't we change a bit the existing code to make sure custom sizes are also labelled with their translated label (when it exists)?

#5 in reply to: ↑ 4 @tmatsuur
2 years ago

Replying to audrasjb:

Hey there, thanks for the PR @mukesh27!

Since get_intermediate_image_sizes() also calls wp_get_additional_image_sizes() which return custom sizes with their label, shouldn't we change a bit the existing code to make sure custom sizes are also labelled with their translated label (when it exists)?

When extending the functionality of the get_intermediate_image_sizes(), I am concerned about the possibility of unintended rewriting by the "intermediate_image_sizes" filter.

I don't think you have to worry about that if you have a new function.

#6 @SergeyBiryukov
2 years ago

  • Keywords has-patch removed
  • Milestone changed from 6.0.1 to 6.0.2

It looks like the PR is closed and a new patch is needed. With 6.0.1 RC1 coming soon, moving to 6.0.2 for now.

#7 @SergeyBiryukov
2 years ago

  • Milestone changed from 6.0.2 to 6.1

Looks like this did not get any further traction yet. With 6.0.2 RC1 coming today, moving to 6.1 for now.

#8 @desrosj
2 years ago

  • Keywords has-patch needs-refresh added
  • Milestone changed from 6.1 to 6.2

This one has not received any of the needed attention this cycle. With 6.1 due out in less than 12 hours, I'm going to punt.

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


23 months ago

#10 @costdev
23 months ago

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

This ticket was reviewed in the bug scrub. As the PR was closed, this ticket needs a new patch. Adding the appropriate keyword.

Additional Props: @afragen, @hellofromTonya

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


23 months ago
#11

  • Keywords has-patch added; needs-patch removed

#12 @petitphp
23 months ago

Patch from @mukesh27 seems ok to me. I did more testing with multiple theme using the filter image_size_names_choose (see. https://wpdirectory.net/search/01GQFGC70R6ASDNSXNS0631KBX) and the names for the custom sizes are displayed correctly.

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


23 months ago

#14 @antpb
23 months ago

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

Assigning to myself to review and likely commit for 6.2

#15 @robinwpdeveloper
22 months ago

Thanks for the updated PR @petitphp
Both PRs are similar in the functionality, It's good to go to 6.2

Optional Feedback: If possible we can reduce the number of line changes to make it same as @mukesh27

#16 @petitphp
22 months ago

Thanks for the validation @robinwpdeveloper.

I've updated the PR with your feedback.

#17 @costdev
22 months ago

  • Keywords commit added

Myself and Mukesh reviewed this ticket. The PR looks ready for commit consideration, adding the keyword.

What do you think @antpb?

Additional props: @mukesh27

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


22 months ago

#19 @costdev
22 months ago

  • Type changed from enhancement to defect (bug)

This ticket was scrubbed pre-6.2 Beta 1. Reclassifying this ticket as a defect (bug).

#20 @johnbillion
22 months ago

I'd be tempted to retain the full array element for consistency -- even though it's not used here -- as it's present in all other instances of that filter.

#21 @azaozz
22 months ago

  • Keywords needs-patch added; has-patch removed

Just FYI, medium-large is a "hidden" image size that is not (should not be) used anywhere in the UI. It was added only to support responsive images (srcset and sizes), same as the other two default image sizes: 1536x1536 and 2048x2048 (these are the "size names" in the image meta), see https://core.trac.wordpress.org/browser/tags/6.1/src/wp-includes/media.php#L5237.

Looking at the PRs, medium-large perhaps shouldn't be "translatable"? WordPress has only four image sub-sizes that are exposed in the UI: thumbnail, medium, large, full size (sometimes). The other three are not.

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

@antpb
22 months ago

#22 @antpb
22 months ago

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

@johnbillion I added full and removed medium large from the array per @azaozz recommendation. Is the attached patch looking good for commit?

I suspect not having thumbnail is fine for this filter? Every other instance usually includes it. I can add if so before commit.

#23 follow-up: @petitphp
22 months ago

Patch look good to me.

I suspect not having thumbnail is fine for this filter? Every other instance usually includes it. I can add if so before commit.

Since thumbnail is a special case and handle separately, I don't think it's useful to add it here.

@petitphp
22 months ago

Edit image UI after applying patch 55840.3

#24 in reply to: ↑ 23 @azaozz
22 months ago

Replying to petitphp:

Since thumbnail is a special case...

The image_size_names_choose is used at 7 more places in core, and the array that is being filtered is consistent everywhere:

array(
	'thumbnail' => __( 'Thumbnail' ),
	'medium'    => __( 'Medium' ),
	'large'     => __( 'Large' ),
	'full'      => __( 'Full Size' ),
)

I don't think it is a good idea to make it different/inconsistent here.

As this will be the eight location of that filter, perhaps having a function that just applies the filter would be better? Not sure if image_size_names_choose() is a good name for a function though, perhaps something like wp_image_size_display_names() is better. Can make a patch a bit later if somebody doesn't beat me to it :)

#25 @azaozz
22 months ago

@petitphp Btw where do you see the settings as in patch_55840_3.png? This is a big "doing it wrong". The image handling in WP cannot handle editing only one sub-size, and it doesn't make any sense.

I'm actually thinking to remove the whole "Apply changes to..." thing. It was intended (as far as I remember) to allow people to switch from square (cropped) thumbnail to a proportional one or the other way round many many years ago, but it never worked well. (That was added to core because of pressure by a vocal minority at the time... Big mistake!!). I'll open a ticket to remove it.

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

#26 @azaozz
22 months ago

Related: #57685.

#27 @azaozz
22 months ago

  • Keywords 2nd-opinion added; has-patch removed

Thinking more about it: because of #57685 this ticket could either be repurposed to add a helper function for image_size_names_choose or closed as "wontfix" and another ticket opened for the helper function. Anybody has any preference?

#28 @petitphp
22 months ago

@azaozz I was indeed using the edit_custom_thumbnail_sizes filter to display all individual sizes.

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


22 months ago

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


22 months ago

#31 @antpb
22 months ago

  • Milestone changed from 6.2 to 6.3

It was agreed in the Media meeting to move forward with a helper function. Moving this to 6.3 so we can scrub and create the new issue in the next cycle.

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


18 months ago

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


18 months ago

#34 @antpb
18 months ago

  • Milestone changed from 6.3 to 6.4

we still have pending the task of making a new issue for the helper function but have missed it in the 6.3 cycle. Moving 6.4 to keep it on our radar for our next scrub.

#35 @oglekler
16 months ago

  • Type changed from defect (bug) to enhancement

It looks not like a bug, but more as an enhancement, so, I am changing the type.

#36 @oglekler
15 months ago

  • Milestone changed from 6.4 to 6.5

Because there is no development in the last 7 months, and we are getting close to Beta 1, I am moving it into 6.5. If there will be no progress, the next move should be to the Future release.

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


11 months ago

#38 @audrasjb
11 months ago

  • Milestone changed from 6.5 to Future Release

From today's bug scrub:
As this is an enhancement and since we’re still waiting for a patch, let’s move it to Future release milestone.
Happy to review and move it back to milestone 6.5 if a complete patch is proposed during the alpha cycle.

Note: See TracTickets for help on using tickets.