Site Health: audit the translation functions

[afercia]

Splitting this out from https://core.trac.wordpress.org/ticket/46573#comment:46

This kind of strings:

<span class="issue-count">0</span> <?php _e( 'Critical issues' ); ?>

(note: the count gets updated via JavaScript) are not translatable in languages that have multiple plural forms. This needs to be changed and meet the best practices used in core for proper localization. There are a few cases like this that should be addressed.

To my knowledge, the best option is to use a placeholder that gets then replaced via JavaScript: I see in other parts of the Site Health code this is implemented correctly.

/Cc @SergeyBiryukov

Also, I'd argue the following usages of context:

_ex( 'Site Health', 'Menu, Section and Page Title' )

is not the best way to use the context parameter: the context shouldn't be an "explanation". Instead, it should be a very short reference to what the strings refer to, meant to avoid collisions with similar translatable text. Also, in this specific case, not sure what is the value added by specifying "Menu, Section and Page Title".

[ocean90]

• wp.18n can be used to translate strings in JavaScript
• The context for 'Site Health' can be removed
• Translations should not be escaped
• In translator comments, placeholders like %1$s: should be just the number 1
• Plural strings like "Your site has %d inactive plugin. Inactive plugins are tempting targets for attackers. if you’re not going to use a plugin, we recommend you remove it." should be split into two strings so that the last past doesn't need to be translated twice. Note also the current typo in "if".
• The context in _x( 'https://make.wordpress.org/hosting/handbook/handbook/server-environment/#php-extensions', 'The address to describe PHP modules and their use.' ) should be a translators comment
  • Related to this __( 'the team handbook' ) should not be translated alone, instead add <a href="%s">the team handbook</a> back to the string.
• Constants in strings should be replaced by placeholders. This also allows to mark them as code, for example <code>WP_DEBUG_LOG</code>
• The badge labels are not translatable

[afercia]

+1 Thanks @ocean90

[SergeyBiryukov]

Created #46755 for the badge labels.

[SergeyBiryukov]

In 45095:

Site Health: Add missing i18n for Security and Performance badge labels.

Props iworks.
Fixes #46755. See #46683.

[SergeyBiryukov]

In 45099:

Site Health: i18n audit, take 1.

• Split plural strings with multiple sentences to avoid duplicating translations.
• Decouple strings where the singular and plural form are not just the same string with different numbers, but essentially two different strings.
• Use an established pattern for numbered placeholders in translator comments.
• Replace constants in translatable strings with placeholders, mark them as code.
• Make sure sentences are translated as a whole, not as separate string parts.
• Remove unnecessary context and escaping.

Props ocean90, SergeyBiryukov.
See #46683.

[SergeyBiryukov]

I think I caught everything from comment:2, except for this item:

• wp.i18n can be used to translate strings in JavaScript

The main issue from the ticket description with strings like this is also still relevant:

• <span class="issue-count">0</span> <?php _e( 'Critical issues' ); ?>

[SergeyBiryukov]

In 45102:

Site Health: Use an established pattern for numbered placeholders in translator comments in WP_Debug_Data.

See #46683.

[ocean90]

Replying to SergeyBiryukov:

The main issue from the ticket description with strings like this is also still relevant:

• <span class="issue-count">0</span> <?php _e( 'Critical issues' ); ?>

To fix this you can use _n() and sprintf() from the wp.i18n package like so:

var _n = wp.i18n._n;
var sprintf = wp.i18n.sprintf;
var headline = sprintf( _n( '%s Critical Issues', '%s Critical Issues', count ), '<span class="issue-count">' + count + '</span>' );

The result should replace the inner HTML of the h3 element.

Any string that's added via SiteHealth.string should probably be switched to using wp.i18n.

[TimothyBlynJacobs]

The result should replace the inner HTML of the h3 element.

Implemented. I used the > h3 selector to get that immediate h3 element for safety. That could have a class added instead.

Any string that's added via SiteHealth.string should probably be switched to using wp.i18n.

I replaced SiteHealth.string.site_health_complete_screen_reader and site_info_copied with wp.i18n. I couldn't find any usages of the other localized strings.

There were also two "Copied!" strings that weren't translated.

I also modified the original PHP printed headlines to use _n as well so that languages that have 0 as singular could do that. The 0 is constant though, so maybe it'd be better to have a translator comment like This is referring to 0 issues or similar?

[TimothyBlynJacobs]

Fixes a merge conflict and replaces the please_wait and site_health_complete strings.

[TimothyBlynJacobs]

Ended up doing a translators comment which I think is more correct.

[SergeyBiryukov]

In 45178:

Site Health: i18n audit, take 2.

• Use wp.i18n to translate JavaScript strings.
• Use _n() for proper plural forms support.

Props TimothyBlynJacobs, ocean90, afercia.
Fixes #46683.

[SergeyBiryukov]

In 45179:

Site Health: Use _n() for %s Items with no issues detected string, missed in [45178].

See #46683.

[afercia]

Coding standards: Variables declared with var still ​need to be at the top of a function block. In this specific case: https://core.trac.wordpress.org/browser/trunk/src/js/_enqueues/admin/site-health.js?rev=45178&marks=149-150#L138 I'm not sure why a variable is used in the first place. The string can be directly passed to sprintf().

[afercia]

In 45197:

Coding standards: Site health: Avoid to use var in the middle of JavaScript function blocks.

Amends [45178].
See #46683.

[afercia]

I used the > h3 selector to get that immediate h3 element for safety. That could have a class added instead.

Let's do it :) JS selectors shouldn't depend on a specific markup that may change at any time in the future.

[afercia]

In 45198:

Site health: Improve jQuery selectors so that they don't depend on a specific markup.

Amends [45178].
See #46683.

[afercia]

PHPCS isn't happy because of missing translators comments:

FILE: src/wp-admin/site-health.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------
  94 | WARNING | A gettext call containing placeholders was found, but was not accompanied by a "translators:" comment ...
 102 | WARNING | A gettext call containing placeholders was found, but was not accompanied by a "translators:" comment ...
 118 | WARNING | A gettext call containing placeholders was found, but was not accompanied by a "translators:" comment ...

[afercia]

In 45199:

Site Health: Add missing translator comments.

Amends [45178].
See #46683.

[ocean90]

In 45244:

Site Health: Enable JavaScript translations for the site-health script.

See #46683.