Decide on move/copy action before items have been dropped

When files have been dropped as uri-list, the callback
function xfdesktop_file_icon_manager_drag_data_received()
decides whether they should be copied or moved depending on
their location, ignoring the selected drag action.

The comment preceding the code in charge reads:

/* If the user didn't pick whether to copy or move via
 * a GDK_ACTION_ASK then determine if we should move/copy
 * by checking if the files are on the same filesystem
 * and are writable by the user.
 */

I don't think this is the right place to make this decision,
because even if the user didn't pick the action via GDK_ACTION_ASK,
it could have been chosen using modifier keys and it has already
been indicated by the mouse cursor.

I've attached a patch where the move/copy action is proposed in the
drag-motion stage, e.g. before the items have been dropped, and can
be overridden by modifier keys.

Signed-off-by: Eric Koegel <eric.koegel@gmail.com>

4 files changed, 184 insertions, 80 deletions

diff --git a/src/xfdesktop-file-icon-manager.c b/src/xfdesktop-file-icon-manager.c
index d5e9ce35..a12c7027 100644
--- a/src/xfdesktop-file-icon-manager.c
+++ b/src/xfdesktop-file-icon-manager.c
@@ -172,6 +172,12 @@ static void xfdesktop_file_icon_manager_drag_data_get(XfdesktopIconViewManager *
                                                       GtkSelectionData *data,
                                                       guint info,
                                                       guint time_);
+static GdkDragAction xfdesktop_file_icon_manager_propose_drop_action(XfdesktopIconViewManager *manager,
+                                                                     XfdesktopIcon *drop_icon,
+                                                                     GdkDragAction action,
+                                                                     GdkDragContext *context,
+                                                                     GtkSelectionData *data,
+                                                                     guint info);
 
 static gboolean xfdesktop_file_icon_manager_check_create_desktop_folder(GFile *file);
 static void xfdesktop_file_icon_manager_load_desktop_folder(XfdesktopFileIconManager *fmanager);
@@ -419,6 +425,7 @@ xfdesktop_file_icon_manager_icon_view_manager_init(XfdesktopIconViewManagerIface
     iface->drag_drop = xfdesktop_file_icon_manager_drag_drop;
     iface->drag_data_received = xfdesktop_file_icon_manager_drag_data_received;
     iface->drag_data_get = xfdesktop_file_icon_manager_drag_data_get;
+    iface->propose_drop_action = xfdesktop_file_icon_manager_propose_drop_action;
 }
 
 
