WordPress.org

Make WordPress Core

Opened 9 days ago

Last modified 7 days ago

#44541 new enhancement

Text length should be localizable

Reported by: miyauchi Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

There are variables to define the text length and they are used in the function wp_trim_words(). And also, there are flag to switch word count type in .po file.

They should be localizable because the CJK (Chinese, Japanese, Korean) has double-width characters like Japanese Kanji and they are counted by chars.

  • The text length of the excerpt.
  • The text length of the excerpt in RSS feed.
  • The text length of comment in dashboard.
  • The text length of draft in dashboard.

My plan is that adds a number of text length into the .po to be able to localize.

These fixes are included in the WP Multibyte Patch plugin in the Japanese package. We will open some other tickets to remove this plugin from Japanese package.

Attachments (4)

44541.patch (17.0 KB) - added by miyauchi 9 days ago.
44541.2.patch (16.4 KB) - added by miyauchi 8 days ago.
Addde annotation
44541.3.patch (13.3 KB) - added by miyauchi 8 days ago.
fixed: https://core.trac.wordpress.org/ticket/44541#comment:6
44541.4.patch (13.3 KB) - added by miyauchi 7 days ago.
fixed: https://core.trac.wordpress.org/ticket/44541#comment:9

Download all attachments as: .zip

Change History (15)

@miyauchi
9 days ago

#1 @miyauchi
9 days ago

All of tickets that I want to open to remove WP Multibyte Patch plugin from Japanese package are listed on following issues.

https://github.com/miya0001/wpdev/issues?q=is%3Aissue+is%3Aopen+label%3Awp-multibyte-patch

#3 @birgire
8 days ago

Hi @miyauchi

I applied the patch on trunk and ran:

grunt watch --phpunit --group=l10n

but got

FAILURES!
Tests: 101, Assertions: 247, Failures: 12.

I see that travis runs this successfully, so it looks like I'm missing something.

ps: The @ticket 44541 annotation is missing on the test methods.

#4 @netweb
8 days ago

  • Milestone changed from Awaiting Review to 5.0

Thanks @miyauchi moving this to 5.0 to coincide with the existing related tickets

Related: #meta3163, #43829, #44296

@miyauchi
8 days ago

Addde annotation

#5 @miyauchi
8 days ago

Hi @birgire

I added annotation. :)

I ran same command on my mac and it looks good to me.

$ grunt watch --phpunit --group=l10n

...
...
...

...............................................................  63 / 101 ( 62%)
......................................                          101 / 101 (100%)

Time: 2.82 seconds, Memory: 102.25MB

OK (101 tests, 247 assertions)

Thanks :)

#6 @birgire
8 days ago

Thanks @miyauchi, it's most likely something regarding my install :)

So instead I just skimmed through the tests and here are few suggestions:

We should consider calling restore_previous_locale() before any $this->assert*, otherwise it will not restore the previous locale on failure, affecting other tests as well.

I noticed the

require_once dirname( dirname( dirname( dirname( __FILE__ ) ) ) ) . '/src/wp-admin/includes/dashboard.php';

Shouldn't it refer to the build directory?

What about:

require_once ABSPATH . 'wp-admin/includes/dashboard.php';

instead?

For these assertions:

$this->assertTrue( !! strpos( $result, $expect ) );

isn't there the possibility of a "false negative" when the string position is 0 ?

What about $this->assertContains() or $this->expectOutputRegex() instead?.

There are some unused variables, like:

$post = $this->factory()->post->create( $args );

what about:

$this->factory()->post->create( $args );

instead to simplify?

Could we define the long version of the "Lorem Ipsum" text to reuse it?

Cheers

#7 follow-up: @miyauchi
8 days ago

Hi @birgire

Thanks! I fixed as you recommended. :)

#9 in reply to: ↑ 7 @birgire
8 days ago

Replying to miyauchi:

Hi @birgire

Thanks! I fixed as you recommended. :)

@miyauchi that's great, thanks, just two more suggestions I have for now :)

1) Make sure the previous locale is restored in case of a test failure. So instead of:

/**
 * @ticket 44541
 */
function test_trims_to_20_counted_by_chars() {
	switch_to_locale( 'ja_JP' );
	$expected = substr( $this->long_text, 0, 20 ) . '…';
	$this->assertEquals( $expected, wp_trim_words( $this->long_text, 20 ) );
	restore_previous_locale();
}

we could consider moving restore_previous_locale() above the assertion:

/**
 * @ticket 44541
 */
function test_trims_to_20_counted_by_chars() {
	switch_to_locale( 'ja_JP' );
	$expected = substr( $this->long_text, 0, 20 ) . '…';
        $actual = wp_trim_words( $this->long_text, 20 );
	restore_previous_locale();
	$this->assertEquals( $expected, $actual );
}

Similar in these methods:

  • Tests_Formatting_WPTrimWords::test_trims_to_20_counted_by_chars_with_double_width_chars()
  • Tests_L10n::test_length_of_comment_excerpt_should_be_counted_by_words()
  • Tests_L10n::test_length_of_comment_excerpt_should_be_counted_by_chars()
  • Tests_L10n::test_length_of_comment_excerpt_should_be_counted_by_chars_in_Japanese()

2) In tests/phpunit/data/languages/ja_JP.po I noticed the src/ reference:

#. translators: This sets the text length for the comment excerpt.
#: src/wp-includes/comment-template.ph:599
msgctxt "comment_excerpt_length"
msgid "20"
msgstr "40"

#. translators: This sets the text length for the comment excerpt.
#: src/wp-admin/includes/dashboard.php:591
msgctxt "draft_length"
msgid "10"
msgstr "40"

The other cases seems to skip it, e.g.:

#. translators: This sets the text length for the excerpt.
#: wp-includes/formatting.php:3640
msgctxt "excerpt_length"
msgid "55"
msgstr "110"

Last edited 8 days ago by birgire (previous) (diff)

#10 @miyauchi
7 days ago

@birgire

Make sure the previous locale is restored in case of a test failure. So instead of:

Oh... Yes! Good catch!

Thanks!

Note: See TracTickets for help on using tickets.