number_format_i18n function doesn't check if the number is float or not before doing the formating

[chombium]

Hi,

Problem description:

I've noticed that there are problems in representation of decimal numbers. The number representation is defined by the following three values in the wordpress.pot translation file: number_format_decimals, number_format_decimal_point and number_format_thousands_sep.

In the Macedonian language translation of wordpress.pot which I
maintain, I've set this values to:
number_format_decimals = 3
number_format_decimal_point = ,
number_format_thousands_sep = .

This means that the proper decimal number representation would be 1.234,567

I've noticed that all the numbers including integers are shown with this format, so for example, I get "23,000" posts on the dashboard instead of "23".

I can fix this behavior by setting:
number_format_decimals = 0
in the translation, but if a floating point number has to be formated it will be truncated to its integer part.

This way all the floating point numbers are not formated correctly, but as I was told by Nikolay Bachiyski, this is the safe way to solve this issue because this function is called only with integers.

I dig trough the code of wp 2.8.3 and I've noticed that the problematic function is number_format_i18n located in wp-includes/functions.php
This function is the only place where php number_format function is
called with the parameters from the translation file.

Solution:

The function number_format_i18n has to be patched so that first it checks what kind of number should be formated (integer or float) and then to do the formating. This means it has to set the correct number of decimal places before calling the php number_format function.

This can be done by adding the line:

if (!is_float($number)) $decimals = 0;

after setting the number of decimals, the line:

$decimals = ( is_null( $decimals ) ) ? $wp_locale->number_format['decimals'] : intval( $decimals );

The proposed patch simply checks if the number which has to be formated is not float and if that is the case it sets the number of decimals to 0 ($decimals = 0)

I don't know if the file: wp-content/plugins/akismet/akismet.php has to be patched as well because it redeclares the number_format_i18n function if it doesn't exist. I see that the function is called with one parameter in this file, which means decimals = 0.

The diff file of wp-includes/functions.php is in the attachment.

Jovan

[chombium]

functions.php diff file

[nacin]

Proper/refreshed patch. Could use a test.

[westi]

Ok.

So the issue here is that if you specify the number of significant figures to show for a particular locale then that is used regardless of whether or not the number is a float.

I think at present everything we format is an integer hence the recommendation to stick with 0 for the value in the translation file.

I have an alternative patch for this but I think the best solution for you for 2.9 is to stick with 0 in the translation file like the other translators do and we can look at what to change here early in 3.0 - I wonder whether or not this value should be offered up for translation at all as it is doesn't feel like a locale issue how many significant figures are displayed

[westi]

Alternative patch which preserves the functionality if the callee supplies a precision

[chombium]

The problem that I experienced was because there was no documentation that only integers are passed to this function and that there is no remark that the value for the decimals in the translation file should always be 0.

Because the value is already in the translation file and if the function is patched, I think the best way to solve this issue is to add a remark in the translation file and to update the ​http://codex.wordpress.org/Translating_WordPress page, that for now, the msgid "number_format_decimals" should be set to 0.

The comment:
"#. translators: $decimals argument for ​http://php.net/number_format, default is 0"
in the file ​http://svn.automattic.com/wordpress-i18n/pot/trunk/wordpress.pot should be updated with something like "set it to 0" as well

[hakre]

I'm a bit confused. We have a pretty well report here, we have a parameter here "number_format_decimals" which actually is configured to values other than 0.

So if there is a parameter then it can get values other than zero and that function should refelct it.

If infact that parameter is not needed because we are only handling integers within that function, then this parameter is just overhead and should be removed.

If you're fighting with the reality that both is the case, why don't you just extend the function that it can deal with any case so everybody is happy?

[westi]

Replying to hakre:

I'm a bit confused. We have a pretty well report here, we have a parameter here "number_format_decimals" which actually is configured to values other than 0.

The point is that I think this was made translatable by accident.

I don't believe that the number of digits displayed after a floating point it something that a translation team would be expected to specify or need to specify.

They need to specify the thousands separator and the decimal point.

We have a translation recommendation to always set this to 0 because it will display numbers as floats.

I am going to take this discussion to the wp-polyglots list to see if there is a need there for this to remain translatable.

[hakre]

Replying to westi:

Replying to hakre:

I'm a bit confused. We have a pretty well report here, we have a parameter here "number_format_decimals" which actually is configured to values other than 0.

The point is that I think this was made translatable by accident.

Makes sense.

I am going to take this discussion to the wp-polyglots list to see if there is a need there for this to remain translatable.

Just update this ticket when you have feedback.

---

If anyone can reference, please do name a language / culture where it is common to have 3 places after the floating point. Can someone please confirm this for the Macedonian language as suggested by the reporter? Please add hyperlinks to that info.

[nbachiyski]

Everywhere in WordPress number_format_i18n is used for integers only. The simplest solution is to make it work only with integers and deprecate all the decimals stuff. If at any point we need the floating point behaviour, we can always put the functionality back.

My patch deprecates the decimal arguments for number_format_i18n and size_format and locale-specific decimals and decimal point settings.

[scribu]

Since we're on the topic, wouldn't it be nice to rename the function to number_i18n, to be consistent with date_i18n ?

[nbachiyski]

Replying to scribu:

Since we're on the topic, wouldn't it be nice to rename the function to number_i18n, to be consistent with date_i18n ?

It is consistent. The PHP functions we are replicating are number_format() and date().

[scribu]

Replying to nbachiyski:

Replying to scribu:

Since we're on the topic, wouldn't it be nice to rename the function to number_i18n, to be consistent with date_i18n ?

It is consistent. The PHP functions we are replicating are number_format() and date().

Then again, PHP isn't know for it's consistent function names...

Either way, I like typing less. :-)

[nbachiyski]

Any objections to the patch?

[westi]

Replying to nbachiyski:

Any objections to the patch?

Looks good to me.

Giving it a little love to fix the rejects and it will be in soon :-)

[westi]

(In [14184]) Remove unnecessary translations of decimal point character and number of decimal places. Fixes #10555 props nbachiyski.

[brentes]

I just updated to the latest and saw all the decimals in my plugin disappear.

I understand everything in WP uses integers, but please consider that many plugins, including all that deal with currency, use floats.

[westi]

Replying to brentes:

I just updated to the latest and saw all the decimals in my plugin disappear.

I understand everything in WP uses integers, but please consider that many plugins, including all that deal with currency, use floats.

We could restore this functionality by reverting part of this patch.

If we bring back the decimal seperator translation and just expect the callee to define the number of decimal places.

Would this be a suitable solution

[westi]

(In [14190]) Restore support for floating point numbers in number_format_i18n(). Fixes #10555.

[westi]

Try that on for size :-)

[brentes]

Thanks Westi!

Needs a slight change though. Line 139 casts the number as an integer. So number_format_i18n( 1123.5813, 2) returns 1123.00, when you'd expect 1123.58.

[nbachiyski]

(In [14192]) Do not force formatted number to int. Props brentes, fixes #10555