@@ -3007,7 +3014,6 @@ xfdesktop_file_icon_manager_drag_data_received(XfdesktopIconViewManager *manager
     gboolean copy_only = TRUE, drop_ok = FALSE;
     GList *file_list;
     GdkDragAction action;
-    gboolean user_selected_action = FALSE;
 
     TRACE("entering");
 
@@ -3020,8 +3026,6 @@ xfdesktop_file_icon_manager_drag_data_received(XfdesktopIconViewManager *manager
             gtk_drag_finish(context, FALSE, FALSE, time_);
             return;
         }
-        /* The user picked whether to move or copy the files */
-        user_selected_action = TRUE;
     }
 
     if(info == TARGET_XDND_DIRECT_SAVE0) {
@@ -3133,47 +3137,6 @@ xfdesktop_file_icon_manager_drag_data_received(XfdesktopIconViewManager *manager
                     base_dest_file = g_object_ref(fmanager->priv->folder);
                 }
 
-                /* If the user didn't pick whether to copy or move via
-                 * a GDK_ACTION_ASK then determine if we should move/copy
-                 * by checking if the files are on the same filesystem
-                 * and are writable by the user.
-                 */
-                if(user_selected_action == FALSE) {
-                    GFileInfo *src_info, *dest_info;
-                    const gchar *src_name, *dest_name;
-
-                    dest_info = g_file_query_info(base_dest_file,
-                                                  XFDESKTOP_FILE_INFO_NAMESPACE,
-                                                  G_FILE_QUERY_INFO_NONE,
-                                                  NULL,
-                                                  NULL);
-                    src_info = g_file_query_info(file_list->data,
-                                                 XFDESKTOP_FILE_INFO_NAMESPACE,
-                                                 G_FILE_QUERY_INFO_NONE,
-                                                 NULL,
-                                                 NULL);
-
-                    if(dest_info != NULL && src_info != NULL) {
-                        dest_name = g_file_info_get_attribute_string(dest_info,
-                                                G_FILE_ATTRIBUTE_ID_FILESYSTEM);
-                        src_name = g_file_info_get_attribute_string(src_info,
-                                                G_FILE_ATTRIBUTE_ID_FILESYSTEM);
-
-                        if((g_strcmp0(src_name, dest_name) == 0)
-                           && g_file_info_get_attribute_boolean(src_info,
-                                            G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE))
-                        {
-                            copy_only = FALSE;
-                            action = GDK_ACTION_MOVE;
-                        }
-                    }
-
-                    if(dest_info != NULL)
-                        g_object_unref(dest_info);
-                    if(src_info != NULL)
-                        g_object_unref(src_info);
-                }
-
                 for (l = file_list; l; l = l->next) {
                     gchar *dest_basename = g_file_get_basename(l->data);
 
@@ -3243,6 +3206,97 @@ xfdesktop_file_icon_manager_drag_data_get(XfdesktopIconViewManager *manager,
     xfdesktop_file_utils_file_list_free(file_list);
 }
 
+static GdkDragAction
+xfdesktop_file_icon_manager_propose_drop_action(XfdesktopIconViewManager *manager,
+                                                XfdesktopIcon *drop_icon,
+                                                GdkDragAction action,
+                                                GdkDragContext *context,
+                                                GtkSelectionData *data,
+                                                guint info)
+{
+    XfdesktopFileIconManager *fmanager = XFDESKTOP_FILE_ICON_MANAGER(manager);
+    XfdesktopFileIcon *file_icon = NULL;
+    GFileInfo *tinfo = NULL;
+    GFile *tfile = NULL;
+    GList *file_list;
+    GFileInfo *src_info, *dest_info;
+    const gchar *src_name, *dest_name;
+
+    if(info == TARGET_TEXT_URI_LIST && action == GDK_ACTION_COPY
+            && (context->actions & GDK_ACTION_MOVE) != 0) {
+
+        if(drop_icon) {
+            file_icon = XFDESKTOP_FILE_ICON(drop_icon);
+            tfile = xfdesktop_file_icon_peek_file(file_icon);
+            tinfo = xfdesktop_file_icon_peek_file_info(file_icon);
+        }
+
+        if(tfile && !g_file_has_uri_scheme(tfile, "file")) {
+            return action;
+        }
+
+        file_list = xfdesktop_file_utils_file_list_from_string((const gchar *)gtk_selection_data_get_data(data));
+        if(file_list) {
+            GFile *base_dest_file = NULL;
+            gboolean dest_is_volume = (drop_icon
+                                       && XFDESKTOP_IS_VOLUME_ICON(drop_icon));
+
+            /* if it's a volume, but we don't have |tinfo|, this just isn't
+             * going to work */
+            if(!tinfo && dest_is_volume) {
+                xfdesktop_file_utils_file_list_free(file_list);
+                return action;
+            }
+
+            if(tinfo && g_file_info_get_file_type(tinfo) == G_FILE_TYPE_DIRECTORY) {
+                base_dest_file = g_object_ref(tfile);
+            } else {
+                base_dest_file = g_object_ref(fmanager->priv->folder);
+            }
+
+            /* Determine if we should move/copy by checking if the files
+             * are on the same filesystem and are writable by the user.
+             */
+
+            dest_info = g_file_query_info(base_dest_file,
+                                          XFDESKTOP_FILE_INFO_NAMESPACE,
+                                          G_FILE_QUERY_INFO_NONE,
+                                          NULL,
+                                          NULL);
+            src_info = g_file_query_info(file_list->data,
+                                         XFDESKTOP_FILE_INFO_NAMESPACE,
+                                         G_FILE_QUERY_INFO_NONE,
+                                         NULL,
+                                         NULL);
+
+            if(dest_info != NULL && src_info != NULL) {
+                dest_name = g_file_info_get_attribute_string(dest_info,
+                                        G_FILE_ATTRIBUTE_ID_FILESYSTEM);
+                src_name = g_file_info_get_attribute_string(src_info,
+                                        G_FILE_ATTRIBUTE_ID_FILESYSTEM);
+
+                if((g_strcmp0(src_name, dest_name) == 0)
+                   && g_file_info_get_attribute_boolean(src_info,
+                                    G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE))
+                {
+                    action = GDK_ACTION_MOVE;
+                }
+            }
+
+            if(dest_info != NULL)
+                g_object_unref(dest_info);
+            if(src_info != NULL)
+                g_object_unref(src_info);
+
+            g_object_unref(base_dest_file);
+            xfdesktop_file_utils_file_list_free(file_list);
+
+        }
+    }
+
+    return action;
+}
+
 
 /* public api */
 
diff --git a/src/xfdesktop-icon-view-manager.c b/src/xfdesktop-icon-view-manager.c
index efab2ece..948c1820 100644
--- a/src/xfdesktop-icon-view-manager.c
+++ b/src/xfdesktop-icon-view-manager.c
@@ -138,3 +138,22 @@ xfdesktop_icon_view_manager_drag_data_get(XfdesktopIconViewManager *manager,
     
     iface->drag_data_get(manager, drag_icons, context, data, info, time_);
 }
+
+GdkDragAction
+xfdesktop_icon_view_manager_propose_drop_action(XfdesktopIconViewManager *manager,
+                                                XfdesktopIcon *drop_icon,
+                                                GdkDragAction action,
+                                                GdkDragContext *context,
+                                                GtkSelectionData *data,
+                                                guint info)
+{
+    XfdesktopIconViewManagerIface *iface;
+
+    g_return_val_if_fail(XFDESKTOP_IS_ICON_VIEW_MANAGER(manager), action);
+
+    iface = XFDESKTOP_ICON_VIEW_MANAGER_GET_IFACE(manager);
+    g_return_val_if_fail(iface->propose_drop_action, action);
+
+    return iface->propose_drop_action(manager, drop_icon, action, context, data,
+                                      info);
+}
diff --git a/src/xfdesktop-icon-view-manager.h b/src/xfdesktop-icon-view-manager.h
index 3170cd25..f0c45e3a 100644
--- a/src/xfdesktop-icon-view-manager.h
+++ b/src/xfdesktop-icon-view-manager.h
@@ -68,6 +68,12 @@ struct _XfdesktopIconViewManagerIface
                           GtkSelectionData *data,
                           guint info,
                           guint time_);
+    GdkDragAction (*propose_drop_action)(XfdesktopIconViewManager *manager,
+                                         XfdesktopIcon *drop_icon,
+                                         GdkDragAction action,
+                                         GdkDragContext *context,
+                                         GtkSelectionData *data,
+                                         guint info);
 };
 
 GType xfdesktop_icon_view_manager_get_type(void) G_GNUC_CONST;
