About wp_date function when locale is 'ja'

[tmatsuur]

Version 5.3 adds the wp_date function.

If locale is 'ja' and 'F' is specified for the parameter of this function, an extra backslash is added to the return value.

Try dumping the return value of wp_date function and date_i18n function.

var_dump( wp_date( 'F' ), date_i18n( 'F' ) );

Dumped contents.

string(6) "\10月"
string(6) "\10月"

In the previous version, the return value of the date_i18n function has no backslash.

string(5) "10月"

I think the cause is in the part where the format is converted according to the locale.

wp-includes/functions.php : 259 - 261

case 'F':
	$new_format .= backslashit( $month );
	break;

This $new_format variable is used as is in the format method.

$date = $datetime->format( $new_format );

The date_i18n function of version 5.2.4 uses the preg_replace function during format conversion.

$dateformatstring     = preg_replace( '/([^\\\])F/', "\\1" . backslashit( $datemonth ), $dateformatstring );

Since wp_date function does not use preg_replace function, I don't think it is necessary to use backslashit function.

[ocean90]

@Rarst Can you confirm this report?

[Rarst]

Can confirm the regression.

First we still do need to slash. Date localisation is double-pass, first WP handles the localized bits, then PHP handles the rest. But since we don't want PHP to interpret parts we already processed as date format we escape them.

The problem is backslashit() has a special case of double-slashing numbers at start of string (I don't know why on top of my head). So in this case 1 turns into \\\\\1 (two escaped slashes).

We don't need this to happen, however since older code was more convoluted \\\\1 collapsed into \1 while being passed around (PHP handles double slashes as single slash whenever it processes a string) and didn't affect the output.

Since new code is more streamlined \\1 reaches the final processing and produces \1 in final output.

So we need to swap backslashit() use in this case with code that only escapes the letters (e.g. addcslashes( $string, 'A..Za..z' ) part of backslashit() implementation). Maybe more narrowly code that only escapes valid date format letters.

We could do this inline or introduce new helper function for clarity. Will think a bit and work on a patch.

cc @remcotolsma who wrote the new implementation for opinion. :)

[remcotolsma]

I don't follow why backslashit adds backslashes before a number at the start of a string:
​https://github.com/WordPress/WordPress/blob/5.2/wp-includes/formatting.php#L2615-L2628

Related commits:

• ​https://github.com/WordPress/WordPress/commit/f1aeebf00b6a38fb1bd1bb30f19bd8fe8ac0361d
• ​https://github.com/WordPress/WordPress/commit/f124c511077be7435ee4e5afafe46078224c06d1

It looks like it was added after the following Trac ticket en WordPress support topic:

• https://core.trac.wordpress.org/ticket/2774
• ​https://wordpress.org/support/topic/1-prefixed-to-the-date-of-all-posts-after-203-upgrade/

Back then it seems to be related to a Date/Time issue with preg_replace and older PHP versions. But since preg_replace is no longer used and WordPress requires PHP 5.6.20+ the backslashit function might even be simplified to:

<?php

function backslashit( $string ) {
        return addcslashes( $string, 'A..Za..z' );
}

But I think using addcslashes with only the date format characters is also a nice fix for this problem.

I have added a unit test to our own datetime library for this issue:
​https://github.com/pronamic/wp-datetime/commit/ff16527c5d7055c559dcf01760e6189f30044f42.

And a fix:
​https://github.com/pronamic/wp-datetime/commit/09424a2c3585f495f8dd6ea880c400f557c475f9

[Rarst]

I am hesitant to change backslashit() implementation. While de-facto core use seems to only be in date stuff, it's not documented as such and who knows how it might be used downstream.

So far I am thinking a conservative fix, changing to addcslashes( $string, 'A..Za..z' ) in wp_date() implementation. Not going to date format only yet. Probably more future proof too (e.g. what if new format character is added upstream).

[remcotolsma]

I also think there is currently an issue when in a translation a backslash is used.

global $wp_locale;

$month_translation = 'ABCD \ 1234';

$wp_locale->month['10'] = $month_translation;
$wp_locale->month_abbrev[ $month_translation ] = $month_translation;

echo date_i18n( 'F' ); // Result: 'ABCD  1234'

Maybe the charlist for addcslashes should be 'A..Za..z
'...

​https://github.com/pronamic/wp-datetime/commit/1b38e58f081db58c120ff808ce9f782d2ba3648f

We should add some unit tests for these cases in WordPress test library.

[Rarst]

Yeah, I was just thinking how are the slashes handled... If I understand it right there are also nuances with order of characters so that escaping slashes don't get themselves escaped.

[SergeyBiryukov]

Replying to Rarst:

So far I am thinking a conservative fix, changing to addcslashes( $string, 'A..Za..z' ) in wp_date() implementation. Not going to date format only yet. Probably more future proof too (e.g. what if new format character is added upstream).

Sounds good to me.

[SergeyBiryukov]

Marking for a second committer's review.

[peterwilsoncc]

LGTM, tests well and unit tests passing.

Couple of non-code changes in 48319.diff​:

• a little white space tidy-up
• added @ticket docblock to the second test so it's included when running npm run test:php -- --group 48319

[SergeyBiryukov]

In 46569:

Date/Time: Make sure wp_date() does not unnecessarily escape localized numbers, but keeps localized slashes.

Props Rarst, tmatsuur, remcotolsma, peterwilsoncc.
Reviewed by peterwilsoncc.
Fixes #48319.