Last Comment Bug 408838 - DOM binding for native JSON
: DOM binding for native JSON
Status: VERIFIED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: P1 normal with 4 votes (vote)
: mozilla1.9.1b1
Assigned To: Robert Sayre
:
Mentors:
: 340987 (view as bug list)
Depends on: 458959 459293 460333 498691
Blocks: 387522 es5 453865 459161
  Show dependency treegraph
 
Reported: 2007-12-18 08:44 PST by Robert Sayre
Modified: 2011-01-09 13:18 PST (History)
47 users (show)
shaver: wanted‑next+
dsicore: wanted1.9.1+
mtschrep: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
JSON stuff in a global "mozilla" object (WIP) (19.93 KB, patch)
2008-02-09 16:43 PST, Robert Sayre
no flags Details | Diff | Review
more complete WIP (25.09 KB, patch)
2008-02-18 21:11 PST, Robert Sayre
no flags Details | Diff | Review
JSON object (74.11 KB, patch)
2008-09-30 20:44 PDT, Robert Sayre
shaver: review+
shaver: superreview+
Details | Diff | Review
bustage fix (75.74 KB, patch)
2008-10-01 10:46 PDT, Robert Sayre
shaver: review+
Details | Diff | Review
small fixes, tests (69.96 KB, patch)
2008-10-01 15:01 PDT, Robert Sayre
shaver: review+
Details | Diff | Review

Description Robert Sayre 2007-12-18 08:44:57 PST
Need to hook up nsIJSON to the DOM and get exception types correctly specified.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2008-01-21 17:55:35 PST
Lots of people want to see this land for web content in FF3. What roadblocks do we have? JSON parsing doesn't seem to be an ES4 spec anymore. Are there any other specs we'd need to worry about?
Comment 2 Mike Schroepfer 2008-01-26 09:58:32 PST
Exposing to content means we need to be real careful about edgecases for security.  I'd love to have this FWIW but not sure we have time left.   Sayre re-nom if I'm wrong.
Comment 3 Robert Sayre 2008-02-09 16:43:29 PST
Created attachment 302356 [details] [diff] [review]
JSON stuff in a global "mozilla" object (WIP)

Sample session from
http://www.squarefree.com/shell/shell.html

JavaScript Shell 1.4

js> window.mozilla == mozilla
true

js> foo = mozilla.JSON.decode('["foo", "bar", "baz"]')
foo,bar,baz

js> foo instanceof Array
true

js> bar = mozilla.JSON.encode([1,2,3,,5,6,7])
[1,2,3,null,5,6,7]

js> typeof bar
string

js> baz = mozilla.JSON.decode('{"p":"q"}')
[object Object]

js> baz.p
q

js> typeof baz.p
string

js> qux = mozilla.JSON.encode({"x":"y"})
{"x":"y"}

js> typeof qux
string

js> qux
{"x":"y"}

js> for (var i in mozilla) print(i)
JSON

