Merge into trunk : mediahandlers : Code : IVLE

Reviewer	Date Requested	Status
Matt Giuca	2010-07-27	Approve on 2010-07-28
David Coles		Approve on 2010-07-28
Review via email: mp+31040@code.launchpad.net

Revision history for this message

Matt Giuca (mgiuca) wrote on 2010-07-28:

#

It looks fantastic! But I see some problems.

* The type handlers. Firstly, looking at the type_handlers dictionary in ivle/webapp/filesystem/browser/media/browser.js, the dictionary only maps to "text" and "video" handlers. I'm a bit confused as to how audio and image handlers work (but I have seen them work, so ... please explain).

* Secondly, I'm not quite sure how the file extension maps onto the handler, but it seems to be a multi-stage process. First, the Python side maps the filename onto a MIME type, right? This would be something like "audio/ogg" or "video/ogg". Then our JavaScript maps the MIME type onto a handler type, like "video" or "audio". The problem is that if you have a ".ogg" file, Python will (I think) map it onto "audio/ogg", so our player will present an OGG Theora video with a ".ogg" extension as an audio file. I suppose there's nothing we can do about this except tell people to use ".ogv" instead. (So this is a non-issue, unless you can think of a better solution.)

* I'm seeing my test video with a length of 1 second, even though it continues playing for minutes. My test video is http://people.xiph.org/~maikmerten/demos/arctic_giant.html -- maybe there's something wrong with the video. Can you offer any suggestions as to why this is? (I see that you added a content-length output to the download link, so that should be working fine. Could be related to the following problem.)

All of the above are permissible. I am rejecting this merge due to the following:

* The major problem: I'm seeing in Firebug that the video is being downloaded in full *twice*. I know that Mozilla Firefox downloads the video multiple times to do buffering, and Mozilla has told us that this doesn't actually equate to double the bandwidth as they cache it. But this is not that -- the browser actually downloads the full file from two separate URLs.

1. GET http://ivle.localhost/fileservice/studenta/stuff/arctic_giant.ogv
Response content-type: text/plain
Response: {"listing": {".": {"isdir": false, "mtime_nice": "Tue Jul 27 2010, 11:58pm", "mtime_short": "Today", "mtime": 1280275123.6, "type": "video/ogg", "type_nice": "OGV file", "size": 23916003}}}

2. GET http://ivle.localhost/fileservice/studenta/stuff/arctic_giant.ogv?return=contents
Response Content-Type: video/ogg
Response body: Full 22MB video

At this point, the video panel appears on the screen in the correct size, with a 1 second total time, and shows a loading throbber.

3. GET http://ivle.localhost/download/studenta/stuff/arctic_giant.ogv
Response Content-Type: video/ogg
Response Content-Length: 23916003
Response body: Full 22MB video

Now the video starts playing.

So here's what's happening:

1. The JavaScript function "navigate" is called. This makes an AJAX call which is request #1 (GET /fileservice). This is a quick JSON response with the data for the file. Upon receiving the contents, it calls handle_response.
2. The "handle_response" function is called with the JSON dictionary for the file. At the bottom of this function, for all files (but not directories), it makes a second AJAX call which is request #2 (GET /fileservice?return=contents). "/* This time, get the contents of th...

It looks fantastic! But I see some problems.

* The type handlers. Firstly, looking at the type_handlers dictionary in ivle/webapp/filesystem/browser/media/browser.js, the dictionary only maps to "text" and "video" handlers. I'm a bit confused as to how audio and image handlers work (but I have seen them work, so ... please explain).

* Secondly, I'm not quite sure how the file extension maps onto the handler, but it seems to be a multi-stage process. First, the Python side maps the filename onto a MIME type, right? This would be something like "audio/ogg" or "video/ogg". Then our JavaScript maps the MIME type onto a handler type, like "video" or "audio". The problem is that if you have a ".ogg" file, Python will (I think) map it onto "audio/ogg", so our player will present an OGG Theora video with a ".ogg" extension as an audio file. I suppose there's nothing we can do about this except tell people to use ".ogv" instead. (So this is a non-issue, unless you can think of a better solution.)

