#10555 closed defect (bug) (fixed)
number_format_i18n function doesn't check if the number is float or not before doing the formating
Reported by: | chombium | Owned by: | nbachiyski |
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | I18N | Keywords: | has-patch dev-feedback |
Focuses: | Cc: |
Description
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
Attachments (4)
Change History (25)
#2
@
11 years ago
- Keywords needs-testing added
- Version changed from 2.8.3 to 2.9
Proper/refreshed patch. Could use a test.
#3
@
11 years ago
- Cc westi added
- Keywords early added
- Milestone changed from 2.9 to 3.0
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
@
11 years ago
Alternative patch which preserves the functionality if the callee supplies a precision
#4
@
11 years ago
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
#5
follow-up:
↓ 6
@
11 years ago
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?
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
11 years ago
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.
#7
in reply to:
↑ 6
@
11 years ago
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.
#8
@
10 years ago
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.
#9
follow-up:
↓ 10
@
10 years ago
Since we're on the topic, wouldn't it be nice to rename the function to number_i18n
, to be consistent with date_i18n
?
#10
in reply to:
↑ 9
;
follow-up:
↓ 11
@
10 years ago
Replying to scribu:
Since we're on the topic, wouldn't it be nice to rename the function to
number_i18n
, to be consistent withdate_i18n
?
It is consistent. The PHP functions we are replicating are number_format()
and date()
.
#11
in reply to:
↑ 10
@
10 years ago
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 withdate_i18n
?
It is consistent. The PHP functions we are replicating are
number_format()
anddate()
.
Then again, PHP isn't know for it's consistent function names...
Either way, I like typing less. :-)
#12
follow-up:
↓ 13
@
10 years ago
- Keywords dev-feedback added; needs-testing early removed
- Status changed from new to accepted
Any objections to the patch?
#13
in reply to:
↑ 12
@
10 years ago
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 :-)
#15
follow-up:
↓ 16
@
10 years ago
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.
#16
in reply to:
↑ 15
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
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
functions.php diff file