dojo.toJson does not work when dom nodes are included

[guest]

If the object includes a dom node, dojo.toJson does not work properly. For example:

  {
     dom: someDomNode,
     test:"dojo"
  }

leads to

{dom:,"test":"dojo"}

Reason is the following code in json.js (line 60)

if(it.nodeType && it.cloneNode){ // isNode
  return ""; 
}

Perhaps this should be something like:

if(it.nodeType && it.cloneNode){ // isNode
  return "\"\""; // return "null"; 
}

It would be even better if the dom node would not show up in the resulting json string ....

[Adam Peller]

see #4186

[kriszyp]

Attached a patch for review

[Adam Peller]

dunno. I'm on the fence, maybe leaning towards outerHTML just so we don't have to worry about exceptions. What does Crockford do?

[kriszyp]

Crockford's library does not have any special processing for DOM nodes; the DOM nodes are processed as objects, and their properties are serialized like a normal JS object. We could also handle it that way, that would be the most compact solution. outerHTML is fine with me as well, my biggest objection to that is the increase in code side. X-browser outerHTML is probably about 4-5 lines of code.

[Adam Peller]

I'm +1 on the minimalist approach, especially if it's consistent with Crockford's library.

[kriszyp]

Removal of DOM check

[kriszyp]

This patch look allright?

[kriszyp]

DOM nodes have cycles, so this technique (no DOM check) causes maximum recursion exceptions. I am fine with that, but I don't know if that would be popular (probably result in another bug report).

[James Burke]

It seems that the maximum recursion error will be guaranteed to occur if there is a DOM node? If so, then it seems no better than the current behavior, and at least the current behavior would fail faster, maybe give a better error indication (easier to inspect the malformed JSON? I'm not sure, have not tried it).

So I vote for either returning something like null or empty string, or just throwing an exception if there is a DOM node, since node-to-JSON has not been worked out formally. It would be curious to note the behavior of other browser objects, like window, document, location, an XHR object behave. Perhaps match that behavior for what we do with DOM nodes?

If that does not help define the behavior, maybe just return "[DOMNode]"?

[kriszyp]

Malformed JSON does not fail faster, rather it fails later, because it fails when the recipient tries to parse it, versus failing during serialization which would certainly be greatly preferable to the current behavior. Creating malformed JSON is simply unacceptable as far as I am concerned, I would rather see a recurse exception.

However, I initially proposed throwing an exception when encountering a DOM node, this fails fastest and provides a more informative error (rather than waiting for recurse error, which is rather cryptic). The drawback to this is the extra 2-3 lines of code. Returning null (or empty string) would be roughly the same code, and I don't care either way. More of question of it is worth it...

I am sure window and document would behave the same way as a DOM node (it has cycles). I don't think the XHR and location objects have cycles and I think they would actually be serialized as an object with properties.

[Kris Zyp]

(In [14078]) Throw an exception when a DOM node is encountered in JSON serialization (fixes #6903)

[Kris Zyp]

(In [14079]) Throw an exception when a DOM node is encountered in JSON serialization (fixes #6903)

[bill]

marking tickets closed in the last three months w/blank milestone to milestone 1.2.