* I'm seeing my test video with a length of 1 second, even though it continues playing for minutes. My test video is http://people.xiph.org/~maikmerten/demos/arctic_giant.html -- maybe there's something wrong with the video. Can you offer any suggestions as to why this is? (I see that you added a content-length output to the download link, so that should be working fine. Could be related to the following problem.)

All of the above are permissible. I am rejecting this merge due to the following:

* The major problem: I'm seeing in Firebug that the video is being downloaded in full *twice*. I know that Mozilla Firefox downloads the video multiple times to do buffering, and Mozilla has told us that this doesn't actually equate to double the bandwidth as they cache it. But this is not that -- the browser actually downloads the full file from two separate URLs.

1. GET http://ivle.localhost/fileservice/studenta/stuff/arctic_giant.ogv
Response content-type: text/plain
Response: {"listing": {".": {"isdir": false, "mtime_nice": "Tue Jul 27 2010, 11:58pm", "mtime_short": "Today", "mtime": 1280275123.6, "type": "video/ogg", "type_nice": "OGV file", "size": 23916003}}}

2. GET http://ivle.localhost/fileservice/studenta/stuff/arctic_giant.ogv?return=contents
Response Content-Type: video/ogg
Response body: Full 22MB video

At this point, the video panel appears on the screen in the correct size, with a 1 second total time, and shows a loading throbber.

3. GET http://ivle.localhost/download/studenta/stuff/arctic_giant.ogv
Response Content-Type: video/ogg
Response Content-Length: 23916003
Response body: Full 22MB video

Now the video starts playing.

So here's what's happening:

1. The JavaScript function "navigate" is called. This makes an AJAX call which is request #1 (GET /fileservice). This is a quick JSON response with the data for the file. Upon receiving the contents, it calls handle_response.
2. The "handle_response" function is called with the JSON dictionary for the file. At the bottom of this function, for all files (but not directories), it makes a second AJAX call which is request #2 (GET /fileservice?return=contents). "/* This time, get the contents of the file, not its metadata */". Upon downloading the file in its entirety, it calls handle_contents_response.
3. The "handle_contents_response" function makes use of the Content-Type of response #2. If the type is "text", then it also uses the body of response #2 to populate the editor (that is fine; the data is only transferred one time). If the type is "binary", it calls handle_binary which throws away the body of response #2, which is wasteful (and that's an old bug). However, in this branch, if the type is "image", "video" or "audio", it calls handle_video (for example), which also ignores the body of response #2 and creates a <video> tag.
4. The <video> tag causes the browser to make request #3 (GET /download) which downloads the full contents of the video again, this time with a Content-Length and for actual rendering in the video panel.

Now this was a problem before as well, because we would download the contents 1 time if it was binary when we should download 0. But it's much worse with video because we download the contents 2 times when we should download 1. So I think this should be fixed before this is merged.

The problem is that for non-text types, response #2 returns the full contents of the file, but then those contents are thrown away. The only part of response #2 which is used is its Content-Type, to determine how to handle the response. But we don't *need* to look at response #2's Content-Type, as we already know the type from the JSON dictionary of response #1. Thus, make the following changes (or come up with your own equivalent change set):

1. Make handle_contents_response take a content_type argument, and pass listing["."].type as its value. Thus handle_contents_response will not look at the Content-Type of response #2.
2. In handle_contents_response, make the handler for "text" actually make an AJAX call to ?return=contents to get the full contents. This introduces the double-download issue for the "text" type as well... for now. But importantly it makes text, image, video and audio all consistent: handle_contents_response no longer makes use of the supplied contents, it fetches its own.
3. Delete the "response" argument to handle_contents_response, as it is no longer used.
4. In handle_contents, delete the AJAX call to ?return=contents, and just directly call handle_contents_response.

This would eliminate request #2 for all non-text media types, so they just go on the JSON response to get the content-type, then use the browser <img>, <audio> and <video> tag to make request #3 (the download URL). The text media type still makes the AJAX request for request #2.

review: Needs Fixing

Revision history for this message

David Coles (dcoles) wrote on 2010-07-28:

#

Download full text (4.8 KiB)

> It looks fantastic! But I see some problems.
>
> * The type handlers. Firstly, looking at the type_handlers dictionary in
> ivle/webapp/filesystem/browser/media/browser.js, the dictionary only maps to
> "text" and "video" handlers. I'm a bit confused as to how audio and image
> handlers work (but I have seen them work, so ... please explain).

