Opened 8 years ago

Last modified 3 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)

number.js.diff (1.0 KB) - added by ChiefPilot 8 years ago.
Possible workaround / solution for dojo number.js
number.js.2.diff (1.5 KB) - added by duong.nguyen 6 years ago.
Fixed number._formatAbsolute method to works woth exponential string representation of very large and small numbers.
numberTest.diff (549 bytes) - added by duong.nguyen 6 years ago.
number.js_regular_expression.png (25.8 KB) - added by barbossusus 6 years ago.

Download all attachments as: .zip

Change History (17)

Changed 8 years ago by ChiefPilot

Attachment: number.js.diff added

Possible workaround / solution for dojo number.js

comment:1 Changed 8 years ago by Adam Peller

Type: defectenhancement

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 in reply to:  1 Changed 8 years ago by ChiefPilot

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 8 years ago by Adam Peller

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 8 years ago by Adam Peller

...but it's true it would be better to fail outright, rather than produce this kind of result. See also #8756

comment:5 Changed 7 years ago by bill

Component: GeneralCore
Owner: set to Adam Peller
Status: newassigned

Maybe you want to close as patchwelcome, or leave open to add an error message.

comment:6 Changed 7 years ago by Adam Peller

Resolution: patchwelcome
Status: assignedclosed

Changed 6 years ago by duong.nguyen

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 6 years ago by duong.nguyen

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 6 years ago by bill

Resolution: patchwelcome
Status: closedreopened

Can you add a test to your patch?

Reopening this ticket since it is marked "patchwelcome".

Changed 6 years ago by duong.nguyen

Attachment: numberTest.diff added

comment:9 Changed 6 years ago by duong.nguyen

Number test patch test is added

comment:10 Changed 6 years ago by barbossusus

This may be a better regular expression: dojo.number._toString = function(value){ ... scientificNotation = valueString.match(/[+\-]*((?:0|[1-9])\d*)(?:[\.][0-9]*){0,1}[eE]([+\-])([0-9]*)/); ... }

Changed 6 years ago by barbossusus

comment:11 Changed 6 years ago by barbossusus

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 3 years ago by dylan

Milestone: tbd1.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 3 years ago by dylan

Milestone: 1.131.15

Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.

Note: See TracTickets for help on using tickets.