Make WordPress Core

Opened 8 years ago

Last modified 3 weeks ago

#37287 assigned defect (bug)

wp_print_admin_notice_templates() does not use _n*() for plural forms

Reported by: ideag's profile ideag Owned by: swissspidy's profile swissspidy
Milestone: 6.6 Priority: normal
Severity: normal Version: 4.6
Component: I18N Keywords: needs-patch
Focuses: javascript, administration Cc:

Description

Function wp_print_admin_notice_templates() (https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/update.php#L615) does not properly use _n() and related functions for plural forms of its strings. It just makes a simple if (count ===1) check and uses two different strings - one for singular, one for plural form.

This presents a problem when translating into languages that have more than one plural form, or more complicated rules for singular form usage. For example, in Lithuanian we also use singular form for 21, 1241 and any other number that ends with 1 (except for 11). And we have two plural forms - one for numbers ending with a zero, another for everything else. In current situation we can not have a proper translation here.

Attachments (2)

Capture d’écran 2022-10-04 à 08.58.29.png (58.1 KB) - added by audrasjb 18 months ago.
After patch: singular form - works fine
Capture d’écran 2022-10-04 à 08.58.35.png (55.4 KB) - added by audrasjb 18 months ago.
After patch: plural form - works fine

Download all attachments as: .zip

Change History (23)

This ticket was mentioned in Slack in #feature-shinyupdates by arunas. View the logs.


8 years ago

#2 @ideag
8 years ago

OK, as I understand, this will not get fixed until #20491 gets fixed. So not in time for 4.6 or anything like it. Just use generic strings.

#3 follow-up: @SergeyBiryukov
8 years ago

Noticed the same when translating yesterday. Yes, it depends on #20491 to be implemented properly (we can't just use _n() here, as it's a JS template).

For now, I would use some kind of a workaround in translation, e.g. "Plugins (%s) successfully updated".

#4 @ocean90
8 years ago

  • Focuses javascript added
  • Milestone changed from Awaiting Review to Future Release

#5 in reply to: ↑ 3 @ideag
8 years ago

Replying to SergeyBiryukov:

Noticed the same when translating yesterday. Yes, it depends on #20491 to be implemented properly (we can't just use _n() here, as it's a JS template).

For now, I would use some kind of a workaround in translation, e.g. "Plugins (%s) successfully updated".

Well, we could hold of on introducing faulty logic into the core and use generic strings right there.
But I guess shiny updates have to be, well, shiny. At least in English :)

I haven't looked into where data.successes comes from, but if it originates in PHP, maybe we could pass a pluralized string to it readily instead of having that logic in JavasScript at all? OK, I see we can't do that here.

Last edited 8 years ago by ideag (previous) (diff)

This ticket was mentioned in Slack in #polyglots by pokeraitis. View the logs.


8 years ago

#7 @swissspidy
8 years ago

#37811 was marked as a duplicate.

#8 @desrosj
5 years ago

  • Keywords needs-patch added

This ticket was mentioned in PR #2778 on WordPress/wordpress-develop by daledupreez.


22 months ago
#9

  • Keywords has-patch added; needs-patch removed

This PR updates the template used for bulk updates to use client-side localisation via wp.i18n._n() to handle plurals correctly. I do have some concerns, though:

  • Will these translations be correctly picked up now that they're in server code that is using the client-side functions?
  • What is the correct way to add translation comments for the code approach above?

Trac ticket: https://core.trac.wordpress.org/ticket/37287

#10 @SergeyBiryukov
22 months ago

  • Milestone changed from Future Release to 6.1

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


18 months ago

@audrasjb
18 months ago

After patch: singular form - works fine

@audrasjb
18 months ago

After patch: plural form - works fine

#12 @audrasjb
18 months ago

  • Keywords dev-feedback added

The patch works fine on my side.

pinging @SergeyBiryukov for additional review, but I think we're good to go with PR2778.

#13 @audrasjb
17 months ago

  • Keywords commit added; dev-feedback removed
  • Owner set to audrasjb
  • Status changed from new to accepted

Self assigning for commit.

#14 @audrasjb
17 months ago

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

In 54469:

I18N: Use wp.i18n._n() for plural forms in wp_print_admin_notice_templates().

This changeset adds better support for plural forms in update admin notices generated on the Themes and Plugins screens. This fixes issues when translating into languages that have more than one plural form, or more complicated rules for singular form usage.

Props ideag, SergeyBiryukov, daledupreez, audrasjb.
Fixes #37287.

#15 @ocean90
17 months ago

  • Keywords revert added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening and suggesting revert of [54469] for now since there's currently no support for parsing JS i18n functions in PHP files which means the strings won't be translatable.

#16 @audrasjb
17 months ago

Oh, alright, I'm reverting it right now.
I'll move it to Future Release after it's reverted. Thanks.

#17 @audrasjb
17 months ago

In 54473:

I18N: Revert [54469].

This changeset reverts [54469] as there is currently no support for parsing JS i18n functions in PHP files which means the strings won't be translatable.

Props ocean90.
Unprops audrasjb.
See #37287.

#18 @audrasjb
17 months ago

  • Keywords revert removed
  • Milestone changed from 6.1 to Future Release

#19 @daledupreez
17 months ago

Thanks for flagging that gap, @ocean90! This was one of my original concerns with my PR.

Do you know if we have an issue for adding that support to our translation tools? I'd like to make sure we flag this issue as blocked by that support (or possibly a motivator for adding it!).

#20 @ocean90
17 months ago

  • Keywords needs-patch added; has-patch removed

@daledupreez I think the proper solution would be to move this somehow into the existing JavaScript file instead of having this template in PHP.

#21 @swissspidy
3 weeks ago

  • Milestone changed from Future Release to 6.6
  • Owner changed from audrasjb to swissspidy
  • Status changed from reopened to assigned
Note: See TracTickets for help on using tickets.