type_handers is just an override mechanism to allow us to map special MIME types (mainly application/x) to a specific hander. By default it uses the first part of the content type. From the code:

* If a file is not on the list, its default action is determined by the first
* part of its content type, where "text/*", "image/*" and "audio/*" are
* treated as above, and other types are simply treated as binary.

> * Secondly, I'm not quite sure how the file extension maps onto the handler,
> but it seems to be a multi-stage process. First, the Python side maps the
> filename onto a MIME type, right? This would be something like "audio/ogg" or
> "video/ogg". Then our JavaScript maps the MIME type onto a handler type, like
> "video" or "audio". The problem is that if you have a ".ogg" file, Python will
> (I think) map it onto "audio/ogg", so our player will present an OGG Theora
> video with a ".ogg" extension as an audio file. I suppose there's nothing we
> can do about this except tell people to use ".ogv" instead. (So this is a non-
> issue, unless you can think of a better solution.)

Yes. We just mimetypes.guess_type the MIME type from a file extension. Older versions of Python return "application/ogg", newer return "audio/ogg". It's now recommended that one should treat all ".ogg" files as audio. See http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions.

I'll fix the override for "application/ogg". This should be mapped to "audio".

> * I'm seeing my test video with a length of 1 second, even though it continues
> playing for minutes. My test video is
> http://people.xiph.org/~maikmerten/demos/arctic_giant.html -- maybe there's
> something wrong with the video. Can you offer any suggestions as to why this
> is? (I see that you added a content-length output to the download link, so
> that should be working fine. Could be related to the following problem.)

See https://bugs.edge.launchpad.net/ivle/+bug/490691/comments/3.

In short, unless we set the X-Content-Duration header or allow Partial Content requests the browser can't work out how long the file is until the end since duration is not in the ogg headers.

> All of the above are permissible. I am rejecting this merge due to the
> following:
>
> * The major problem: I'm seeing in Firebug that the video is being downloaded
> in full *twice*. I know that Mozilla Firefox downloads the video multiple
> times to do buffering, and Mozilla has told us that this doesn't actually
> equate to double the bandwidth as they cache it. But this is not that -- the
> browser actually downloads the full file from two separate URLs.

Right. This is bug #588337.

> Now this was a problem before as well, because we would download the contents
> 1 time if it was binary when we should download 0. But it's much worse with
> video because we download the c...

> It looks fantastic! But I see some problems.
> 
> * The type handlers. Firstly, looking at the type_handlers dictionary in
> ivle/webapp/filesystem/browser/media/browser.js, the dictionary only maps to
> "text" and "video" handlers. I'm a bit confused as to how audio and image
> handlers work (but I have seen them work, so ... please explain).

type_handers is just an override mechanism to allow us to map special MIME types (mainly application/x) to a specific hander. By default it uses the first part of the content type. From the code:

* If a file is not on the list, its default action is determined by the first
 * part of its content type, where "text/*", "image/*" and "audio/*" are
 * treated as above, and other types are simply treated as binary.

> * Secondly, I'm not quite sure how the file extension maps onto the handler,
> but it seems to be a multi-stage process. First, the Python side maps the
> filename onto a MIME type, right? This would be something like "audio/ogg" or
> "video/ogg". Then our JavaScript maps the MIME type onto a handler type, like
> "video" or "audio". The problem is that if you have a ".ogg" file, Python will
> (I think) map it onto "audio/ogg", so our player will present an OGG Theora
> video with a ".ogg" extension as an audio file. I suppose there's nothing we
> can do about this except tell people to use ".ogv" instead. (So this is a non-
> issue, unless you can think of a better solution.)

Yes. We just mimetypes.guess_type the MIME type from a file extension. Older versions of Python return "application/ogg", newer return "audio/ogg". It's now recommended that one should treat all ".ogg" files as audio. See http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions.

I'll fix the override for "application/ogg". This should be mapped to "audio".

> * I'm seeing my test video with a length of 1 second, even though it continues
> playing for minutes. My test video is
> http://people.xiph.org/~maikmerten/demos/arctic_giant.html -- maybe there's
> something wrong with the video. Can you offer any suggestions as to why this
> is? (I see that you added a content-length output to the download link, so
> that should be working fine. Could be related to the following problem.)

