Closed Bug 509184 Opened 12 years ago Closed 12 years ago

FF 3.5.2 native JSON.stringify replacer applied only to array items, not object properties


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
blocking2.0 --- beta1+


(Reporter: lsmith, Assigned: sayrer)




(Keywords: regression, Whiteboard: fixed-in-tracemonkey)


(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/20090729 Firefox/3.5.2
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/20090729 Firefox/3.5.2

It appears that the replacer function passed as the second arg to JSON.stringify only allows value replacement to array items.  The only exception I've found is returning undefined from the replacer will remove object properties.

Reproducible: Always

Steps to Reproduce:
JSON.stringify({x:1,arr:[1]}, function (k,v) {
    return typeof v === 'number' ? 3 : value;
Actual Results:  

Expected Results:  
Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090815 Minefield/3.7a1pre's json2.js implementation gives '{"x":3,"arr":[3]}' as the result. That's what you meant, right?
Component: General → DOM
Ever confirmed: true
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → general
Doh!  Yes. IE8, json2.js, WebKit, and Chrome 3 return

This is correct according to the ES5 spec.  FF is catching the array items, but not the object properties.  Sorry for the bogus Expected value!
Oh, this is a regression! It actually worked properly for a brief period of time in May.

regression window:

Bisecting now...
Regression from bug 493586.
Assignee: nobody → general
Blocks: 493586
Component: DOM → JavaScript Engine
QA Contact: general → general
Assignee: general → sayrer
Duplicate of this bug: 512443
Attached patch WIP (obsolete) — Splinter Review
stupid, unnecessary code. mea culpa.

I think this tests out, gonna check with test suites from MS / Opera / WebKit, then add one of our own.
blocking2.0: --- → ?
Blocks: es5
Flags: wanted1.9.2?
Despite of stating that the issue will be fixed in I can still confirm it on (linux x86). It is blocking me from using native MOZ json entirely. 

Please update the release vision for fixing the issue so people could set the js code workarounds accoringly (i.e. [if < 1.9.1.X] => emulate ).
blocking2.0: ? → alpha1
blocking2.0: alpha1 → beta1
Attached patch fixSplinter Review
this fixes one problem, the patch in bug 512447 fixes another.
Attachment #398724 - Attachment is obsolete: true
Attachment #433951 - Flags: review?(jorendorff)
Attachment #433951 - Flags: review?(jorendorff) → review?(jwalden+bmo)
Attachment #433951 - Flags: review?(jwalden+bmo) → review?(jorendorff)
Comment on attachment 433951 [details] [diff] [review]

Hm, didn't realize you'd shifted this until just now -- looked it over, looks fine to me.

Any particular reason to move Str step 1 into callers, rather than having it in Str directly?  Does call overhead show up on benchmarks or something here?  If that's the case, it would seem preferable to make Str JS_ALWAYS_INLINE and have it tail-call "StrHelper" so as to match the structure of the spec.
Attachment #433951 - Flags: review?(jorendorff) → review+
The StrHelper idea, I think, would also make fixing bug 512447 easier, because then I think you might be able to avoid the not-in-the-spec-method callReplacer argument.
Whiteboard: fixed-in-tracemonkey
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Version: unspecified → Trunk
Duplicate of this bug: 548462
You need to log in before you can comment on or make changes to this bug.