Bug 28118 - JSON.stringify replacer returning undefined does not omit object properties
Summary: JSON.stringify replacer returning undefined does not omit object properties
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://lucassmith.name/pub/JSON-test-...
Keywords: HasReduction, InRadar
Depends on:
Reported: 2009-08-08 21:17 PDT by Luke Smith
Modified: 2009-08-09 16:56 PDT (History)
1 user (show)

See Also:

Patch v1 (9.62 KB, patch)
2009-08-09 16:15 PDT, Oliver Hunt
staikos: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Smith 2009-08-08 21:17:29 PDT
JSON.stringify({remove:"me"}, function (k,v) { return k === 'remove' ? undefined : v; })  should return '{}', but returns '{"remove":null}'

Per the ECMA5 spec, undefined returned from a replacer should result in omission from the resulting JSON string in the case of object properties, and null in the case of array items.  Currently, null is used for both cases.

Comment 1 Mark Rowe (bdash) 2009-08-09 13:36:59 PDT
Comment 2 Oliver Hunt 2009-08-09 14:53:52 PDT
Goddammit -- fixed locally.  Stupid mistake, and i was sure this worked at one point.  It must have got broken when darin made this code iterative.  Apparently replacer functions aren't being tested as thoroughly as might be hoped.

Anyhoo, fixed locally just need to turn a build to verify that all is well, and add tests
Comment 3 Oliver Hunt 2009-08-09 16:15:46 PDT
Created attachment 34435 [details]
Patch v1
Comment 4 George Staikos 2009-08-09 16:33:57 PDT
Comment on attachment 34435 [details]
Patch v1

>      // Handle cycle detection, and put the holder on the stack.
>      if (!m_holderCycleDetector.add(object).second) {
> -        throwError(m_exec, TypeError);
> +        throwError(m_exec, TypeError, "JSON.stringify cannot serialise cyclic structures.");
>          return StringifyFailed;

   Should we not prefer US English?
Comment 5 George Staikos 2009-08-09 16:52:22 PDT
Comment on attachment 34435 [details]
Patch v1

r=me with the spelling fixed.
Comment 6 Oliver Hunt 2009-08-09 16:56:07 PDT
Committed r46967