557371 - Does JSON.stringify(new String("foo")) return '"foo"' unconditionally, or does it depend on String.prototype.toString?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Spec sez it depends on String.prototype.toString.  Chrome and WebKit both perform per spec; Firefox and Opera return the string object's primitive value.  My IE9 developer preview seems horked for some reason, so I can't tell what IE does here (does it even implement JSON?  really not sure).

Either behavior is somewhat reasonable, I kind of prefer our behavior but it's not to spec, so either this has to change or there has to be an erratum.

Comment 1

[Jeff Walden [:Waldo]]

Started an es5-discuss thread on this:

https://mail.mozilla.org/pipermail/es5-discuss/2010-April/003529.html

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Patch and test

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 500724 [details] [diff] [review]
Patch and test

This corner of ES5 is gross. Oh well.

r=me.

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/567f16dd81f3

Comment 5

[Jeff Walden [:Waldo]]

Backed out, I forgot (as always) about the xpcshell tests (dom/src/json/test/unit/test_encode.js, dom/src/json/test/unit/test_wrapper.js) that need to be changed whenever JSON is changed.  A followup's usually easy.  But those tests compare against json2.js, which is not compatible with ES5 (no, really), so that's a strike against a fast fix.  Worse yet, json2.js appears to pollute [BNS].prototype with toJSON methods not in ES5, and those additions mean the ES5 algorithm short-circuits before the changes here could take effect, so basically it's impossible to fix fast.

http://hg.mozilla.org/tracemonkey/rev/039f81de26e5

Punting after 4.0, at which time I *will* kill those non-js/src/tests JSON tests -- they've been a problem for JSON-hacking SpiderMonkey developers for too long.

Comment 7

[Jeff Walden [:Waldo]]

Attachment: Patch, updated and with the dom/src/json/test/unit tests fixed

I did the very minimum amount of work needed to make the DOM tests pass.  I'll file another bug to move/remove/etc. them.  Now waiting for the tree to be ready to take this.

Comment 8

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/c7e3c056478b

Comment 9

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/c7e3c056478b