Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#30694 closed defect (bug) (fixed)

Missing localization for image toolbar

Reported by: pavelevap's profile pavelevap Owned by: azaozz's profile 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 10 years ago.

Download all attachments as: .zip

Change History (17)

#1 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 4.1

#2 @azaozz
10 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
10 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
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 4.1.

#5 follow-up: @pavelevap
10 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
10 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 10 years ago by azaozz (previous) (diff)

#7 @azaozz
10 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
10 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
10 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
10 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
10 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
10 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 10 years ago by cais (previous) (diff)

#13 @ocean90
10 years ago

  • Keywords has-patch added

#14 @nacin
10 years ago

  • Keywords commit fixed-major added

[30835] and [30839] appear good.

#15 @johnbillion
10 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
10 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.