This patch contains the following changes I've made:

  1. Rewrote the JSON renderer. A preprocess step for making MIT Simile/Exhibit JSON has been added to streamline rendering. json_encode() will be used if it's available otherwise rendering will be done based on code from the drupal_to_js() Drupal core function. This allows for all data types including nested objects and arrays to be encoded properly. Preview rendering will be done using a more hacked version of the code from drupal_to_js() to allow for pretty-printing.
  2. Reorganized the style settings screen.
  3. Changed to the array form of str_replace() in views_json_encode_special_chars().
  4. Empty fields are now included. I don't know why they were originally excluded.
  5. Now using the "application/json" content type.
  6. Renamed views_json_get_json() to views_json_get(). It now returns an object by default and also has an optional parameter to return the raw JSON string.

I'm still a little confused about how to do a patch properly but hopefully this latest one is formatted better than my previous ones.

CommentFileSizeAuthor
#3 627384_views_datasource_json_improvements_v3.patch20.22 KBicylace
#1 627384_views_datasource_json_improvements_v2.patch20.25 KBicylace
views_datasource_json_improvements.patch19.58 KBicylace

Comments

icylace’s picture

icylace commented
StatusFileSize
new 627384_views_datasource_json_improvements_v2.patch20.25 KB

Updated the patch with the following:

  1. Renamed views_json_strip_illegal_chars() to views_json_check_label(). It's less of a misnomer and more informative of its intended use.
  2. Corrected function comments for views_json_check_label().
  3. Using views_json_check_label() on object labels as it should be.
allisterbeharry’s picture

allisterbeharry commented
Assigned: Unassigned » allisterbeharry
Status: Needs review » Postponed (maintainer needs more info)

Looks good but I'd prefer keeping the JSON Content-type to text/javascript so the output can be displayed in browsers. Any reason you changed it to application/json?

icylace’s picture

icylace commented
StatusFileSize
new 627384_views_datasource_json_improvements_v3.patch20.22 KB

It avoids a security vulnerability:
http://jibbering.com/blog/?p=514

And "application/json" is in the standard:
http://www.ietf.org/rfc/rfc4627.txt

"text/javascript" may have been necessary in the past with the older render code because of trailing commas that are illegal in JSON but since rendering has improved the proper content type should be used.

Thoughts ?

Also, here's a newer version of the patch that fixes a bug. It removes a call to a non-existent function in the MIT Simile/Exhibit JSON preprocess step.

allisterbeharry’s picture

allisterbeharry commented
Status: Postponed (maintainer needs more info) » Fixed

Ok cool, if that's the standard. Committed to DRUPAL-6--1 branch.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.