Status

()

defect
--
minor
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: jruderman, Assigned: sayrer)

Tracking

({regression, testcase})

Trunk
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
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)".
(Assignee)

Comment 1

11 years ago
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.
What do you mean by "fail" and "won't work?" Setting a readonly property silently does nothing, it doesn't throw.
(Assignee)

Comment 3

11 years ago
Assignee: general → sayrer
(Assignee)

Comment 4

11 years ago
(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 = { ...
(Assignee)

Updated

11 years ago
Attachment #342332 - Flags: superreview?(mrbkap)
Attachment #342332 - Flags: review?(mrbkap)
(Assignee)

Comment 5

11 years ago
Attachment #342332 - Attachment is obsolete: true
Attachment #342344 - Flags: superreview?(mrbkap)
Attachment #342344 - Flags: review?(mrbkap)
Attachment #342332 - Flags: superreview?(mrbkap)
Attachment #342332 - Flags: review?(mrbkap)
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?
Attachment #342344 - Flags: superreview?(mrbkap)
Attachment #342344 - Flags: superreview+
Attachment #342344 - Flags: review?(mrbkap)
Attachment #342344 - Flags: review+

Comment 7

11 years ago
(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?
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Depends on: 459233

Comment 8

11 years ago
Does this causes the regression bug 459293 "Sites not loading - redeclaration const JSON error on console"?
Depends on: 459293

Comment 10

11 years ago
/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
Flags: in-testsuite+
Flags: in-litmus-

Comment 11

11 years ago
v 1.9.1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.