See https://bugs.edge.launchpad.net/ivle/+bug/490691/comments/3.

In short, unless we set the X-Content-Duration header or allow Partial Content requests the browser can't work out how long the file is until the end since duration is not in the ogg headers.

> All of the above are permissible. I am rejecting this merge due to the
> following:
> 
> * The major problem: I'm seeing in Firebug that the video is being downloaded
> in full *twice*. I know that Mozilla Firefox downloads the video multiple
> times to do buffering, and Mozilla has told us that this doesn't actually
> equate to double the bandwidth as they cache it. But this is not that -- the
> browser actually downloads the full file from two separate URLs.

Right. This is bug #588337.

> Now this was a problem before as well, because we would download the contents
> 1 time if it was binary when we should download 0. But it's much worse with
> video because we download the contents 2 times when we should download 1. So I
> think this should be fixed before this is merged.
> 
> The problem is that for non-text types, response #2 returns the full contents
> of the file, but then those contents are thrown away. The only part of
> response #2 which is used is its Content-Type, to determine how to handle the
> response. But we don't *need* to look at response #2's Content-Type, as we
> already know the type from the JSON dictionary of response #1. Thus, make the
> following changes (or come up with your own equivalent change set):
> 
> 1. Make handle_contents_response take a content_type argument, and pass
> listing["."].type as its value. Thus handle_contents_response will not look at
> the Content-Type of response #2.
> 2. In handle_contents_response, make the handler for "text" actually make an
> AJAX call to ?return=contents to get the full contents. This introduces the
> double-download issue for the "text" type as well... for now. But importantly
> it makes text, image, video and audio all consistent: handle_contents_response
> no longer makes use of the supplied contents, it fetches its own.
> 3. Delete the "response" argument to handle_contents_response, as it is no
> longer used.
> 4. In handle_contents, delete the AJAX call to ?return=contents, and just
> directly call handle_contents_response.
> 
> This would eliminate request #2 for all non-text media types, so they just go
> on the JSON response to get the content-type, then use the browser <img>,
> <audio> and <video> tag to make request #3 (the download URL). The text media
> type still makes the AJAX request for request #2.

I'll take a look at implementing that now.

Revision history for this message

Matt Giuca (mgiuca) wrote on 2010-07-28:

#

> type_handers is just an override mechanism to allow us to map special MIME
> types (mainly application/x) to a specific hander. By default it uses the
> first part of the content type.

Oh OK. That makes sense.

> Yes. We just mimetypes.guess_type the MIME type from a file extension. Older
> versions of Python return "application/ogg", newer return "audio/ogg". It's
> now recommended that one should treat all ".ogg" files as audio. See
> http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions.
>
> I'll fix the override for "application/ogg". This should be mapped to "audio".

OK. I'm happy with this behaviour (after that fix is applied).

> In short, unless we set the X-Content-Duration header or allow Partial Content
> requests the browser can't work out how long the file is until the end since
> duration is not in the ogg headers.

Oh yes. I remember this issue now. We should probably set X-Content-Duration. But that can be done after the merge (and after 1.0.2). Is there a bug on this? If not I'll file one now.

> > * The major problem: I'm seeing in Firebug that the video is being
> downloaded
> > in full *twice*. I know that Mozilla Firefox downloads the video multiple
> > times to do buffering, and Mozilla has told us that this doesn't actually
> > equate to double the bandwidth as they cache it. But this is not that -- the
> > browser actually downloads the full file from two separate URLs.
>
> Right. This is bug #588337.

OK. I didn't notice that bug before. I've posted my suggestion to that bug. It should probably be fixed in trunk, then merged to mediahandlers.

With those two fixes (application/ogg is audio, and the double-download bug), I'll accept the merge.

lp:~ivle-dev/ivle/mediahandlers updated on 2010-07-28

1797. By David Coles on 2010-07-28

Map 'application/ogg' to audio hander as recomended by XIPH. Note: Newer versions of Python's guess_type map '.ogg' to 'audio/ogg' anyway.

1798. By David Coles on 2010-07-28

Change handle_content_response so that we only fetch file contents inside-
fetch_text and not for all media types.

Also remove present_editorhead since it's only useful action is an unreachable
codepath and requires threading some otherwise unneeded varaibles.

1799. By David Coles on 2010-07-28

