Closed Bug 557371 Opened 11 years ago Closed 10 years ago

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


(Core :: JavaScript Engine, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)




(Whiteboard: fixed-in-tracemonkey)


(1 file, 1 obsolete file)

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.
Started an es5-discuss thread on this:
Attached patch Patch and test (obsolete) — Splinter Review
Assignee: general → jwalden+bmo
Attachment #500724 - Flags: review?(jorendorff)
Comment on attachment 500724 [details] [diff] [review]
Patch and test

This corner of ES5 is gross. Oh well.

Attachment #500724 - Flags: review?(jorendorff) → review+
Whiteboard: fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b10
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.

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.
Whiteboard: fixed-in-tracemonkey
Target Milestone: mozilla2.0b10 → ---
Attachment #500724 - Flags: review?(jwalden+bmo)
Blocks: 633256
Duplicate of this bug: 635009
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.
Attachment #500724 - Attachment is obsolete: true
Attachment #514495 - Flags: review+
Attachment #500724 - Flags: review?(jwalden+bmo)
Whiteboard: fixed-in-tracemonkey
Closed: 10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 633256
You need to log in before you can comment on or make changes to this bug.