Make WordPress Core

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#31859 closed defect (bug) (fixed)

Avoid using HTML tags in translation strings - remove the count class

Reported by: ramiy's profile ramiy Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

Ok guys, it's a huge mission, but someone have to do it.

We need to replace all the translation strings containing <span class="count">(%s)</span> and replace them with only %s.

Before: All <span class="count">(%s)</span>
After: All %s

We have 20 string to change accross the core:

http://i.imgur.com/tAZuzQk.png

For the full list, go to
https://translate.wordpress.org/projects/wp/dev/admin/he/default?filters%5Bterm%5D=class%3D%22count%22&filters%5Bstatus%5D=current_or_waiting_or_fuzzy_or_untranslated

Related: #31858, #31857, #31855, #31853, #31847

Attachments (8)

31859-remove-count-class.diff (8.7 KB) - added by Tmeister 9 years ago.
31859-remove-count-class-2.diff (11.0 KB) - added by Tmeister 9 years ago.
31859-remove-count-class-3.diff (11.5 KB) - added by Tmeister 9 years ago.
31859-remove-count-class-4.diff (12.1 KB) - added by Tmeister 9 years ago.
31859.idea.patch (1.7 KB) - added by ocean90 9 years ago.
31859.patch (2.2 KB) - added by SergeyBiryukov 8 years ago.
31859.2.patch (2.3 KB) - added by SergeyBiryukov 8 years ago.
31859.3.patch (2.5 KB) - added by SergeyBiryukov 8 years ago.

Download all attachments as: .zip

Change History (33)

#1 @Tmeister
9 years ago

  • Keywords has-patch added

This is what I could find.

Patch attached.

#2 @ramiy
9 years ago

Great work! Thanks for the patch.

But it seems like few string are missing:

  • Pending <span class="count">(<span class="pending-count">%s</span>)</span>
  • Spam <span class="count">(<span class="spam-count">%s</span>)</span>
  • Trash <span class="count">(<span class="trash-count">%s</span>)</span>
  • %1$s <span class="count">(%2$s)</span>

#3 @Tmeister
9 years ago

Strings added please test the last patch.

#4 @ramiy
9 years ago

  • Keywords needs-testing added

One string is missing - %1$s <span class="count">(%2$s)</span>

It's for the user roles in wp-admin/includes/class-wp-users-list-table.php line 169.

#5 @Tmeister
9 years ago

I don't know why that line needs translation

$name = sprintf( __('%1$s <span class="count">(%2$s)</span>'), $name, number_format_i18n( $avail_roles[$this_role] ) );

There is nothing to translate there, the translation is done in line 167

https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-users-list-table.php#L167-L169

Please test the latest patch.

Last edited 9 years ago by Tmeister (previous) (diff)

#6 @DrewAPicture
9 years ago

  • Version trunk deleted

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


9 years ago

#8 @pavelevap
9 years ago

I am not sure about it... We are not removing only HTML, but also brackets for numbers?

It was helpful sometimes to have HTML as a part of these strings as a good hint :-) It can be also replaced by translators comments...

#9 @ramiy
9 years ago

@Tmeister,
Can you add translation strings like @pavelevap asked.

Add this line above the strings:
/* translators: count */

#10 @Tmeister
9 years ago

Done, /* translators: count */ added.

#11 @ocean90
9 years ago

  • Keywords close added

This request seems to be wontfix since there are locales which are translating ( and ) too, see ticket:31251:17.

#12 follow-up: @SergeyBiryukov
9 years ago

We could still replace <span class="pending-count">%s</span> with %s.

#13 @ramiy
9 years ago

Actualy, there are two ways around it.

We can make ( and ) two separate strings.
This way the chinese language can use theire charecters.

Or we can leave only (%s) without the HTML.
This way HTML is out, and the ( and ) are in.

#14 @wonderboymusic
9 years ago

  • Milestone changed from Awaiting Review to 4.4
  • Owner set to SergeyBiryukov
  • Status changed from new to assigned

#15 in reply to: ↑ 12 ; follow-up: @ocean90
9 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

Replying to SergeyBiryukov:

We could still replace <span class="pending-count">%s</span> with %s.

Maybe, but is it worth the effort? I'm still voting for leaving it as it is.

#16 follow-up: @ocean90
9 years ago

Just had another idea: How would this look if we make <span class="count">(%s)</span> a separate string to translate?

#17 in reply to: ↑ 15 @ramiy
9 years ago

Replying to ocean90:

Replying to SergeyBiryukov:

We could still replace <span class="pending-count">%s</span> with %s.

Maybe, but is it worth the effort? I'm still voting for leaving it as it is.

From a translator point of view it's worth the effort.

#18 in reply to: ↑ 16 ; follow-up: @ramiy
9 years ago

Replying to ocean90:

Just had another idea: How would this look if we make <span class="count">(%s)</span> a separate string to translate?

It's a good idea!

(Unless some languages show the count before the text)

@ocean90
9 years ago

#19 in reply to: ↑ 18 @ocean90
9 years ago

Replying to ramiy:

(Unless some languages show the count before the text)

That's still possible, see 31859.idea.patch.

#20 @ramiy
9 years ago

:-)

Very nice workaround!

#21 @SergeyBiryukov
8 years ago

  • Keywords has-patch added; close needs-patch removed

31859.idea.patch seems to defeat the purpose of _n_noop() for correct plural forums there.

31859.patch is what I had in mind. It removes the non-translatable bits from those strings and makes them consistent with others.

This ticket was mentioned in Slack in #core-i18n by sergey. View the logs.


8 years ago

#23 @SergeyBiryukov
8 years ago

31859.3.patch adds a context, so that these strings don't clash with post status labels.

#24 @SergeyBiryukov
8 years ago

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

In 34424:

Remove extra HTML from translatable strings in WP_Comments_List_Table::get_views().

Add a context and translator comments.

Props Tmeister for initial patch.
Fixes #31859.

This ticket was mentioned in Slack in #core-i18n by ocean90. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.