Add generic <object> media handler for suporting other media types (such as SVG)

1800. By David Coles on 2010-07-28

Use "?return=contents" URL instead of '/download/' since "Content-Disposition:
attachment" causes issues with certain embeded media (mainly SVG).

NB: The URLs generated do not support browsing revisions. See bug #610745.

Revision history for this message

David Coles (dcoles) wrote on 2010-07-28:

#

Ok. Fixed both issues. The double-download fix is pretty much done as you described.

Also added a generic object handler to you can view SVG files (does not work with handle_image) and use ?return=content URLs for media rather than download URLs (with extra Content-Disposition headers). Still doesn't fix Chomium's issue with not playing ogg media from IVLE (will have to open a bug if this is merged as is).

Revision history for this message

Matt Giuca (mgiuca) wrote on 2010-07-28:

#

I'd say open a bug on that last issue and merge as-is.

Just tried to merge; got 2 merge conflicts. Must have been with stuff Will has been doing today. Can you merge from trunk into mediahandlers, then I'll approve.

review: Needs Fixing

Revision history for this message

David Coles (dcoles) wrote on 2010-07-28:

#

On 28 July 2010 17:22, Matt Giuca <email address hidden> wrote:

> Review: Needs Fixing
> I'd say open a bug on that last issue and merge as-is.
>
> Just tried to merge; got 2 merge conflicts. Must have been with stuff Will
> has been doing today. Can you merge from trunk into mediahandlers, then I'll
> approve.
> --
> https://code.launchpad.net/~ivle-dev/ivle/mediahandlers/+merge/31040<https://code.launchpad.net/%7Eivle-dev/ivle/mediahandlers/+merge/31040>
> You proposed lp:~ivle-dev/ivle/mediahandlers for merging.
>

Right. I'll open the bug now - won't be able to merge until I get home (At
"work" waiting for people to head home).

lp:~ivle-dev/ivle/mediahandlers updated on 2010-07-28

1801. By David Coles on 2010-07-28: Merge from trunk

Revision history for this message

David Coles (dcoles) wrote on 2010-07-28:

#

Resolved conflicts with trunk. Mostly just due to cjson->json changes.

Will merge into trunk.

review: Approve

Revision history for this message

Matt Giuca (mgiuca) wrote on 2010-07-28:

#

I say you can merge too!

review: Approve

IVLE

Merge lp:~ivle-dev/ivle/mediahandlers into lp:ivle

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'ivle/dispatch/request.py'
 --- ivle/dispatch/request.py	2010-07-03 11:03:10 +0000
 +++ ivle/dispatch/request.py	2010-07-28 10:48:51 +0000
@@ -95,6 +95,8 @@
              in class Request.
          content_type (write)
              String. The Content-Type (mime type) header value.
++        content_length (write)
++            Integer. The number of octets to be transfered.
          location (write)
              String. Response "Location" header value. Used with HTTP redirect
              responses.
@@ -148,6 +150,7 @@
          # Default values for the output members
          self.status = Request.HTTP_OK
          self.content_type = None        # Use Apache's default
++        self.content_length = None        # Don't provide Content-Length
          self.location = None
          # In some cases we don't want the template JS (such as the username
          # and public FQDN) in the output HTML. In that case, set this to 0.
@@ -176,6 +179,8 @@
          # Prepare the HTTP and HTML headers before the first write is made
          if self.content_type != None:
              self.apache_req.content_type = self.content_type
++        if self.content_length:
++            self.apache_req.set_content_length(self.content_length)
          self.apache_req.status = self.status
          if self.location != None:
              self.apache_req.headers_out['Location'] = self.location
 === modified file 'ivle/webapp/filesystem/browser/media/browser.css'
 --- ivle/webapp/filesystem/browser/media/browser.css	2010-02-15 07:21:58 +0000
 +++ ivle/webapp/filesystem/browser/media/browser.css	2010-07-28 10:48:51 +0000
@@ -47,6 +47,7 @@
  #actions {
      padding: 0.3em 0.5em;
      border-top: white 1px solid;        /* Top edge of 3D effect */