@@ -98,6 +104,12 @@ void xfdesktop_icon_view_manager_drag_data_get(XfdesktopIconViewManager *manager
                                                GtkSelectionData *data,
                                                guint info,
                                                guint time_);
+GdkDragAction xfdesktop_icon_view_manager_propose_drop_action(XfdesktopIconViewManager *manager,
+                                                              XfdesktopIcon *drop_icon,
+                                                              GdkDragAction action,
+                                                              GdkDragContext *context,
+                                                              GtkSelectionData *data,
+                                                              guint info);
 
 
 
diff --git a/src/xfdesktop-icon-view.c b/src/xfdesktop-icon-view.c
index d7b3013d..5d5e20e2 100644
--- a/src/xfdesktop-icon-view.c
+++ b/src/xfdesktop-icon-view.c
@@ -154,6 +154,10 @@ struct _XfdesktopIconViewPrivate
     
     gboolean drag_dest_set;
     GdkDragAction foreign_dest_actions;
+
+    gboolean dropped;
+    GdkDragAction proposed_drop_action;
+    guint16 hover_row, hover_col;
     
     guchar    label_alpha;
     guchar    selected_label_alpha;
@@ -1403,10 +1407,9 @@ xfdesktop_icon_view_drag_motion(GtkWidget *widget,
         else  /* #3 */
             our_action = gdk_drag_context_get_suggested_action(context);
     } else {
-        /* start with everything */
-        GdkDragAction allowed_actions = (GDK_ACTION_MOVE | GDK_ACTION_COPY
-                                         | GDK_ACTION_LINK);
-        
+        /* start with all available actions */
+        GdkDragAction allowed_actions = context->actions;
+
         if(is_local_drag) {  /* #2 */
             /* check to make sure we aren't just hovering over ourself */
             GList *l;
@@ -1428,31 +1431,25 @@ xfdesktop_icon_view_drag_motion(GtkWidget *widget,
         /* #2 or #4 */
         allowed_actions &= xfdesktop_icon_get_allowed_drop_actions(icon_on_dest);
         
-        if(allowed_actions & gdk_drag_context_get_suggested_action(context))
-            our_action = gdk_drag_context_get_suggested_action(context);
-        else {
-            /* priority: move, copy, link */
-            if(allowed_actions & GDK_ACTION_MOVE)
-                our_action = GDK_ACTION_MOVE;
-            else if(allowed_actions & GDK_ACTION_COPY)
-                our_action = GDK_ACTION_COPY;
-            else if(allowed_actions & GDK_ACTION_LINK)
-                our_action = GDK_ACTION_LINK;
-        }
-    }
-    
-    if(!our_action) {
-        xfdesktop_icon_view_clear_drag_highlight(icon_view, context);
-        return FALSE;
+        /* priority: move, copy, link */
+        if(allowed_actions & GDK_ACTION_MOVE)
+            our_action = GDK_ACTION_MOVE;
+        else if(allowed_actions & GDK_ACTION_COPY)
+            our_action = GDK_ACTION_COPY;
+        else if(allowed_actions & GDK_ACTION_LINK)
+            our_action = GDK_ACTION_LINK;
     }
-    
-    /* at this point we can be reasonably sure that a drop is possible */
-    
-    gdk_drag_status(context, our_action, time_);
-    
-    xfdesktop_icon_view_draw_drag_highlight(icon_view, context,
-                                            hover_row, hover_col);
-        
+
+    /* allow the drag dest to override the selected action based on the drag data */
+    icon_view->priv->hover_row = hover_row;
+    icon_view->priv->hover_col = hover_col;
+    icon_view->priv->proposed_drop_action = our_action;
+    icon_view->priv->dropped = FALSE;
+    gtk_drag_get_data(widget, context, target, time_);
+
+    /* the actual call to gdk_drag_status() is deferred to
+     * xfdesktop_icon_view_drag_data_received() */
+
     return TRUE;
 }
 
@@ -1481,6 +1478,8 @@ xfdesktop_icon_view_drag_drop(GtkWidget *widget,
     XfdesktopIcon *icon_on_dest = NULL;
     
     TRACE("entering: (%d,%d)", x, y);
+
+    icon_view->priv->dropped = TRUE;
     
     target = gtk_drag_dest_find_target(widget, context, 
                                        icon_view->priv->native_targets);
@@ -1611,17 +1610,37 @@ xfdesktop_icon_view_drag_data_received(GtkWidget *widget,
     
     TRACE("entering");
     
-    xfdesktop_xy_to_rowcol(icon_view, x, y, &row, &col);
-    if(row >= icon_view->priv->nrows || col >= icon_view->priv->ncols)
-        return;
-    
     icon_on_dest = g_object_get_data(G_OBJECT(context),
                                      "--xfdesktop-icon-view-drop-icon");
     
-    xfdesktop_icon_view_manager_drag_data_received(icon_view->priv->manager,
-                                                   icon_on_dest,
-                                                   context, row, col, data,
-                                                   info, time_);
+    if(icon_view->priv->dropped) {
+        xfdesktop_xy_to_rowcol(icon_view, x, y, &row, &col);
+        if(row >= icon_view->priv->nrows || col >= icon_view->priv->ncols)
+            return;
+
+        xfdesktop_icon_view_manager_drag_data_received(icon_view->priv->manager,
+                                                       icon_on_dest,
+                                                       context, row, col, data,
+                                                       info, time_);
+    } else {
+        /* FIXME: cannot use x and y here, for they doen't seem to have any
+         * meaningful value */
+
+        GdkDragAction action = icon_view->priv->proposed_drop_action;
+        action = xfdesktop_icon_view_manager_propose_drop_action(icon_view->priv->manager,
+                                                                 icon_on_dest,
+                                                                 action,
+                                                                 context, data,
+                                                                 info);
+
+        if(action == 0)
+            xfdesktop_icon_view_clear_drag_highlight(icon_view, context);
+        else
+            xfdesktop_icon_view_draw_drag_highlight(icon_view, context,
+                                                    icon_view->priv->hover_row,
+                                                    icon_view->priv->hover_col);
+        gdk_drag_status(context, action, time_);
+    }
 }
 
 static gint