207421 - XSLT format-number() function formats number incorrectly causing it to be rounded down and the fractional part to begin with a colon (:)

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Peter Ryan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

The following:

  <xsl:value-of select="format-number(999999999999.00, '#,##0.00##')" />

outputs "999,999,999,999.00" as expected, however this:

  <xsl:value-of select="format-number(999999999998.00, '#,##0.00##')" />

outputs "999,999,999,997.:0" which isn't what I expected!

The same test in Saxon and IE6 (MS-XML 3?) works as I would expect.

Reproducible: Always

Steps to Reproduce:
1. Go to
http://clickindustrial.com/test/public/2003-05-25/moz-format-number/index.xml to
see the problem.
2. The stylesheet is accessible at
http://clickindustrial.com/test/public/2003-05-25/moz-format-number/index.xsl



Problem discovered in Firebird 0.6 but reproduceable in Mozilla 1.3

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

works for me in a recent build. Reporter, please try this with a more recent
nightly or with the 1.4rc1 release.

Reopen this bug if it still doesn't work or mark it as verified if it works.

Comment 2

[Peter Ryan]

Tried 1.4rc1 on the same Win2k Pro SP3 machine and it still fails. (I had
removed the old 1.3 version from Program Files prior to install, however the
same user profile was being used - I suspect this irelevant, but mention it just
in case!)

I also tried the test on a Linux (Red Hat 7) machine, using an older Moz build
(User-Agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.3) Gecko/20030312) and
it works fine.

So perhaps this affects win32 or Windows 2000 only?
When I get a chance, I'll test 1.4rc1 on WinXP.

Prior to filling this bug, I did post the problem to:
http://www.mozillazine.org/forums/viewtopic.php?t=11859

where another person did have the same problem, so I don't believe its a problem
with my OS installation.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I know we used to have this problem, but it was fixed back in bug 156594, so
that might be what the other mz-person is seeing.

Hmm.. I still can't reproduce this on my win2000 mashine. Do you have yours set
to any particular language? And are you *sure* you're testing using the 1.4rc1
build? Please open Help->AboutMozilla just to be sure.

Comment 4

[Peter Ryan]

Definately testing with 1.4rc1.
I have now additonally tested on NT4 and WinXP and got screen grabs to prove it!

1). http://clickindustrial.com/test/public/2003-05-25/moz-format-number/w2k.png
2). http://clickindustrial.com/test/public/2003-05-25/moz-format-number/linux.png
3). http://clickindustrial.com/test/public/2003-05-25/moz-format-number/nt4.png
4). http://clickindustrial.com/test/public/2003-05-25/moz-format-number/winxp.jpg

1). My crufty Win2000pro machine. This has had previous Moz's on it and all
sorts of junk, so if it were only this machine failing, I'd blame the OS!
2). Linux RedHat7 and Mozilla 1.3.  This is the *only* working system I have.
3). NT4. This machine is *very* clean. Just NT4, SP6a, MS-SQL7, NT Option pack,
VNC and thats about it. It has never had any previous Moz version on it.
4). WinXP. This machine has never had a previous Moz version on it, but does
have various other junk.

Just to re-cap. I originally found the problem in Moz-Firebird 0.6 on my Win2K
machine. At the time I still had Moz 1.3 loaded and this exhibited the same
problem. I've since loaded Moz 1.4rc1 and this too has the same problem.
Moz-Firebird 0.6 on a very clean NT4 machine and slightly less clean WinXP
machine also exhibits the same problem.

But hey, Linux is fine so I guess that tells me something!

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This is *very* weird, I can reproduce this with an official 1.3 or 1.4rc1 build,
but not with my home-made tip-build. And there has been no functional changes to
format-number() since 1.2.

The only thing I can think of is if NSPR is broken and behaves differently in
official builds.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

cc'ing wtc since I suspect NSPR returns the wrong string from PR_dtoa. Basically
it seems like PR_dtoa returns a string like "999999999997:0" instead of the
expected "99999999999800". Wtc: do you know if NSPR has had, or do have any
problems like this. We're using mode 0 with ndigits 0. The call is located at

http://lxr.mozilla.org/mozilla/source/extensions/transformiix/source/xslt/functions/txFormatNumberFunctionCall.cpp#298

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Cc'ing khanson too since he knows dtoa

Comment 8

[Peter Van der Beken [:peterv]]

CC'ing dbradley, I think he once solved a similar problem.

Comment 9

[Wan-Teh Chang]

Jonas, Peter, I am not familiar with PR_dtoa.  Sorry.
That function was originally added for JavaScript
before JavaScript had its own copy (jsdtoa.c).  So
someone working on JavaScript (e.g., khanson) might
be able to help.

PR_dtoa is based on David M. Gay's dtoa.c.  We are
using an old version, released in 1991.  You might
want to check the dtoa.c site to see if this is a
known bug that has been fixed in the current version.
See bug 108305 for the location of the dtoa.c site.

Comment 10

[Peter Van der Beken [:peterv]]

Dbradley points out that this looks similar to bug 160602 which was about a bug
in jsdtoa.