++    line-height: 1.6em;
+ }
  /* class "choice" is for all choices, enabled and disabled. This is for both
   * the <a> actions and <option> actions.
 === modified file 'ivle/webapp/filesystem/browser/media/browser.js'
 --- ivle/webapp/filesystem/browser/media/browser.js	2010-07-20 05:42:59 +0000
 +++ ivle/webapp/filesystem/browser/media/browser.js	2010-07-28 10:48:51 +0000
@@ -31,6 +31,7 @@
   * "text" : When navigating to a text file, the text editor is opened.
   * "image" : When navigating to an image, the image is displayed (rather than
   *              going to the text editor).
++ * "video" : When navigation to a video file, a "play" button is presented.
   * "audio" : When navigating to an audio file, a "play" button is presented.
   * "binary" : When navigating to a binary file, offer it as a download through
   *              "serve".
@@ -43,7 +44,9 @@
      "application/x-javascript" : "text",
      "application/javascript" : "text",
      "application/json" : "text",
--    "application/xml" : "text"
++    "application/xml" : "text",
++    "application/ogg" : "audio",
++    "image/svg+xml": "object"
  };
  /* Mapping MIME types to icons, just the file's basename */
@@ -193,7 +196,8 @@
+ }
  /** Determines the "handler type" from a MIME type.
-- * The handler type is a string, either "text", "image", "audio" or "binary".
++ * The handler type is a string, either "text", "image", "video", "audio",
++ * "object" or "binary".
   */
  function get_handler_type(content_type)
+ {
@@ -205,7 +209,7 @@
      {   /* Based on the first part of the MIME type */
          var handler_type = content_type.split('/')[0];
          if (handler_type != "text" && handler_type != "image" &&
--            handler_type != "audio")
++            handler_type != "video" && handler_type != "audio")
              handler_type = "binary";
          return handler_type;
+     }
@@ -329,46 +333,37 @@
+     }
      else
+     {
--        /* Need to make a 2nd ajax call, this time get the actual file
--         * contents */
--        callback = function(response)
--            {
--                /* Read the response and set up the page accordingly */
--                handle_contents_response(path, response);
--            }
--        /* Call the server and request the listing. */
--        if (url_args)
--            args = shallow_clone_object(url_args);
--        else
--            args = {};
--        /* This time, get the contents of the file, not its metadata */
--        args['return'] = "contents";
--        ajax_call(callback, service_app, path, args, "GET");
++        /* Read the response and set up the page accordingly */
++        var content_type = current_file.type;
++        handle_contents_response(path, content_type, url_args);
++
+     }
      update_actions(isdir);
