pcirella / dynatree

Automatically exported from code.google.com/p/dynatree
0 stars 0 forks source link

Error in parsing JSON quoted parameters #156

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
According to JSON parsing methods http://api.jquery.com/jQuery.parseJSON/
everything should be between quotes.

Thus:

      "isFolder":"false"

      "isFolder":false

Give different results; the first always evaluates to true.
Probably because it's value is not being parsed as a Boolean type.

What version of the product are you using?
Current DynaTree 0.5.4

Original issue reported on code.google.com by prana001 on 6 Oct 2010 at 7:12

GoogleCodeExporter commented 9 years ago
I would rather interpret your link as 'all *keys* must be quoted'.
This is a change that came with the new built-in JSON parsers of the browsers 
that jQuery can use now.

Before, a simple evaluate() was used, and that would also accept 
    isFolder: false,
(isFolder without quotes, which is valid JavaScript, but invalid JSON)

The boolean type is perfectly valid JSON (http://json.org/).
I also think boolean is the natural type for 'isFolder'. Otherwise we would 
also have to test for 'no' '0', 0, 'off', etc.

Original comment by moo...@wwwendt.de on 7 Oct 2010 at 5:50

GoogleCodeExporter commented 9 years ago
But if valid JSON requires the usage of only quoted keys, then why not follow
that guideline?  E.g. I ran into this issue because I like my site to comply 
with web standards, as much as possible.

So, it's mosty likely that in the future someone else will fille this same 
"bug".

I think only testing for true, "true", false and "false" would suffice, just 
state
that clear in your documentation.  0 and 1 would do as well, just limit the 
total number of options.

Original comment by prana001 on 7 Oct 2010 at 6:01

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Valid JSON according to web standard is 
{
  "key1": "string_value",
  "key2": 42,
  "key3": true,
  "key4": null
}

So you are perfectly compliant if you pass 
  "isFolder": false,

Since testing is done like
    if( data.key ) ...
standard Javascript casting takes place, so these values are considered false:
    0, false, null
Others are true, like:
    1, 12, true, "any string", 

I still think, that boolean is the natural representation (and keeps the 
handling code straight forward).

(If this really turns out to be an issue for other users, maybe we could add a 
type check warning.)

Original comment by moo...@wwwendt.de on 7 Oct 2010 at 6:48

GoogleCodeExporter commented 9 years ago
Hi Martin,

I hope you don't get annoyed with me. :)

As a co-developer, I understand you perfectly, no misunderstanding there.

But (always the but..) What if DynaTree were to be called from some automated 
CMS mechanisme which, for JSON-compliance sake, put quotes around all JSON keys.

There would be some head-scratching there for the developers as to why and how 
etc.
Also, I you use the term, technology JSON, I personally think you should at 
least follow all the guideliness surrounding the technology.
If pure JSON requires this, then why not make comply?

Last but not least, the key testing can be done on initialisation, as I 
mentioned before: just limit the available options.
On init you parse/process the values and then set the value permanently as 
boolena.

if (data.key === true || data.key === "true") { data.key = true; }

This way you can still use the boolean testing style.

And.. if it turns out to be an issue for other users...
Well then in fact, you're too late :) since now you're aware of the "situation".

Keep up your excellent work!

Original comment by prana001 on 7 Oct 2010 at 6:56

GoogleCodeExporter commented 9 years ago
> What if DynaTree were to be called from some automated CMS mechanisme which, 
for JSON-compliance sake, put quotes around all JSON keys.

I still can't follow here: it's 
    "key": value
the key must be quoted (double quotes), the value depends on the type.
Treating a string with the value "false" === false would be an extension that I 
consider *not* compliant.

Anyway I see your point, that servers may deliver invalid formats and we could 
gracefully handle it.
So I re-open this issue and let other users 'star' for it.

Original comment by moo...@wwwendt.de on 7 Oct 2010 at 9:46

GoogleCodeExporter commented 9 years ago
There is now a documented workaround in the docs 'Loading custom formats':
"""
If we need more control, or if the server cannot provide JSON in Dynatree's 
native format, we could also use jQuery.ajax() to fetch the data, then 
transform it and call node.addChild(): ...
"""

Original comment by moo...@wwwendt.de on 23 Jan 2011 at 1:18