js> for (var i in mozilla.JSON) print(i)
decode
encode
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2008-02-10 19:05:45 PST
Proposal: put the mozilla object (or the JSON object?  less a fan of this since there's then no hook for future extensions) on navigator instead of on window.  navigator has a history of being polluted with Mozilla-specific (buildDate et al.) not-intended-to-be-general (cf. window.DOMParser) properties, whereas window has fewer (Components only?).  It's less likely existing code is going to stumble over a navigator.mozilla binding than over a window.mozilla binding.  This is less accessible for JS components, but I don't think the usual method of getting access to any XPCOM component is overly burdensome for them.

I don't feel super-strongly one way or the other, but I thought I'd at least throw this idea out for consideration.
Comment 5 Brendan Eich [:brendan] 2008-02-10 19:19:55 PST
Just because people peed in the well doesn't mean we should do so. The navigator object is part of the DOM level 0, so we should not treat it as a free for all.

I'm in favor of starting to populate mozilla.* carefully, with things like mozilla.JSON.*, any workalikes for google.* (Google Gears stuff), etc.

/be
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2008-02-10 19:25:46 PST
(In reply to comment #5)
> The navigator object is part of the DOM level 0, so we should not treat it as a
> free for all.

And window is not?  I was suggesting adding navigator.mozilla.JSON, not navigator.JSON.
Comment 7 Robert Sayre 2008-02-10 19:31:53 PST
I think we should squat it now. Should have squatted it years ago.
Comment 8 Brendan Eich [:brendan] 2008-02-10 19:46:20 PST
Sayrer's "it" is window.mozilla, I think. And yeah, it's ours. Come and take, to quote the Spartans.

/be
Comment 9 Robert Sayre 2008-02-18 21:11:20 PST
Created attachment 304152 [details] [diff] [review]
more complete WIP
Comment 10 Eric Shepherd [:sheppy] 2008-03-27 12:33:19 PDT
Do we have any idea yet if this will be resolved for Fx3?  Planning documentation work going forward toward final release.
Comment 11 John Resig 2008-03-27 12:35:06 PDT
(In reply to comment #10)
> Do we have any idea yet if this will be resolved for Fx3?  Planning
> documentation work going forward toward final release.

This won't make Fx3 - definitely getting bumped to a later release.

Comment 12 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-27 12:35:17 PDT
Nope, it's for Fx.next.
Comment 13 Damon Sicore (:damons) 2008-06-04 16:39:58 PDT
marking wanted1.9.1? to get this in the triage queue.  If this needs to be blocking1.9.1?, please mark it as so.
Comment 14 Damon Sicore (:damons) 2008-06-10 16:25:17 PDT
wanted1.9.1+, P1.
Comment 15 Adam Barth 2008-07-14 11:48:42 PDT
I'm working on this feature for WebKit:

https://bugs.webkit.org/show_bug.cgi?id=20031

ES 3.1 specs that the JSON object should appear at window.JSON.  Is that where you're planning to put the API?  Also, how strict are you planning to make the JSON parsing?  Are you planning to match RFC 4627?
Comment 16 Robert Sayre 2008-07-14 12:15:44 PDT
We allow trailing commas, leading zeros, and tab characters.
Comment 17 Damon Sicore (:damons) 2008-07-22 11:12:15 PDT
Sayre, were you going to attach a patch here for jst's review?  Thought this was going to happen last week.  Everything OK?
Comment 18 Robert Sayre 2008-07-22 11:25:03 PDT
<http://wiki.ecmascript.org/doku.php?id=es3.1:es3.1_proposal_working_draft>

ES3.1 changed and added the way the optional parameters work. I need to change the patch to match it.
Comment 19 Mikeal Rogers [:mikeal] (mrogers@) 2008-08-22 15:58:09 PDT
I have a build question about this.

There are some people that are building/embedding spidermonkey (now looking at tracemonkey) and would really like to have this native JSON stuff in.

How would they go about getting it built in?
Comment 20 mdew 2008-09-10 05:32:11 PDT
Just for reference, IE8 will have Native JSON.

http://blogs.msdn.com/ie/archive/2008/09/09/what-s-new-for-jscript-for-ie8-beta-2.aspx
Comment 21 davey 2008-09-12 17:04:31 PDT
this is listed on https://wiki.mozilla.org/Firefox3.1/Features#Gecko_1.9.1 but with no status, is this going to be bumped again? or is the feeling it will make it in?
Comment 22 Ben Turner (not reading bugmail, use the needinfo flag!) 2008-09-17 11:03:04 PDT
This now blocks bug 453865.
Comment 23 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-09-30 05:24:15 PDT
Last day to get it into 3.1, so please let me know if I can help with review or otherwise!
Comment 24 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-09-30 09:01:11 PDT
(Report from the ECMA meeting indicates that there is still spec work to be done on at least the encoding part, so we might need to take some/all of this in the b2 window.)
Comment 25 Robert Sayre 2008-09-30 20:44:06 PDT
Created attachment 341211 [details] [diff] [review]
JSON object
Comment 26 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-09-30 21:36:32 PDT
Comment on attachment 341211 [details] [diff] [review]
JSON object

r=shaver with fixes from phone review. yay!
Comment 27 Reed Loden [:reed] (use needinfo?) 2008-10-01 01:24:18 PDT
Had to back this out due to a test failure.

/builds/slave/trunk_linux-7/build/tools/test-harness/xpcshell-simple/test_all.sh: line 111:  3881 Segmentation fault      (core dumped) NATIVE_TOPSRCDIR="$native_topsrcdir" TOPSRCDIR="$topsrcdir" $xpcshell -s $headfiles -f $t $tailfiles 2>$t.log 1>&2
NEXT ERROR TEST-UNEXPECTED-FAIL | ../../../../_tests/xpcshell-simple/json_test/unit/test_encode.js | test failed, see log
../../../../_tests/xpcshell-simple/json_test/unit/test_encode.js.log:
>>>>>>>
*** test pending
after first yield

<<<<<<<
Comment 28 Robert Sayre 2008-10-01 10:46:59 PDT
Created attachment 341305 [details] [diff] [review]
bustage fix

In the section of write_string that escapes control characters, I incorrectly called JS_NewString, when I wanted JS_NewStringCopy.
Comment 29 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-10-01 10:56:05 PDT
Comment on attachment 341305 [details] [diff] [review]
bustage fix

r=shaver; we can get some cosmetics when we update for the next spec pass.
Comment 30 Robert Sayre 2008-10-01 15:01:10 PDT
Created attachment 341351 [details] [diff] [review]
small fixes, tests

In JSON.parse, fix the format specifier for JS_ConvertArguments, init JSBool ok, and add mochitest for these API entry points.
Comment 31 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-10-06 08:20:36 PDT
Comment on attachment 341351 [details] [diff] [review]
small fixes, tests

>+    JSBool ok = JS_TRUE;  
>+    JSONParser *jp = js_BeginJSONParse(cx, vp);
>+    if (!jp)
>+        ok = JS_FALSE;

Better as

JSBool ok = jp != NULL;

IMO!
Comment 32 Sylvain Pasche 2008-10-28 16:31:36 PDT
Marking dev-doc-needed. I added an entry in "DOM changes" of https://developer.mozilla.org/En/Firefox_3.1_for_developers
Comment 33 Robert Sayre 2009-05-09 13:26:09 PDT
*** Bug 340987 has been marked as a duplicate of this bug. ***
Comment 34 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-06-27 18:03:43 PDT
This seems to cause some compat issues with iGoogle's "Google Latitude" widget.  See bug 498691.
Comment 35 Eli Grey (:sephr) 2009-08-07 20:50:47 PDT
An ending "," in an array is invalid JSON (though it's valid JavaScript) yet JSON.parse allows it.

JSON.parse("[1,2,3,]") should throw a SyntaxError.
Comment 36 :Ms2ger 2011-01-09 13:18:22 PST
(In reply to comment #35)
> An ending "," in an array is invalid JSON (though it's valid JavaScript) yet
> JSON.parse allows it.
> 
> JSON.parse("[1,2,3,]") should throw a SyntaxError.

For the record, this was fixed in bug 564621.

Note You need to log in before you can comment on or make changes to this bug.