+ }
--function handle_contents_response(path, response)
++function handle_contents_response(path, content_type)
+ {
      /* Treat this as an ordinary file. Get the file type. */
--    var content_type = response.getResponseHeader("Content-Type");
++    //var content_type = response.getResponseHeader("Content-Type");
      var handler_type = get_handler_type(content_type);
--    would_be_handler_type = handler_type;
      /* handler_type should now be set to either
--     * "text", "image", "audio" or "binary". */
++     * "text", "image", "video", "audio" or "binary". */
      switch (handler_type)
+     {
      case "text":
--        handle_text(path, response.responseText,
--            would_be_handler_type);
++        handle_text(path, content_type);
          break;
      case "image":
--        /* TODO: Custom image handler */
--        handle_binary(path, response.responseText);
++        handle_image(path);
++        break;
++    case "video":
++        handle_video(path, content_type);
          break;
      case "audio":
--        /* TODO: Custom audio handler */
--        handle_binary(path, response.responseText);
++        handle_audio(path, content_type);
++        break;
++    case "object":
++        handle_object(path, content_type);
          break;
      case "binary":
          handle_binary(path);
@@ -540,6 +535,11 @@
   */
  function handle_binary(path)
+ {
++    // Disable save button
++    using_codepress = false;
++    disable_save_if_safe();
++
++    // Show download link
      var files = document.getElementById("filesbody");
      var div = document.createElement("div");
      files.appendChild(div);
@@ -554,6 +554,141 @@
      div.appendChild(par2);
+ }
++/** Displays an image file.
++ */
++function handle_image(path)
++{
++    /* Disable save button */
++    using_codepress = false;
++    disable_save_if_safe();
++
++    /* URL */
++    var url = app_url(service_app, path) + "?return=contents";
++
++    /* Image Preview */
++    var img = $("<img />");
++    img.attr("alt", path);
++    img.attr("src", url);
++
++    /* Show Preview */
++    var div = $('<div class="padding" />');
++    div.append('<h1>Image Preview</h1>');
++    div.append(img);
++    $("#filesbody").append(div);
++}
++
++/** Displays a video.
++ */
++function handle_video(path, type)
++{
++    /* Disable save button and hide the save panel */
++    using_codepress = false;
++    disable_save_if_safe();
++
++    /* URL */
++    var url = app_url(service_app, path) + "?return=contents";
++    var download_url = app_url(download_app, path);
++
++    /* Fallback Download Link */
++    var link = $('<p>Could not display video in browser.<p><p><a /></p>');
++    var a = link.find('a');
++    a.attr("href", download_url);
++    a.text("Download " + path);
++
++    /* Fallback Object Tag */
++    var obj = $('<object />');
++    obj.attr("type", type);
++    obj.attr("data", url);
++    obj.append(link);
++
++    /* HTML 5 Video Tag */
++    var video = $('<video controls="true" autoplay="true" />');
++    video.attr("src", url);
++    var support = video[0].canPlayType && video[0].canPlayType(type);
++    if (support != "probably" && support != "maybe") {
++        // Use Fallback
++        video = obj;
++    }
++
++    /* Show Preview */
++    var div = $('<div class="padding" />');
++    div.append('<h1>Video Preview</h1>');
++    div.append(video);
++    $("#filesbody").append(div);
++}
++
++/** Display audio content
++ */
++function handle_audio(path, type)
++{
++    /* Disable save button and hide the save panel */
++    using_codepress = false;
++    disable_save_if_safe();
++
++    /* URL */
++    var url = app_url(service_app, path) + "?return=contents";
++    var download_url = app_url(download_app, path);
++
++    /* Fallback Download Link */
++    var link = $('<p>Could not display audio in browser.<p><p><a /></p>');
++    var a = link.find('a');
++    a.attr("href", download_url);
++    a.text("Download " + path);
++
++    /* Fallback Object Tag */
++    var obj = $('<object />');
++    obj.attr("type", type);
++    obj.attr("data", url);
++    obj.append(link);
++
++    /* HTML 5 Audio Tag */
++    var audio = $('<audio controls="true" autoplay="true" />');
++    audio.attr("src", url);
++    var support = audio[0].canPlayType && audio[0].canPlayType(type);
++    if (support != "probably" && support != "maybe") {
++        // Use Fallback
++        audio = obj;
++    }
++
++    /* Show Preview */
++    var div = $('<div class="padding" />');
++    div.append('<h1>Audio Preview</h1>');
++    div.append(audio);
++    $("#filesbody").append(div);
++}
++
++/** Display generic object content
++ */
++function handle_object(path, content_type)
++{
++    /* Disable save button and hide the save panel */
++    using_codepress = false;
++    disable_save_if_safe();
++
++    /* URL */
++    var url = app_url(service_app, path) + "?return=contents";
++    var download_url = app_url(download_app, path);
++
++    /* Fallback Download Link */
++    var link = $('<p><a /></p>');
++    var a = link.find('a');
++    a.attr("href", download_url);
++    a.text("Download " + path);
++
++    /* Object Tag */
++    var obj = $('<object width="100%" height="500px" />');
++    obj.attr("type", content_type);
++    obj.attr("data", url);
++    obj.append('Could not load object');
++
++    /* Show Preview */
++    var div = $('<div class="padding" />');
++    div.append('<h1>Preview</h1>');
++    div.append(obj);
++    div.append(link);
++    $("#filesbody").append(div);
++}
++
  /* Enable or disable actions1 moreactions actions. Takes either a single
   * name, or an array of them.*/
  function set_action_state(names, which, allow_on_revision)
@@ -841,11 +976,6 @@
          var moreactions = document.getElementById("moreactions_area");
          moreactions.setAttribute("style", "display: inline;");
+     }
--    else
--    {
--        var actions2_file = document.getElementById("actions2_file");
--        actions2_file.setAttribute("style", "display: inline;");
--    }
      return;
+ }
 === modified file 'ivle/webapp/filesystem/browser/media/editor.js'
 --- ivle/webapp/filesystem/browser/media/editor.js	2010-07-28 04:12:08 +0000
 +++ ivle/webapp/filesystem/browser/media/editor.js	2010-07-28 10:48:51 +0000
