Closed Bug 207421 Opened 21 years ago Closed 21 years ago

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

Categories

(NSPR :: NSPR, defect)

Product:
NSPR
Component:
NSPR
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: peterx14, Assigned: wtc)

References

(
URL
)

Details

(Keywords: fixed1.4, regression)

Attachments

(2 files)

Proposed patch: compile prdtoa.c with /Op
931 bytes, patch
netscape
: review+
asa
: approval1.4+
Details | Diff | Splinter Review
patch that disables optimization for prdtoa.c
466 bytes, patch
Details | Diff | Splinter Review 
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
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.
URL: http://clickindustrial.com/test/publi... → http://clickindustrial.com/test/publi...
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago
Resolution: --- → WORKSFORME
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.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
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.
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!
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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

Cc'ing khanson too since he knows dtoa
CC'ing dbradley, I think he once solved a similar problem.
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.
Dbradley points out that this looks similar to bug 160602 which was about a bug
in jsdtoa.
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.
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?
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.
Status: NEW → ASSIGNED
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
Attachment #125006 - Flags: review?(seawood)

        Attachment #125006 -
        Flags: review?(seawood) → review+

    
    

    
  
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)
Keywords: regression

    
    

    
  
Changed product from Browser:XSLT to NSPR.
Assignee: peterv → wtc
Status: ASSIGNED → NEW
Component: XSLT → NSPR
Product: Browser → NSPR
QA Contact: keith → wtc
Target Milestone: --- → 4.4
Version: Trunk → 4.3

    
    

    
  
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.

        Attachment #125006 -
        Flags: approval1.4?

    
    

    
  
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).

    
    

    
  
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?

    
    

    
  
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.

    
    

    
  
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
Flags: blocking1.4?

    
    

    
  
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.

    
    

    
  
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.

    
    

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

    
    

    
  
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?

    
    

    
  
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?

    
    

    
  
just evaluate the js

alert(String(819187200000));

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



    
    

    
  
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.

        Attachment #125006 -
        Flags: approval1.4? → approval1.4+

    
    

    
  
a=adt to land this on the 1.4 branch.

    
    

    
  
Patch checked into MOZILLA_1_4_BRANCH (Mozilla 1.4 final).
Status: NEW → RESOLVED
Closed: 21 years ago → 21 years ago
Keywords: fixed1.4
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: