Closed Bug 408838 Opened 17 years ago Closed 16 years ago

DOM binding for native JSON

Categories

(Core :: DOM: Core & HTML, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b1

People

(Reporter: sayrer, Assigned: sayrer)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 4 obsolete files)

Need to hook up nsIJSON to the DOM and get exception types correctly specified.
Blocks: 387522
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?
Flags: blocking1.9?
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.
Flags: blocking1.9? → blocking1.9-
OS: Mac OS X → All
Hardware: PC → All
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
Assignee: nobody → sayrer
Status: NEW → ASSIGNED
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.
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
(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.
I think we should squat it now. Should have squatted it years ago.
Sayrer's "it" is window.mozilla, I think. And yeah, it's ours. Come and take, to quote the Spartans.

/be
Attached patch more complete WIP (obsolete) — Splinter Review
Attachment #302356 - Attachment is obsolete: true
Do we have any idea yet if this will be resolved for Fx3?  Planning documentation work going forward toward final release.
(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.

Nope, it's for Fx.next.
Flags: wanted-next+
marking wanted1.9.1? to get this in the triage queue.  If this needs to be blocking1.9.1?, please mark it as so.
Flags: wanted1.9.1?
Priority: -- → P1
wanted1.9.1+, P1.
Flags: wanted1.9.1? → wanted1.9.1+
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?
We allow trailing commas, leading zeros, and tab characters.
Blocks: es5
Sayre, were you going to attach a patch here for jst's review?  Thought this was going to happen last week.  Everything OK?
<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.
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?
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?
Last day to get it into 3.1, so please let me know if I can help with review or otherwise!
(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.)
Attached patch JSON object (obsolete) — Splinter Review
Attachment #341211 - Flags: review?(shaver)
Comment on attachment 341211 [details] [diff] [review]
JSON object

r=shaver with fixes from phone review. yay!
Attachment #341211 - Flags: superreview+
Attachment #341211 - Flags: review?(shaver)
Attachment #341211 - Flags: review+
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

<<<<<<<
Attached patch bustage fix (obsolete) — Splinter Review
In the section of write_string that escapes control characters, I incorrectly called JS_NewString, when I wanted JS_NewStringCopy.
Attachment #304152 - Attachment is obsolete: true
Attachment #341211 - Attachment is obsolete: true
Comment on attachment 341305 [details] [diff] [review]
bustage fix

r=shaver; we can get some cosmetics when we update for the next spec pass.
Attachment #341305 - Flags: review+
In JSON.parse, fix the format specifier for JS_ConvertArguments, init JSBool ok, and add mochitest for these API entry points.
Attachment #341305 - Attachment is obsolete: true
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!
Attachment #341351 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
Depends on: 458959
Depends on: 459293
Depends on: 460333
Marking dev-doc-needed. I added an entry in "DOM changes" of https://developer.mozilla.org/En/Firefox_3.1_for_developers
Keywords: dev-doc-needed
Depends on: 498691
This seems to cause some compat issues with iGoogle's "Google Latitude" widget.  See bug 498691.
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.
(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.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.