Object.prototype.toLocaleString() wrong

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Yuh-Ruey Chen, Assigned: mrbkap)

Tracking

({js1.6, verified1.8})

Trunk
x86
Windows XP
js1.6, verified1.8
Points:
---
Bug Flags:
blocking1.8b5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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'; }};
o.toLocaleString();

Actual Results:  
"[object Object]"

Expected Results:  
"foo"
(Reporter)

Comment 1

12 years ago
Created attachment 196309 [details]
testcase
(Reporter)

Comment 2

12 years ago
Forgot to mention that both IE6 and Opera8 have the correct behavior.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Easy to fix, alfred says it comes up now and then.  Blake, can you take this bug
and fix it for 1.8b5?

/be
Flags: blocking1.8b5?
Keywords: js1.6
(Assignee)

Comment 4

12 years ago
Sure.
Assignee: general → mrbkap
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

12 years ago
Created attachment 197729 [details] [diff] [review]
Implement toLocaleString in the image of ECMA-262

Brendan, is this something like what you had in mind?
Attachment #197729 - Flags: review?(brendan)
(Assignee)

Comment 6

12 years ago
Created attachment 197730 [details] [diff] [review]
Actually compiling

It helps to change _all_ occurances of a changed function namne...
Attachment #197729 - Attachment is obsolete: true
Attachment #197730 - Flags: review?(brendan)
(Assignee)

Updated

12 years ago
Attachment #197729 - Flags: review?(brendan)

Comment 7

12 years ago
Is this a regression? What's the risk of taking this patch? Bob, do we have
tests around this?

Comment 8

12 years ago
(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.

Flags: testcase?

Comment 9

12 years ago
2005-09-29 12:09	bob%bclary.com 	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.
Flags: testcase? → testcase+
(Assignee)

Updated

12 years ago
Attachment #197730 - Flags: review?(shaver)
Comment on attachment 197730 [details] [diff] [review]
Actually compiling

r=shaver
Attachment #197730 - Flags: review?(shaver) → review+
(Assignee)

Comment 11

12 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

12 years ago
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).
Attachment #197730 - Flags: approval1.8b5?
Comment on attachment 197730 [details] [diff] [review]
Actually compiling

Nit: canonical name for a JSString * is str.

/be
Attachment #197730 - Flags: review?(brendan) → review+

Updated

12 years ago
Attachment #197730 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 14

12 years ago
Fix checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8

Updated

12 years ago
Flags: blocking1.8b5? → blocking1.8b5+

Comment 15

12 years ago
verified fixed 1.8.x and trunk.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8 → verified1.8
You need to log in before you can comment on or make changes to this bug.