Make WordPress Core

Opened 6 years ago

Closed 21 months ago

#44340 closed enhancement (wontfix)

Use sprintf() to remove <strong> tags from translating strings

Reported by: saimonh's profile saimonh Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9.6
Component: I18N Keywords:
Focuses: coding-standards Cc:

Description (last modified by hellofromTonya)

In the wp-includes/user.php file (and others), there are some <strong> tag has which has been used inside the translated __() function.

So it's better to make use of sprintf function here than hard-coding the <strong> tag.

Screenshot: https://d.pr/free/i/qbHq2z

Regards

Attachments (1)

patch.diff (8.4 KB) - added by saimonh 6 years ago.

Download all attachments as: .zip

Change History (5)

@saimonh
6 years ago

#1 @abdullahramzan
6 years ago

  • Focuses coding-standards added

#2 @GaryJ
6 years ago

  • Keywords close added

Hi @saimonh,

Congrats on creating your first ticket :-)

In this case, I'd say that the change is unnecessary.

It's fine for strings to have basic HTML in them. In some cases, it's necessary, so that the translator can choose what word has the strong emphasis, or which words are the link text, etc.

In this case, you've split the strings up into multiple strings, which doesn't help with understanding context - they might be a different word for "ERROR", depending on what the issue is, but after your patch, that wouldn't be easy to adjust for, without also adding context (_x()) which then results in the same string having to be translated mutliple times.

Also, you've left the colon (:) in the sprintf template, meaning other punctuation can't be used if other locales use something different to a colon.

If markup is only at the ends of a string, then that's more likely to be a good candidate for pulling the markup out.

#3 @hellofromTonya
21 months ago

  • Component changed from Users to I18N
  • Description modified (diff)
  • Keywords has-patch removed
  • Summary changed from Make use of sprintf function while translating strings to Use sprintf() to remove <strong> tags from translating strings
  • Type changed from defect (bug) to enhancement

Hello @saimonh,

Welcome and thank you for the ticket and patch!

I'm popping in to do a little ticket triage to raise visibility.

First, I updated the ticket to be a more general request across Core.

Why? The usage of <strong>Error</strong> is a common pattern that exists not only in the wp-includes/user.php file but also in the login, comments, options, and more. By making this a general request, it targets the I18N component maintainers and contributors for their feedback.

Second, I removed the has-patch keyword.

Why? Recently this pattern was slightly updated to move the : within the strong tags for better consistency: see #50785 / [53458] and [53476]. These changes will no longer apply cleanly to the proposed patch.

#4 @SergeyBiryukov
21 months ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Hi there, thanks for the ticket!

As noted in comment:2, it is normal for strings to have some basic HTML in them. Splitting strings into smaller chunks often leads to losing important context and making them less translatable without an obvious benefit. Complete sentences, on the other hand, are much easier to translate.

Looking at the patch, the current code:

sprintf(
	/* translators: %s: User name. */
	__( '<strong>ERROR</strong>: The password you entered for the username %s is incorrect.' ),
	'<strong>' . $username . '</strong>'
) 

is significantly more i18n-friendly than the proposed alternative:

sprintf(
	'<strong>%s</strong>: %s <strong>%s</strong> %s',
	__( 'ERROR' ),
	__( 'The password you entered for the username' ),
	username,
	__( 'is incorrect.' )
)

Note that in the former example, the second <strong> tag is already moved out of the translatable string, as part of the previous efforts on removing unnecessary HTML from strings. This is a balance between allowing some basic HTML to preserve complete sentences and moving any extra HTML out of translations.

Note: See TracTickets for help on using tickets.