Skip to content

Commit

Permalink
Full pass over all the JSON encoding and JS dialog code. We now abide
Browse files Browse the repository at this point in the history
by the following rules:

1) An initial dialog or panel load can take either HTML or JSON, but
   the mime type must accurately reflect its payload.

2) dialog form submits can handle a pure HTML response, but the mime
   type must also be correct.  This properly resolves the problem
   where the reauth code gets a JSON response first from the reauth
   code, and then an HTML response when you reauth and continue on to
   a given form -- try it out with Admin > Settings > Advanced.

3) All JSON replies must set the mime type correctly.  The json::reply
   convenience function does this for us.

4) By default, any HTML content sent back in the JSON response should be
   in the "html" field, no longer the "form" field.

The combination of these allows us to stop doing boilerplate code like
this in our controllers:

  // Print our view, JSON encoded
  json::reply(array("form" => (string) $view));

instead, controllers can just return HTML, eg:

  // Print our view
  print $view;

That's much more intuitive for developers.
  • Loading branch information
bharat committed Aug 1, 2010
1 parent be4ad8e commit 7607e1f
Show file tree
Hide file tree
Showing 25 changed files with 114 additions and 88 deletions.
37 changes: 31 additions & 6 deletions lib/gallery.dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
url: sHref,
type: "GET",
beforeSend: function(xhr) {
// Until we convert to jquery 1.4, we need to save the
// XMLHttpRequest object
// Until we convert to jquery 1.4, we need to save the XMLHttpRequest object so that we
// can detect the mime type of the reply
this.xhrData = xhr;
},
success: function(data, textStatus, xhr) {
Expand Down Expand Up @@ -122,17 +122,42 @@
_ajaxify_dialog: function() {
var self = this;
$("#g-dialog form").ajaxForm({
dataType: "json",
beforeSubmit: function(formData, form, options) {
form.find(":submit")
.addClass("ui-state-disabled")
.attr("disabled", "disabled");
return true;
},
beforeSend: function(xhr) {
// Until we convert to jquery 1.4, we need to save the XMLHttpRequest object so that we
// can detect the mime type of the reply
this.xhrData = xhr;
},
success: function(data) {
if (data.form) {
var formData = unescape(data.form);
$("#g-dialog").html(formData);
// Pre jquery 1.4, get the saved XMLHttpRequest object
xhr = this.xhrData;
if (xhr) {
var mimeType = /^(\w+\/\w+)\;?/.exec(xhr.getResponseHeader("Content-Type"));

var content = "";
if (mimeType[1] == "application/json") {
data = JSON.parse(data);
} else {
data = {"html": escape(data)};
}
} else {
// Uploading files (eg: watermark) uses a fake xhr in jquery.form.js so
// all we have is in the data field, which should be some very simple JSON.
// Weirdly enough in Chrome the result gets wrapped in a <pre> element and
// looks like this:
// <pre style="word-wrap: break-word; white-space: pre-wrap;">{"result":"success",
// "location":"\/~bharat\/gallery3\/index.php\/admin\/watermarks"}</pre>
// bizarre. Strip that off before parsing.
data = JSON.parse(data.match("({.*})")[0]);
}

if (data.html) {
$("#g-dialog").html(unescape(data.html));
$("#g-dialog").dialog("option", "position", "center");
$("#g-dialog form :submit").removeClass("ui-state-disabled")
.attr("disabled", null);
Expand Down
6 changes: 3 additions & 3 deletions lib/gallery.panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
var content = "";
if (mimeType[1] == "application/json") {
data = JSON.parse(data);
content = unescape(data.form);
content = unescape(data.html);
} else {
content = data;
}
Expand Down Expand Up @@ -79,8 +79,8 @@
return true;
},
success: function(data) {
if (data.form) {
$("#g-panel td").html(data.form);
if (data.html) {
$("#g-panel td").html(data.html);
self._ajaxify_panel();
}
if (data.result == "success") {
Expand Down
6 changes: 3 additions & 3 deletions modules/comment/controllers/comments.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ public function create($id) {
$view->comment = $comment;

json::reply(array("result" => "success",
"view" => (string) $view,
"form" => (string) comment::get_add_form($item)));
"view" => (string)$view,
"form" => (string)comment::get_add_form($item)));
} else {
$form = comment::prefill_add_form($form);
json::reply(array("result" => "error", "form" => (string) $form));
json::reply(array("result" => "error", "form" => (string)$form));
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/exif/controllers/exif.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ public function show($item_id) {
$view = new View("exif_dialog.html");
$view->details = exif::get($item);

json::reply(array("form" => (string) $view));
print $view;
}
}
2 changes: 1 addition & 1 deletion modules/gallery/controllers/admin_advanced_settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public function edit($module_name, $var_name) {
$group->input("var_name")->label(t("Setting"))->value($var_name)->disabled(1);
$group->textarea("value")->label(t("Value"))->value($value);
$group->submit("")->value(t("Save"));
json::reply(array("form" => (string) $form));
print $form;
}

public function save($module_name, $var_name) {
Expand Down
24 changes: 12 additions & 12 deletions modules/gallery/controllers/admin_maintenance.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function start($task_callback) {
log::info("tasks", t("Task %task_name started (task id %task_id)",
array("task_name" => $task->name, "task_id" => $task->id)),
html::anchor("admin/maintenance", t("maintenance")));
json::reply(array("form" => (string) $view));
print $view;
}

/**
Expand All @@ -86,7 +86,7 @@ public function resume($task_id) {
log::info("tasks", t("Task %task_name resumed (task id %task_id)",
array("task_name" => $task->name, "task_id" => $task->id)),
html::anchor("admin/maintenance", t("maintenance")));
json::reply(array("form" => (string) $view));
print $view;
}

/**
Expand All @@ -103,7 +103,7 @@ public function show_log($task_id) {
$view = new View("admin_maintenance_show_log.html");
$view->task = $task;

json::reply(array("form" => (string) $view));
print $view;
}

/**
Expand Down Expand Up @@ -212,18 +212,18 @@ public function run($task_id) {
}
// Using sprintf("%F") to avoid comma as decimal separator.
json::reply(array("result" => "success",
"task" => array(
"percent_complete" => sprintf("%F", $task->percent_complete),
"status" => (string) $task->status,
"done" => (bool) $task->done),
"location" => url::site("admin/maintenance")));
"task" => array(
"percent_complete" => sprintf("%F", $task->percent_complete),
"status" => (string) $task->status,
"done" => (bool) $task->done),
"location" => url::site("admin/maintenance")));

} else {
json::reply(array("result" => "in_progress",
"task" => array(
"percent_complete" => sprintf("%F", $task->percent_complete),
"status" => (string) $task->status,
"done" => (bool) $task->done)));
"task" => array(
"percent_complete" => sprintf("%F", $task->percent_complete),
"status" => (string) $task->status,
"done" => (bool) $task->done)));
}
}
}
2 changes: 1 addition & 1 deletion modules/gallery/controllers/admin_themes.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function preview($type, $theme_name) {
} else {
$view->url = item::root()->url("theme=$theme_name");
}
json::reply(array("form" => (string) $view));
print $view;
}

public function choose($type, $theme_name) {
Expand Down
8 changes: 4 additions & 4 deletions modules/gallery/controllers/albums.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public function create($parent_id) {

json::reply(array("result" => "success", "location" => $album->url()));
} else {
json::reply(array("result" => "error", "form" => (string) $form));
print $form;
}
}

Expand Down Expand Up @@ -159,7 +159,7 @@ public function update($album_id) {
json::reply(array("result" => "success"));
}
} else {
json::reply(array("result" => "error", "form" => (string) $form));
json::reply(array("result" => "error", "html" => (string)$form));
}
}

Expand All @@ -168,14 +168,14 @@ public function form_add($album_id) {
access::required("view", $album);
access::required("add", $album);

json::reply(array("form" => (string) album::get_add_form($album)));
print album::get_add_form($album);
}

public function form_edit($album_id) {
$album = ORM::factory("item", $album_id);
access::required("view", $album);
access::required("edit", $album);

json::reply(array("form" => (string) album::get_edit_form($album)));
print album::get_edit_form($album);
}
}
4 changes: 2 additions & 2 deletions modules/gallery/controllers/login.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Login_Controller extends Controller {
public function ajax() {
$view = new View("login_ajax.html");
$view->form = auth::get_login_form("login/auth_ajax");
json::reply(array("form" => (string) $view));
print $view;
}

public function auth_ajax() {
Expand All @@ -34,7 +34,7 @@ public function auth_ajax() {
} else {
$view = new View("login_ajax.html");
$view->form = $form;
json::reply(array("result" => "error", "form" => (string) $view));
json::reply(array("result" => "error", "html" => (string)$view));
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/gallery/controllers/move.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function browse($source_id) {
$view = new View("move_browse.html");
$view->source = $source;
$view->tree = $this->_get_tree_html($source, ORM::factory("item", 1));
json::reply(array("form" => (string) $view));
print $view;
}

public function save($source_id) {
Expand Down
4 changes: 2 additions & 2 deletions modules/gallery/controllers/movies.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public function update($movie_id) {
json::reply(array("result" => "success"));
}
} else {
json::reply(array("result" => "error", "form" => (string) $form));
json::reply(array("result" => "error", "html" => (string) $form));
}
}

Expand All @@ -102,6 +102,6 @@ public function form_edit($movie_id) {
access::required("view", $movie);
access::required("edit", $movie);

json::reply(array("form" => (string) movie::get_edit_form($movie)));
print movie::get_edit_form($movie);
}
}
2 changes: 1 addition & 1 deletion modules/gallery/controllers/permissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function browse($id) {
$view->parents = $item->parents();
$view->form = $this->_get_form($item);

json::reply(array("form" => (string) $view));
print $view;
}

function form($id) {
Expand Down
4 changes: 2 additions & 2 deletions modules/gallery/controllers/photos.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public function update($photo_id) {
json::reply(array("result" => "success"));
}
} else {
json::reply(array("result" => "error", "form" => (string) $form));
json::reply(array("result" => "error", "html" => (string)$form));
}
}

Expand All @@ -102,6 +102,6 @@ public function form_edit($photo_id) {
access::required("view", $photo);
access::required("edit", $photo);

json::reply(array("form" => (string) photo::get_edit_form($photo)));
print photo::get_edit_form($photo);
}
}
7 changes: 3 additions & 4 deletions modules/gallery/controllers/quick.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function form_delete($id) {
$v = new View("quick_delete_confirm.html");
$v->item = $item;
$v->form = item::get_delete_form($item);
json::reply(array("form" => (string) $v));
print $v;
}

public function delete($id) {
Expand Down Expand Up @@ -127,8 +127,7 @@ public function delete($id) {
$from_id != $id /* deleted the item we were viewing */) {
json::reply(array("result" => "success", "reload" => 1));
} else {
json::reply(array("result" => "success",
"location" => $parent->url()));
json::reply(array("result" => "success", "location" => $parent->url()));
}
}

Expand All @@ -154,6 +153,6 @@ public function form_edit($id) {
// Pass on the source item where this form was generated, so we have an idea where to return to.
$form->hidden("from_id")->value((int)Input::instance()->get("from_id", 0));

json::reply(array("form" => (string) $form));
print $form;
}
}
6 changes: 3 additions & 3 deletions modules/gallery/controllers/reauthenticate.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
*/
class Reauthenticate_Controller extends Controller {
public function index($share_translations_form=null) {
public function index() {
if (!identity::active_user()->admin) {
access::forbidden();
}
Expand All @@ -29,7 +29,7 @@ public function index($share_translations_form=null) {
$v = new View("reauthenticate.html");
$v->form = self::_form();
$v->user_name = identity::active_user()->name;
json::reply(array("form" => (string) $v));
print $v;
} else {
self::_show_form(self::_form());
}
Expand Down Expand Up @@ -58,7 +58,7 @@ public function auth() {
$v = new View("reauthenticate.html");
$v->form = $form;
$v->user_name = identity::active_user()->name;
json::reply(array("form" => (string) $v));
json::reply(array("html" => (string)$v));
} else {
self::_show_form($form);
}
Expand Down
3 changes: 1 addition & 2 deletions modules/gallery/controllers/uploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ public function index($id) {
$item = $item->parent();
}

json::reply(array("form" => (string)$this->_get_add_form($item)));
//print $this->_get_add_form($item);
print $this->_get_add_form($item);
}

public function start() {
Expand Down
4 changes: 2 additions & 2 deletions modules/gallery/controllers/user_profile.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function show($id) {

public function contact($id) {
$user = identity::lookup_user($id);
json::reply(array("form" => (string) user_profile::get_contact_form($user)));
print user_profile::get_contact_form($user);
}

public function send($id) {
Expand All @@ -63,7 +63,7 @@ public function send($id) {
message::success(t("Sent message to %user_name", array("user_name" => $user->display_name())));
json::reply(array("result" => "success"));
} else {
json::reply(array("result" => "error", "form" => (string)$form));
json::reply(array("result" => "error", "html" => (string)$form));
}
}
}
2 changes: 1 addition & 1 deletion modules/organize/controllers/organize.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function dialog($album_id) {
$v->controller_uri = url::site("organize") . "/";
$v->swf_uri = url::file("modules/organize/lib/Gallery3WebClient.swf?") .
filemtime(MODPATH . "organize/lib/Gallery3WebClient.swf");
json::reply(array("form" => (string) $v));
print $v;
}

function add_album_fields() {
Expand Down
6 changes: 3 additions & 3 deletions modules/server_add/controllers/server_add.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function browse($id) {
$view->tree = new View("server_add_tree.html");
$view->tree->files = $files;
$view->tree->parents = array();
json::reply(array("form" => (string) $view));
print $view;
}

public function children() {
Expand Down Expand Up @@ -112,8 +112,8 @@ function run($task_id) {
// Prevent the JavaScript code from breaking by forcing a period as
// decimal separator for all locales with sprintf("%F", $value).
json::reply(array("done" => (bool)$task->done,
"status" => (string)$task->status,
"percent_complete" => sprintf("%F", $task->percent_complete)));
"status" => (string)$task->status,
"percent_complete" => sprintf("%F", $task->percent_complete)));
}

/**
Expand Down
Loading

0 comments on commit 7607e1f

Please sign in to comment.