@@ -58,25 +58,6 @@
      window.onbeforeunload = confirm_beforeunload;
+ }
--/** Presents the "editor heading" inserting it into a given element at
-- *  the front. Note that the save widget is handled by the Python.
-- */
--function present_editorhead(elem, path, handler_type)
--{
--    var div = document.getElementById("actions2");
--
--    /* Print a warning message if this is not actually a text file.
--     */
--    if (handler_type != "text")
--    {
--        var warn = dom_make_text_elem("p",
--            "Warning: You are editing a binary " +
--            "file, which explains any strange characters you may see. If " +
--            "you save this file, you could corrupt it.");
--        div.appendChild(warn);
--    }
--}
--
  function highlighting_changed(select)
+ {
      codemirror_language(select.value);
@@ -84,23 +65,40 @@
  /** Presents the text editor.
   */
--function handle_text(path, text, handler_type)
++function handle_text(path, content_type, url_args)
++{
++    /* Need to make a 2nd ajax call, this time get the actual file
++     * contents */
++    callback = function(response)
++        {
++            /* Read the response and set up the page accordingly */
++            handle_text_response(path, content_type, response.responseText);
++        }
++    /* Call the server and request the listing. */
++    if (url_args)
++        args = shallow_clone_object(url_args);
++    else
++        args = {};
++    /* This time, get the contents of the file, not its metadata */
++    args['return'] = "contents";
++    ajax_call(callback, service_app, path, args, "GET");
++}
++
++function handle_text_response(path, content_type, response_text)
+ {
      /* Create a textarea with the text in it
       * (The makings of a primitive editor).
       */
      var files = document.getElementById("filesbody");
--    /* Put our UI at the top */
--    present_editorhead(files, path, handler_type);
      var div = document.createElement("div");
      div.style.height = '100%';
      files.appendChild(div);
      var txt_elem = document.createElement("textarea");
--    txt_elem.value = text.toString();
++    txt_elem.value = response_text.toString();
      div.appendChild(txt_elem);
      txt_elem.setAttribute("id", "editbox");
--    language = language_from_mime(current_file.type);
++    language = language_from_mime(content_type);
      // Assume plaintext if no type can be determined.
      language = language ? language : "text";
@@ -176,6 +174,9 @@
      } else {
          codemirror.setParser("DummyParser")
+     }
++
++    // Show actions bar
++    $("#actions2_file").show();
+ }
  function language_from_mime(mime)
 === modified file 'ivle/webapp/filesystem/serve/__init__.py'
 --- ivle/webapp/filesystem/serve/__init__.py	2010-07-28 05:06:15 +0000
 +++ ivle/webapp/filesystem/serve/__init__.py	2010-07-28 10:48:51 +0000
@@ -139,6 +139,7 @@
                           "attachment; filename=%s" %
                               response['name'].encode('utf-8'))
          req.content_type = response['type'].encode('utf-8')
++        req.content_length = response.get('size')
          req.write(out)
  class DownloadView(ServeView):
 === modified file 'services/serveservice'
 --- services/serveservice	2010-07-20 04:52:31 +0000
 +++ services/serveservice	2010-07-28 10:48:51 +0000
@@ -37,7 +37,7 @@
  def determine_file_type(filename):
      filetype = mimetypes.guess_type(filename)[0]
      if filetype is None:
--         filetype = ivle.mimetypes.DEFAULT_MIMETYPE
++        filetype = ivle.mimetypes.DEFAULT_MIMETYPE
      return filetype
  def throw_error(message, extra={}):
@@ -142,14 +142,18 @@
      zipmod.make_zip(zipbasepath, paths, zipfile)
      print json.dumps({'type': zip_mimetype,
--                      'name': zipfilename.decode('utf-8')})
++                      'name': zipfilename.decode('utf-8'),
++                      'size': len(zipfile.getvalue()),
++                      })
      stream = zipfile
      stream.seek(0)
  else:
      print json.dumps({'type': determine_file_type(filename),
--                      'name': os.path.basename(filename).decode('utf-8')})
++                      'name': os.path.basename(filename).decode('utf-8'),
++                      'size': os.path.getsize(filename),
++                      })
      stream = open(filename)
  next = stream.read(1024)