Comment 11

[Peter Van der Beken [:peterv]]

Also, see bug 140852.

Comment 12

[Wan-Teh Chang]

Attachment: Proposed patch: compile prdtoa.c with /Op

This is indeed the same as bug 140852.	It happens if you compile
prdtoa.c optimized with Visual C++ 6.0 Processor Pack.	If you
did not install the Processor Pack, you don't have this problem.

The same fix works.  I am only compiling prdtoa.c with /Op.  I'm
not using /Op on other files in NSPR.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

wtc: do you think we could get this on the 1.4branch? Seems kind'a serious since
apparently many big numbers get wrongly formatted? Who should r/sr?

Comment 14

[Wan-Teh Chang]

Anyone familiar with bug 140852 and its solution can
review my patch.  My patch is the same as the fix for
that bug except that I use the /Op flag only on prdtoa.c.
I've verified that that's sufficient.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 125006 [details] [diff] [review]
Proposed patch: compile prdtoa.c with /Op

requesting review. This patch is basically the same as from bug 140852, but
enables it for the NSPR-copy of dtoa.c

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

wtc: What is the protocol for landing stuff in NSPR? Do you think this patch is
safe enough to land on the 1.4 branch? It would be very nice to have since it is
a regression and it breaks outputting large numbers in XSLT (and i suspect any
other place that uses NSPR to convert double->string)

Comment 18

[Wan-Teh Chang]

Changed product from Browser:XSLT to NSPR.

Comment 19

[Wan-Teh Chang]

Comment on attachment 125006 [details] [diff] [review]
Proposed patch: compile prdtoa.c with /Op

Requesting mozilla 1.4 approval.  This is a low risk
build change.  It disables certain floating-point
related compiler optimizations on prdtoa.c so that
the floating-point calculations in that file produce
the correct results.  This is a known problem (bug
140852) that has been fixed in JavaScript's copy of
this file (jsdtoa.c).  I am merely applying the same
fix to prdtoa.c.

Comment 20

[Wan-Teh Chang]

Comment on attachment 125006 [details] [diff] [review]
Proposed patch: compile prdtoa.c with /Op

Patch checked into NSPR tip (4.4+) and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.5
alpha).

Comment 21

[Christopher Blizzard (:blizzard)]

This is actually biting me on linux on the s390x as well.  Is there an easy way
to disable optimization for this file on that platform as well?

Comment 22

[Wan-Teh Chang]

Chris, if you tell me what compiler flag I should use, I
can put together a patch to disable optimization for prdtoa.c
on linux on the s390x.

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

requesting blocking since this affects multiple platforms and is lowrisk. The
reason it is lowrisk is that the same compilerflag has been set on a copy of
this file in the js-engine and has worked fine there since 1.3. See also comment 19

Comment 24

[Christopher Blizzard (:blizzard)]

I'm not sure which flags it does use, wouldn't that be part of what's generated
from autoconf?  We just need to remove any -O type flags.

Comment 25

[Wan-Teh Chang]

Sorry I wasn't clear.  I wanted to know if there is a gcc
flag that disables only the optimizations that make the
floating-point calculations in prdtoa.c produce incorrect
results.  If there is none, then we can remove any -O type
flags, disabling *all* optimizations.

Can you try -ffloat-store? (See bug 140852 comment 34.)
Do this:

1. Understand the proposed patch for this bug.

2. In your build tree, edit nsprpub/pr/src/misc/Makefile
and add the following underneath the change I added for
WINNT:

ifeq ($(OS_ARCH),Linux)
$(OBJDIR)/prdtoa.$(OBJ_SUFFIX): prdtoa.c
	@$(MAKE_OBJDIR)
	$$(CC) -o $@ -c $(CFLAGS) -ffloat-store $<
endif

Note the use of tabs at the beginning of makefile rule
commands.

3. In nsprpub/pr/src/misc, remove prdtoa.o.

4. Change directory to nsprpub and do gmake.
This will recompile prdtoa.c with -ffloat-store and
rebuild libnspr4.so.

Comment 26

[Christopher Blizzard (:blizzard)]

Using -ffloat-store didn't seem to make a difference for me.

Comment 27

[Christopher Blizzard (:blizzard)]

Attachment: patch that disables optimization for prdtoa.c

Comment 28

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 125441 [details] [diff] [review]
patch that disables optimization for prdtoa.c

doesn't this need to be done for the js-copy as well?

Comment 29

[Christopher Blizzard (:blizzard)]

I'm not sure.  I haven't had any problems with the JS version, but it might be
that I haven't exercised it enough.  Any suggestions how to do that?

Comment 30

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

just evaluate the js

alert(String(819187200000));

It should give you something other then "819187200000" if the bug is in js too

Comment 31

[Asa Dotzler [:asa]]

Comment on attachment 125006 [details] [diff] [review]
Proposed patch: compile prdtoa.c with /Op

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 32

[Samir Gehani]

a=adt to land this on the 1.4 branch.

Comment 33

[Wan-Teh Chang]

Patch checked into MOZILLA_1_4_BRANCH (Mozilla 1.4 final).