Incorrect use of plural in class-wp-customize-themes-section.php

[tobifjellner]

Use of __() where _n() is needed.

/wp-includes/customize/class-wp-customize-themes-section.php

/* translators: %s: number of themes displayed. */
echo sprintf( __( '%s themes' ), '<span class="theme-count">0</span>' );

[danieltj]

I'm not too sure of the issue here, I could be wrong though. After looking at the code in question, the number that is used is hard-coded as 0 (zero).

So irrespective of whether we switch to using _n() or not, when loaded it will always say 0 themes. If this is correct, I'm tempted to change it to _x() instead so we can provide additional context about the 0 in this case.

[tobifjellner]

If the number really can be only 0 (zero), then the straightforward solution is to simply use a string with the number 0 explicitly given in the string.
However, if this value can be changed anywhere, then account should be made that many languages have several different plural forms, depending on the specific count.
Alternative approach, less nice, but workable:
Number of themes: followed by the number.

[swissspidy]

Unfortunately _n() can't be used as these strings are loaded in a JavaScript context, not in PHP. See also #42139.

Although the count is initially 0, it is updated in wp.customize.ThemesSection.updateCount().

We could add a separate string for when there's exactly 1 theme. Then, wp.customize.ThemesSection.updateCount() would switch strings accordingly. Or we really use something like Number of themes: %s or Themes: %s.

As mentioned in the other ticket, this won't be a problem after something like #20491 gets into core.

[celloexpressions]

Yes, this is something that should be addressed once we have a proper set of JS i18n functions in #20491. We don't have a singular workaround there for English because that could still result in issues for other languages. In the meantime, there is an option of leaving it slightly broken or using a colon-type syntax to sidestep the issue.

[dlh]

I think this string can now be translated with wp.i18n._n(), as can a handful of other strings in the Customizer JS with singular and plural forms for which various workarounds have been used. 42140.diff​ contains these updates.

For backcompat, I opted to leave unchanged the affected strings in PHP that have been localized into JS object properties until now. It seemed safe to switch %s theme string originally in question here to use _n(), though, for consistency with the JS translation.

I'm adding this to the 5.4 milestone in the hope that it can be a relatively quick improvement!

[dlh]

42140.2.diff​ fixes a couple missing spaces and formatting inconsistencies.

[whyisjake]

Bumping to 5.6 since we are close to closing out the beta.

[hellofromTonya]

Refreshes 42140.2.dff to the latest version of wordpress-develop's master branch.

[hellofromTonya]

Patch 41240.2.diff did not cleanly apply. Refreshed with 42140.3.diff.

@dlh @SergeyBiryukov Can you double check that all changes are appropriately applied? If yes, what is next for this one to land in 5.6 Beta?

[dlh]

Thanks, @hellofromTonya! I'll try to review the latest patch this week.

Other than that, I think the next thing is for a committer to review this ticket and make a decision. If I may take the liberty of pinging @ocean90, who I think has committed a few similar changes recently: Perhaps you'd be able to weigh in on the patch?

[hellofromTonya]

In talking with @dlh, he hasn't had time yet to review. This ticket needs more eyes on it. RC1 is tomorrow. punting this ticket to Future Release.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[SergeyBiryukov]

Just noting this would be consistent with the work done for some other scripts in 5.5:
https://core.trac.wordpress.org/query?summary=~wp.i18n&milestone=5.5

The patch looks mostly good at a glance, a few notes though:

• Any strings with placeholders need to keep their translator comments.
• In wp-includes/customize/class-wp-customize-themes-section.php, the _n() usage is incorrect: the third argument should be a number, not a string.
• In js/_enqueues/wp/customize/controls.js, It looks like countEl.text() now receives a %s themes string instead of just a number, but the associated container still has the same string, so it might be duplicated. This needs some testing.

Let's clean this up a bit in 5.7.

[poena]

Moves the theme-count span and makes use of the data.count value

[poena]

You were right, the text string was duplicated.

Would 42140.4 work?
What I am attempting to do is use {{ data.count }} as the number inside the _n(),
and move the <span class="theme-count"> so that the complete text string is inside it,
removing the duplication.

[hellofromTonya]

As we are minutes from starting 5.7 RC1 release, ran out of time for this one. Moving to 5.8 which will also allow the fix to run through a full beta cycle.

[hellofromTonya]

Today is 5.8 Beta 1. As there's been no activity and been punted 2x, punting to Future Release.

[tobifjellner]

Can we tag this ticket for 5.9 and simply change these lines into:

/* translators: %s: number of themes displayed. */
echo sprintf( __( 'Number of themes: %s' ), '<span class="theme-count">0</span>' );

@SergeyBiryukov what do you think?