WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30694 closed defect (bug) (fixed)

Missing localization for image toolbar

Reported by: pavelevap Owned by: azaozz
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: I18N Keywords: has-patch commit fixed-major
Focuses: Cc:

Description

I am not sure, but localization did not work for me for edit and remove buttons. See attached patch...

We can use existing strings, so no problems for translators. Also there is no need for context, I guess...

Attachments (1)

missing_localization_for_image_toolbar.patch (694 bytes) - added by pavelevap 6 years ago.

Download all attachments as: .zip

Change History (17)

#1 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 4.1

#2 @azaozz
6 years ago

The Remove string is not in there but Edit is just few lines above. It's used in the TinyMCE menubar. To enable it for testing, comment out 'menubar' => false, https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-editor.php#L653

#3 @azaozz
6 years ago

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

In 30835:

TinyMCE: make the tooltip for the remove button in the image toolbar translatable. Props pavelevap, fixes #30694 for trunk.

#4 @azaozz
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.1.

#5 follow-up: @pavelevap
6 years ago

Yes, I know about previous "Edit" string, but this was in different context. For TinyMCE menu it has meaning "noun" in our language. In Image toolbar it has meaning of action (verb). So, I suggested to use other current string "Edit" (without context) which is available. Using the same string for TinyMCE menu and action for editing image is not good...

So we have to rename the whole variable to enable context for this string?

#6 in reply to: ↑ 5 @azaozz
6 years ago

Replying to pavelevap:

So we have to rename the whole variable to enable context for this string?

Yes. TinyMCE uses simple JS object for translations. The English strings are keys, the translated strings -- values. If there are two identical keys, the second will overwrite the first.

Don't see a straightforward way to fix this. Perhaps we can change the tooltip to "Edit image". Would be better to keep it at "Edit" so we can eventually use it for the wpViews mini-toolbar too.

Last edited 6 years ago by azaozz (previous) (diff)

#7 @azaozz
6 years ago

This will (most likely) be fixed upstream. For now we can use a space to add another Edit string. Spaces are "meaningless" in HTML so this is 100% backwards compatible.

#8 @azaozz
6 years ago

In 30839:

TinyMCE: add another Edit translatable string for the tooltip of the Edit button on the image toolbar. Props pavelevap, see #30694.

#9 follow-up: @cais
6 years ago

With all of the localization strings in this section of the code using PHP comments to define the string more clearly, wouldn't this be an indicator that the _x function should be used instead of the standard __ function?

#10 @pavelevap
6 years ago

I do not think that context is needed here, because "Edit" is used as simple verb in other cases. And I also did not want to create new string before release...

#11 in reply to: ↑ 9 ; follow-up: @SergeyBiryukov
6 years ago

Replying to cais:

With all of the localization strings in this section of the code using PHP comments to define the string more clearly, wouldn't this be an indicator that the _x function should be used instead of the standard __ function?

_x() is meant to distinguish the same string used in different contexts. It should not be used as a replacement for translator comments or PHP comments.

#12 in reply to: ↑ 11 @cais
6 years ago

Replying to SergeyBiryukov:

Thanks for the clarification ... that function's use was a little fuzzy to me in this case. My thinking was, if you are adding PHP comments to describe the string to begin with why not add the extra character or two and make it the context parameter.

Last edited 6 years ago by cais (previous) (diff)

#13 @ocean90
6 years ago

  • Keywords has-patch added

#14 @nacin
6 years ago

  • Keywords commit fixed-major added

[30835] and [30839] appear good.

#15 @johnbillion
6 years ago

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

In 30861:

TinyMCE: make the tooltip for the remove button in the image toolbar translatable.

Merges [30835] to the 4.1 branch.

Props pavelevap
Fixes #30694

#16 @johnbillion
6 years ago

In 30862:

TinyMCE: add another Edit translatable string for the tooltip of the Edit button on the image toolbar.

Merges [30839] to the 4.1 branch.

Props pavelevap
Fixes #30694

Note: See TracTickets for help on using tickets.