Opened 10 years ago
Last modified 5 years ago
#14066 reopened enhancement
[patch] dojo.number.format may misformat very large numbers
Reported by: | ChiefPilot | Owned by: | Adam Peller |
---|---|---|---|
Priority: | high | Milestone: | 1.15 |
Component: | Core | Version: | 1.6.1 |
Keywords: | number format exponent | Cc: | |
Blocked By: | Blocking: |
Description
Using dojo.number.format with an exponent >= 21 returns a misformatted string. Using the Firebug console, the following input :
n=3.1415e21 str = dojo.number.format(n); console.log(str)
...generates the following output:
3.141
I *think* the issue happens in dojo.number._formatAbsolute() function within number.js, when it splits a string representation of the value apart using the decimal as the separator at line 192. This results in ['3', '1415e21']. Subsequent pattern matches then result in the 'e21' being stripped.
A possible workaround or solution is attached. This alters the behavior such that the exponent is preserved if present. It does not alter the underlying javascript behavior of changing large values into exponential format.
Attachments (4)
Change History (17)
Changed 10 years ago by
Attachment: | number.js.diff added |
---|
comment:1 follow-up: 2 Changed 10 years ago by
Type: | defect → enhancement |
---|
at this time, exponents are not supported and precision is limited (at least) by Javascript binary floating point representation. Patterns using 'E' generate a warning. Not sure it's worth accommodating e input given all these limitations.
comment:2 Changed 10 years ago by
Replying to peller:
at this time, exponents are not supported and precision is limited (at least) by Javascript binary floating point representation. Patterns using 'E' generate a warning. Not sure it's worth accommodating e input given all these limitations.
That makes sense and I saw several places in the code where such was stated. I believe there is a bug here, however, even for non-exponents. The input
n=10000000000000000000000 str = dojo.number.format(n); console.log(str)
...produces output of :
1e,+22
...which is clearly not correct. I discovered this shortly after submitting the ticket; I do not have a patch to submit at this time but am working on one and would be happy to share if there is any interest.
comment:3 Changed 10 years ago by
right. dojo.number basically doesn't handle large numbers, wherever scientific notation comes into play, nor is it able to deal with precision beyond (or perhaps even approaching) the limits of binary floating point.
comment:4 Changed 10 years ago by
...but it's true it would be better to fail outright, rather than produce this kind of result. See also #8756
comment:5 Changed 9 years ago by
Component: | General → Core |
---|---|
Owner: | set to Adam Peller |
Status: | new → assigned |
Maybe you want to close as patchwelcome, or leave open to add an error message.
comment:6 Changed 9 years ago by
Resolution: | → patchwelcome |
---|---|
Status: | assigned → closed |
Changed 8 years ago by
Attachment: | number.js.2.diff added |
---|
Fixed number._formatAbsolute method to works woth exponential string representation of very large and small numbers.
comment:7 Changed 8 years ago by
This issue also makes NumberTextBox? validation fails on both very large and very small absolute values, on which toString returns exponential format. I have attached a patch even though it does not fix the precision issue.
comment:8 Changed 8 years ago by
Resolution: | patchwelcome |
---|---|
Status: | closed → reopened |
Can you add a test to your patch?
Reopening this ticket since it is marked "patchwelcome".
Changed 8 years ago by
Attachment: | numberTest.diff added |
---|
comment:10 Changed 8 years ago by
Changed 8 years ago by
Attachment: | number.js_regular_expression.png added |
---|
comment:11 Changed 8 years ago by
Please ignore the previous regular expression. This would be a better one: dojo.number._toString = function(value){ ...................................................................................
var valueString = String(value),
scientificNotation = valueString.match(/([+\-]{0,1}(?:0|[1-9][0-9]*)[\.][0-9]*){0,1}[eE]([+\-])([0-9]*)/);
................................................................................... }
Also, see attached screenshot. Thanks.
comment:12 Changed 5 years ago by
Milestone: | tbd → 1.12 |
---|---|
Summary: | dojo.number.format may misformat very large numbers → [patch] dojo.number.format may misformat very large numbers |
Need to verify CLA and integrity of patch. Will review for 1.12.
comment:13 Changed 5 years ago by
Milestone: | 1.13 → 1.15 |
---|
Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.
Possible workaround / solution for dojo number.js