Last Comment Bug 308806 - Object.prototype.toLocaleString() wrong
: Object.prototype.toLocaleString() wrong
: js1.6, verified1.8
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2005-09-16 04:19 PDT by Yuh-Ruey Chen
Modified: 2006-04-09 20:25 PDT (History)
7 users (show)
asa: blocking1.8b5+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (490 bytes, text/html)
2005-09-16 04:20 PDT, Yuh-Ruey Chen
no flags Details
Implement toLocaleString in the image of ECMA-262 (2.45 KB, patch)
2005-09-28 10:32 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Actually compiling (2.47 KB, patch)
2005-09-28 10:34 PDT, Blake Kaplan (:mrbkap)
brendan: review+
shaver: review+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Yuh-Ruey Chen 2005-09-16 04:19:48 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

Object.prototype.toLocaleString should return the result of toString(). Right
now, it's set to simply return "[object Object]". That means that if toString is
modified, calling toLocaleString() won't reflect the change.

Reproducible: Always

Steps to Reproduce:
var o = {toString: function() { return 'foo'; }};

Actual Results:  
"[object Object]"

Expected Results:  
Comment 1 Yuh-Ruey Chen 2005-09-16 04:20:34 PDT
Created attachment 196309 [details]
Comment 2 Yuh-Ruey Chen 2005-09-16 04:21:21 PDT
Forgot to mention that both IE6 and Opera8 have the correct behavior.
Comment 3 Brendan Eich [:brendan] 2005-09-28 07:14:30 PDT
Easy to fix, alfred says it comes up now and then.  Blake, can you take this bug
and fix it for 1.8b5?

Comment 4 Blake Kaplan (:mrbkap) 2005-09-28 10:30:06 PDT
Comment 5 Blake Kaplan (:mrbkap) 2005-09-28 10:32:03 PDT
Created attachment 197729 [details] [diff] [review]
Implement toLocaleString in the image of ECMA-262

Brendan, is this something like what you had in mind?
Comment 6 Blake Kaplan (:mrbkap) 2005-09-28 10:34:34 PDT
Created attachment 197730 [details] [diff] [review]
Actually compiling

It helps to change _all_ occurances of a changed function namne...
Comment 7 Asa Dotzler [:asa] 2005-09-29 11:49:29 PDT
Is this a regression? What's the risk of taking this patch? Bob, do we have
tests around this?
Comment 8 Bob Clary [:bc:] 2005-09-29 11:57:45 PDT
(In reply to comment #7)
> Is this a regression? 

I don't know off hand, but will find out asap.

> Bob, do we have tests around this?

No, but we will have in a few minutes.

Comment 9 Bob Clary [:bc:] 2005-09-29 12:11:03 PDT
2005-09-29 12:09 	mozilla/ js/ tests/ js1_5/ Object/
regress-308806-01.js 	1.1 	0/0  	Object.prototype.toLocaleString should track
Object.prototype.toString, Regression test for bug 308806 by Bryant Chen, not
part of the build

This test fails in Mozilla 1.4, Firefox 1.0 and today's Firefox 1.5.
Comment 10 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-09-29 14:13:38 PDT
Comment on attachment 197730 [details] [diff] [review]
Actually compiling

Comment 11 Blake Kaplan (:mrbkap) 2005-09-29 14:55:37 PDT
Fix checked into trunk.
Comment 12 Blake Kaplan (:mrbkap) 2005-09-29 14:56:57 PDT
Comment on attachment 197730 [details] [diff] [review]
Actually compiling

This fix is extremely safe (only affects callers of toLocaleString) and fixes a
standards compliance issue by simply doing what the spec tells us to do (return
the result of toString).
Comment 13 Brendan Eich [:brendan] 2005-09-29 15:00:39 PDT
Comment on attachment 197730 [details] [diff] [review]
Actually compiling

Nit: canonical name for a JSString * is str.

Comment 14 Blake Kaplan (:mrbkap) 2005-09-30 14:47:21 PDT
Fix checked into MOZILLA_1_8_BRANCH.
Comment 15 Bob Clary [:bc:] 2006-04-09 20:25:04 PDT
verified fixed 1.8.x and trunk.

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