Not too difficult to add, any patches for this?

Created attachment 36886 [details] [review]
Patch that adds support for DnD of files

Ok to commit? :-)

+static void
+ev_window_dragged_file_cb (GtkWidget *widget, GdkDragContext *context,
+			   gint x, gint y, GtkSelectionData *selection_data,
+			   guint info, guint time, gpointer gdata)

Please call this drag_data_received_cb

+{
+	GList    *uris = NULL;
+	EvWindow *ev_window;
+	gchar    *mime_type;
+	gchar    *uri;
+
+	if (info == 0) {

What is the reason of this check?

+		uris = gnome_vfs_uri_list_parse ((gchar *) selection_data->data);
+
+		while (uris != NULL && uris->data != NULL) {

I dont think you ned to check uris->data here. You can just put a
g_return_if_fail inside the while, if you want to be safe.

+			uri = gnome_vfs_uri_to_string (uris->data,
+						       GNOME_VFS_URI_HIDE_NONE);
+			mime_type = gnome_vfs_get_mime_type (uri);
+
+			if (mime_type &&
+			    ((g_ascii_strcasecmp (mime_type, "application/pdf") == 0) ||
+			    (g_ascii_strcasecmp (mime_type, "application/postscript") == 0) ||
+			    (g_ascii_strcasecmp (mime_type, "image/x-eps") == 0))) {

I think we should not duplicate the mime checks here. Maybe the check should be
factored about to a gboolean is_file_supported func or similar and used in both
places.

+				if (ev_window_is_empty (EV_WINDOW (widget))) {
+					ev_window = EV_WINDOW (widget);
+				} else {
+					ev_window = ev_application_new_window (EV_APP);
+				}
+				
+				ev_window_open (ev_window,
+						gnome_vfs_uri_to_string (uris->data,
+									 GNOME_VFS_URI_HIDE_NONE));

We should probably have a ev_window_open_uri_list, static for now, and have this
code there.

+				gtk_drag_finish (context, TRUE, FALSE, time);
+				gtk_widget_show (GTK_WIDGET (ev_window));
+			} else {
+				gtk_drag_finish (context, FALSE, FALSE, time);
+			}

I dont think we need to call drag_finish here.

+			if (mime_type) g_free (mime_type);
+			if (uri) g_free (uri);

g_free is NULL safe no need for these checks.

+			uris = g_list_next (uris);
+		}
+			
+		gnome_vfs_uri_list_free (uris);

Arent you freeing a NULL here? You should use a GList *l to iterate. But please
see my previous comment about factoring out the code to open a list of uris in a
window.

+	}
+}

I hope this is clear, feel free to ask if it's not enough!

Ccing the patch submitter

Created attachment 37259 [details] [review]
An updated patch

Marco,	
sorry for the delay, I'm very busy these days because of the uni exams ...

Comment on attachment 37259 [details] [review]
An updated patch

Nice!

+	static gchar *supported_types [] = {

It would be good to make this a char *, Gtype struct and reuse it in
ev_window_open, I wont require it though ;)

+static GtkTargetEntry ev_drop_types[] = {
+	{ "text/uri-list", 0, 0}
+};

Please move this at the top of the file along with other global stuff.

+		gtk_drag_finish (context, TRUE, FALSE, time);

Is this really needed? If I'm not mistake this will be called by gtk on some
other events. data_received doesnt sounds like the right signal to finish the
drago to me.

> Is this really needed? If I'm not mistake this will be called by gtk on some
> other events. data_received doesnt sounds like the right signal to finish the
> drago to me.

I'm not sure at all, but the gtk documentation says that it's used to inform the
source that the drop is finished. I saw it in the example of the
drag-data-received signal
(http://developer.gnome.org/doc/API/2.0/gtk/GtkWidget.html#GtkWidget-drag-data-received)

Yeah reading the docs that looks correct. (Weird I've never seen it in any code
so far).
When you fixed the two other nitpicks please commit the patch. Thanks!

Looks like the fix was checked in. Thanks!