JSON.stringify(empty array from another scope) -> {}

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: Mardak, Assigned: gal)

Tracking

({regression})

Trunk
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [compartments], fixed-in-tracemonkey)

Attachments

(2 attachments, 1 obsolete attachment)

function startup() {
  AddonManager.getAddonsByIDs([], function(a) {
    _("uneval", uneval(a));
    _("constructor", a.constructor);
    _("constructor == Array", a.constructor == Array);
    _("constructor == Array + ''", a.constructor == Array + '');
    _("json", JSON.stringify(a));
  });
}


results in..

[]
function Array() { [native code] }
false
true
{}

For non-empty arrays, it'll end up stringifying as {"0": "val", ...}
This doesn't seem to affect code crossing iframes, so it might be only chrome code importing another script that returns an array.

I suppose one workaround is to manually copy the array into a local array and hope there aren't nested arrays.
blocking2.0: --- → ?
This might be hitting ctypes used from restartless addons as well assuming the same issue of arrays looking like objects:

ctypes.StructType("foo", [..])

"Error: second argument must be an array"
http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#3990
Perhaps this is related to how the bootstrap code is run? Is the loading code based on other code that might be affected by the compartments change?

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#4130

They're created with..

createInstance(Ci.nsIPrincipal)
Components.utils.Sandbox(principal)
mozIJSSubScriptLoader.loadSubScript
blocking2.0: ? → beta7+
Assignee: general → mrbkap
Assignee: mrbkap → gal
Posted patch patch (obsolete) — Splinter Review
Posted patch patchSplinter Review
Attachment #484574 - Attachment is obsolete: true
Attachment #484587 - Flags: review?(mrbkap)
Attachment #484587 - Flags: review?(vladimir)
Attachment #484587 - Flags: review?(dvander)
dvander: change to the method jit

vlad: typed arrays now accept everything with a length property
(vlad, we could also use GetLengthProperty unconditionally, and if there is no length property, its treated like an array of length 0, what do you think?)
Comment on attachment 484587 [details] [diff] [review]
patch

r=me on methodjit part
Attachment #484587 - Flags: review?(dvander) → review+
Attachment #484587 - Flags: review?(mrbkap) → review+
content/base/src/nsContentUtils.cpp:5857: error: 'struct js::ClassExtension' has no member named 'wrappedObject'
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#5856
Comment on attachment 484587 [details] [diff] [review]
patch

Not sure about the lack of length property meaning 0... do other APIs behave like that?
Attachment #484587 - Flags: review?(vladimir) → review+
vlad: other array functions behave like that, so if you are ok with it I would duplicate that behavior
http://hg.mozilla.org/tracemonkey/rev/438afef93fd5
Whiteboard: [compartments], fixed-in-tracemonkey
Pushed with the behavior Array.prototype.* have. Will follow-up with a fix if vlad wants something else. This blocks b7 so I want to get it landed.
Backed out due to suspected orange.
Whiteboard: [compartments], fixed-in-tracemonkey → [compartments]
Gal will fix test and land asap.
http://hg.mozilla.org/tracemonkey/rev/ab80a372cfe1
Whiteboard: [compartments] → [compartments], fixed-in-tracemonkey
Follow-up, removing a bogus assert.

http://hg.mozilla.org/tracemonkey/rev/6d88ce7787ad
Blocks: 607126
http://hg.mozilla.org/mozilla-central/rev/139dcd10518f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 618577
Duplicate of this bug: 604475
You need to log in before you can comment on or make changes to this bug.