458959 - this.JSON is enumerable

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> uneval(this)
({JSON:JSON})

js> for (i in this) print(i)
JSON

Seems like a bug to me.  Is there something special about the JSON object that it should be enumerable when everything else (Math, gc, toSource, etc) is not?

If this is a bug, I'd appreciate a quick fix, because this change frustrates the reproducibility of jsfunfuzz bugs like bug 458679 that involve "uneval(this)" or "for (i in this)".

Comment 1

[Robert Sayre]

It looks to me like the flag should be JSPROP_READONLY | JSPROP_PERMANENT.

The only snag with this is that it will cause the export of JSON.jsm to fail, because it tries to export a "JSON" symbol and its JS_SetProperty call won't work.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

What do you mean by "fail" and "won't work?" Setting a readonly property silently does nothing, it doesn't throw.

Comment 3

[Robert Sayre]

Attachment: Switch to JSON native where possible, rename JSON module symbol, fix JSPROPs

Comment 4

[Robert Sayre]

(In reply to comment #2)
> What do you mean by "fail" and "won't work?" Setting a readonly property
> silently does nothing, it doesn't throw.

This turns out to be what was happening. It was GlobalForLocation failing to compile the script, because of

var JSON = { ...

Comment 5

[Robert Sayre]

Attachment: Switch to JSON native where possible, rename JSON module symbol, fix JSPROPs

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 342344 [details] [diff] [review]
Switch to JSON native where possible, rename JSON module symbol, fix JSPROPs

Looks good. Are there bugs filed on making the other JSON consumer switch to the native API?

Comment 7

[Simon Bünzli]

(In reply to comment #6)
> Are there bugs filed on making the other JSON consumer switch to the native API?

I'm waiting for review in bug 407110 (BTW: fixing bug 442059 would make that significantly simpler). Shall I completely remove JSON.jsm once we're done?

Comment 8

[Diego]

Does this causes the regression bug 459293 "Sites not loading - redeclaration const JSON error on console"?

Comment 9

[Bob Clary [:bc] (inactive)]

fixed by http://hg.mozilla.org/mozilla-central/rev/e18564ab198d 
not on tracemonkey

Comment 10

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_8_1/JSON/regress-458959.js,v  <--  regress-458959.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/b04c04268a94

Comment 11

[Bob Clary [:bc] (inactive)]

v 1.9.1