WordPress.org

Make WordPress Core

Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#51893 closed defect (bug) (fixed)

Don’t split translatable strings in block templates

Reported by: tobifjellner Owned by: audrasjb
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.5
Component: Editor Keywords: has-patch i18n-change commit dev-reviewed
Focuses: Cc:

Description

WordPress RC 1 contains a couple of strings that are almost impossible to translate well without specifically figuring out how the underlying code is created.

The reason is:
Thou shalt never
split a sentence in two parts and then glue them together again after translation.

But that is exactly what happens now in:
https://build.trac.wordpress.org/browser/branches/5.6/wp-includes/block-patterns/large-header-button.php?marks=10#L10

Code excerpt:
<strong>" . __( 'Thou hast seen' ) . '</strong><br><strong>' . __( 'nothing yet' ) . "</strong></p>

Some examples of why this is a problem:

  • German word order would typically put the verb at the end of the sentence.
  • In Swedish, the best translation would split out the negation (from string 2) and put it next to the verb.
  • etcetera.

Even if the translator knows exactly how these strings are used, you still have the problem that somewhere else you might have, say, a result of a search operation that says "nothing yet". Imagine the German translation suddenly having an unexpected verb there, or the Swedish translation actually lacking the "no"...

If you necessarily need to split a sentence that should be translated as a whole as two or more separate chunks, then you MUST use _x() and in the context string lock this partial translation to this particular case.
But the best way to handle is to include the whole sentence in the string. If you, due to technical limitations, can't allow tags in the translated string then we need to find a workaround. But a few formatting tags or br-tags in the string to be translated is totally OK, and much-much better than this kind of magical brewery.

H/t @kebbet who alarmed me about this problem. I'll upload his example from his test run of the Swedish translation in use.

During translation, the Victorian-sounding "Thou" prompted me to just keep the English text, since I had no clue how it was to be used. But the other string "nothing yet" is totally normal, and thus got translated...

Attachments (5)

image (1).png (51.4 KB) - added by tobifjellner 12 months ago.
Swedish test run of WP 5.6 RC1
51893.diff (5.0 KB) - added by audrasjb 12 months ago.
51893.patch (5.0 KB) - added by mukesh27 12 months ago.
Patch that remove BR tag
51893.2.diff (5.0 KB) - added by audrasjb 12 months ago.
moving <strong> outside of translatable string
Capture d’écran 2020-11-29 à 16.57.13.png (1.7 MB) - added by audrasjb 12 months ago.
51893.2.diff

Download all attachments as: .zip

Change History (23)

@tobifjellner
12 months ago

Swedish test run of WP 5.6 RC1

This ticket was mentioned in Slack in #core-editor by tobifjellner. View the logs.


12 months ago

#2 @SergeyBiryukov
12 months ago

  • Milestone changed from Awaiting Review to 5.6

#3 @SergeyBiryukov
12 months ago

Thanks for the report!

Looks like this is not a regression in 5.6 RC1 though, the strings were added in [48639] / #50550 for WP 5.5.

I think the string could just be replaced with __( 'Thou hast seen<br> nothing yet' ).

This also needs to be fixed upstream in Gutenberg. Some previous examples:

#4 @hellofromTonya
12 months ago

  • Version changed from 5.6 to 5.5

As Sergey notes, the strings were added in 5.5.

Looks like this is not a regression in 5.6 RC1 though, the strings were added in [48639] / #50550 for WP 5.5.

@audrasjb
12 months ago

#5 follow-up: @audrasjb
12 months ago

Oh. Due to a browser bug (?), I didn't see the comments above before uploading my patch.

By the way, the above patch keeps the current markup. The string becomes:

<strong>Thou hast seen</strong><br><strong>nothing yet</strong>
Last edited 12 months ago by audrasjb (previous) (diff)

#6 @ocean90
12 months ago

  • Keywords has-patch i18n-change added
  • Summary changed from Block editor: Serious i18n flaw in block patterns for WP 5.6 to Don’t split translatable strings in block templates

#7 in reply to: ↑ 5 @SergeyBiryukov
12 months ago

Replying to audrasjb:

By the way, the above patch keeps the current markup. The string becomes:

<strong>Thou hast seen</strong><br><strong>nothing yet</strong>

Is there a difference in display between that and the <strong> tags outside of the string?

'<strong>' . __( 'Thou hast seen<br> nothing yet' ) . '</strong>'

I'd like to keep the string as simple as possible :)

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

@mukesh27
12 months ago

Patch that remove BR tag

#8 @mukesh27
12 months ago

51893.patch patch remove <br> tag from string and change font size to adjust the text in two lines

@audrasjb
12 months ago

moving <strong> outside of translatable string

#9 follow-up: @audrasjb
12 months ago

@SergeyBiryukov you're right :) 51893.2.diff moves <strong> tags outside of the translatable string.
@mukesh27 the <br /> tag is here by design, I wouldn't touch it without asking the design team. Better to keep it as it is now.

#10 @SergeyBiryukov
12 months ago

  • Keywords commit added

51893.2.diff looks good to me :)

Let's also patch this upstream in Gutenberg, the file that needs changing is here:
https://github.com/WordPress/gutenberg/blob/master/lib/patterns/large-header-button.php

#11 in reply to: ↑ 9 @mukesh27
12 months ago

Yup. 51893.2.diff looks good to me.

Replying to audrasjb:

@SergeyBiryukov you're right :) 51893.2.diff moves <strong> tags outside of the translatable string.
@mukesh27 the <br /> tag is here by design, I wouldn't touch it without asking the design team. Better to keep it as it is now.

#13 @audrasjb
12 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

Merged in Gutenberg: https://github.com/WordPress/gutenberg/pull/27361
We're waiting for a second committer sign-off.

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


12 months ago

#15 @azaozz
12 months ago

  • Keywords dev-reviewed added

51893.2.diff looks good here too. +1 to commit to the 5.6 branch.

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


12 months ago

#17 @SergeyBiryukov
12 months ago

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

In 49722:

Editor: Don't unnecessarily split a translatable string in block templates.

As a best practice, strings available for translation should contain entire sentences whenever possible.

Splitting a sentence in two parts and putting them back together after translation should be avoided, as the word order in other languages can be different from English.

Props tobifjellner, kebbet, audrasjb, mukesh27, hellofromTonya, azaozz, SergeyBiryukov.
Fixes #51893.

#18 @SergeyBiryukov
12 months ago

In 49723:

Editor: Don't unnecessarily split a translatable string in block templates.

As a best practice, strings available for translation should contain entire sentences whenever possible.

Splitting a sentence in two parts and putting them back together after translation should be avoided, as the word order in other languages can be different from English.

Props tobifjellner, kebbet, audrasjb, mukesh27, hellofromTonya, azaozz, SergeyBiryukov.
Reviewed by azaozz, SergeyBiryukov.
Merges [49722] to the 5.6 branch.
Fixes #51893.

Note: See TracTickets for help on using tickets.