PDA

View Full Version : [FIXED-654] Bug in Ext.util.Format.number



ExtMore
3 Dec 2009, 11:23 AM
Hi,

There's a problem with Ext.util.Format.number() when using "/i" format option to get international thousands separator and decimal point.

When using the format "0.000,00/i" everything is ok. But when using "0,00/i" to omit the thousands separator, then the decimal point is not changed to ',' as expected.

Here's a complete test case to be dropped in a subdirectory of the examples directory in the SDK.



<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title id='title'>Ext.util.Format.number problem</title>

<link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" />

<script type="text/javascript" src="../../adapter/ext/ext-base.js"></script>
<script type="text/javascript" src="../../ext-all-debug.js"></script>

<script type="text/javascript">
Ext.BLANK_IMAGE_URL = '../../resources/images/default/s.gif';

document.write("<table width=400>")

test(1);
test(1.1);
test(123);
test(123.456);
test(1123);
test(1123.456);

document.write("</table>");

function test(num) {
document.write("<tr><td>");

// Formats correctly with international thousands separator and decimal point
document.write(Ext.util.Format.number(num, '0.000,00/i'));

document.write("</td><td>");

// The decimal point is wrong when you omit thousands separator AND use '/i'
document.write(Ext.util.Format.number(num, '0,00/i'));

document.write("</td></tr>");
}
</script>
</head>
<body>
</body>
</html>
Here a proposed fix as a patch against ext-all-debug.js in 3.0.3.



*** ext-all-debug.js.orig Sun Oct 11 19:25:02 2009
--- ext-all-debug.js.fixed Thu Dec 3 11:04:28 2009
***************
*** 12215,12223 ****
}

var fnum = v.toString();
! if(hasComma){
! psplit = fnum.split('.');

var cnum = psplit[0], parr = [], j = cnum.length, m = Math.floor(j / 3), n = cnum.length % 3 || 3;

for(var i = 0; i < j; i += n){
--- 12215,12223 ----
}

var fnum = v.toString();
! psplit = fnum.split('.');

+ if(hasComma){
var cnum = psplit[0], parr = [], j = cnum.length, m = Math.floor(j / 3), n = cnum.length % 3 || 3;

for(var i = 0; i < j; i += n){
***************
*** 12231,12237 ****
if(psplit[1]){
fnum += dec + psplit[1];
}
! }

return (neg ? '-' : '') + format.replace(/[\d,?\.?]+/, fnum);
},
--- 12231,12241 ----
if(psplit[1]){
fnum += dec + psplit[1];
}
! } else {
! if(psplit[1]){
! fnum = psplit[0] + dec + psplit[1];
! }
! }

return (neg ? '-' : '') + format.replace(/[\d,?\.?]+/, fnum);
},

ExtMore
14 Dec 2009, 12:47 AM
What's the status on this?

Does "[UNKNOWN]" in subject mean that it has been seen by the dev team but not yet verified?

I am not pushing for immediate action or anything -- I realize that there are more urgent problems right now.

But some kind of ack that this bug has been registered, and that it will be fixed when the dust settles, would be nice. :)

Thanks!

ExtMore
14 Dec 2009, 3:05 AM
[...]
Does "[UNKNOWN]" in subject mean that it has been seen by the dev team but not yet verified?
[...]


OK, found the answer to this myself in the sticky thread here. Sorry.. (:|

abraxxa
8 Jan 2010, 6:22 AM
The same happens with 3.1.0.
In which version will /i be fixed?

ExtMore
24 Feb 2010, 8:45 AM
Reading the fine sticky about bug post status (https://www.extjs.com/forum/showthread.php?t=92770), I see that the "UNKNOWN" status is no longer used.

This report is well over 2 months old, and seems to have slipped -- therefore this gentle bump.. :)

Jamie Avins
24 Feb 2010, 9:23 AM
Thank you, this is now properly assigned.

Jamie Avins
5 Mar 2010, 4:17 PM
[type]: fix
[module]: Format.number
[id]: #654
[desc]: Fixed #654. Fixed issue with international formatting when not using the thousand's separator.

mikecar
11 Mar 2010, 6:50 PM
I was about to report the same bug, but fortunately I searched first :)
on the same subject, there is another problem with this function.
The last line reads:



return (neg ? '-' : '') + format.replace(/[\d,?\.?]+/, fnum);
according to my understanding of regexp, this says that :

replace a sequence of one or more of these characters : 0-9, comma, period, questionMark
with the formatted number.

I am quite sure this is not what the author intended. IMHO he typed the extra ? characters in error. These are quantifiers and/or greedy markers, and have no place or function in a character SET specification. I believe that what he meant is this:



return (neg ? '-' : '') + format.replace(/[\d,.]+/, fnum);
in other words, replace the FORMATTING characters only, with the formatted number,
preserving as much as possible of the format string.
(AFAIK, the period does not need to be escaped in a set, as it is not a wild card in that context)

So, why is this important ?
The way the format function is written intends to protect and preserve non-format characters. so,
the format :


'0,000.00%'
gives you 123,456.00%, and this is fine. However, the format :


'0,000.00?'
does NOT give you 123,456.00?, as it should.

I understand this is not very important, however, I hate seeing messed-up Regular Expressions, and since you are fixing this, I thought I should mention it.:D