From 41721381548124e72c8a291a8f0d14b33788a8b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Tue, 16 Oct 2018 19:04:54 +0000 Subject: [PATCH] DnD: fix setting icon in drag-begin Commit 1c96b703 changed the way icon information is given to DnD. Previously an icon helper was kept at the drag source site. Now an image definition is stored there. The difference is that icon helper is an object that changes its state in response to an icon being set, thus the object survived multiple icon changes. Whereas image definition is destroyed and re-created from scratch every time a drag icon is changed. This created a problem where gtk_drag_begin_internal() would receive the value of site->image_def when a drag just began, then it emits "drag-begin" signal, in response to which an application can set drag icon, changing the value of site->image_def. However, gtk_drag_begin_internal() is unable to know about that change and continues to use the old value it received from up the stack. Not only does it prevent drag icon from being set from "drag-begin", it also can induce a crash, since the old image_def value used by gtk_drag_begin_internal() points to a freed memory region. Fix this by only setting a default icon (which is created in-place) in gtk_drag_begin_internal() if the caller does not care about icons. Otherwise gtk_drag_begin_internal() will return a boolean that indicates whether an icon needs to be set. Then the caller can invoke gtk_drag_set_icon_definition() to set the icon, if needed. Fixes #1407. --- gtk/gtkdnd.c | 40 +++++++++++++++++++--------------------- gtk/gtkdndprivate.h | 2 +- gtk/gtkdragsource.c | 11 ++++++++--- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/gtk/gtkdnd.c b/gtk/gtkdnd.c index dd66413f5d..96edea65f8 100644 --- a/gtk/gtkdnd.c +++ b/gtk/gtkdnd.c @@ -1699,18 +1699,20 @@ gtk_drag_is_managed (GtkWidget *source_widget) FALSE; } -/* Like gtk_drag_begin(), but also takes a GtkIconHelper - * so that we can set the icon from the source site information +/* Like gtk_drag_begin(), but also communicates the need to + * create an icon for the drag operation back to the caller. + * If the caller passes out_needs_icon == NULL, it means that + * the caller does not care. */ GdkDragContext * -gtk_drag_begin_internal (GtkWidget *widget, - GtkImageDefinition *icon, - GtkTargetList *target_list, - GdkDragAction actions, - gint button, - const GdkEvent *event, - int x, - int y) +gtk_drag_begin_internal (GtkWidget *widget, + gboolean *out_needs_icon, + GtkTargetList *target_list, + GdkDragAction actions, + gint button, + const GdkEvent *event, + int x, + int y) { GtkDragSourceInfo *info; GList *targets = NULL; @@ -1873,20 +1875,16 @@ gtk_drag_begin_internal (GtkWidget *widget, * application may have set one in ::drag_begin, or it may * not have set one. */ - if (!info->icon_widget) + if (!info->icon_widget && out_needs_icon == NULL) { - if (icon) - { - set_icon_helper (info->context, icon, 0, 0); - } - else - { - icon = gtk_image_definition_new_icon_name ("text-x-generic"); - set_icon_helper (info->context, icon, 0, 0); - gtk_image_definition_unref (icon); - } + GtkImageDefinition *icon = gtk_image_definition_new_icon_name ("text-x-generic"); + set_icon_helper (info->context, icon, 0, 0); + gtk_image_definition_unref (icon); } + if (out_needs_icon != NULL) + *out_needs_icon = (info->icon_widget == NULL); + if (managed) { g_signal_connect (context, "drop-performed", diff --git a/gtk/gtkdndprivate.h b/gtk/gtkdndprivate.h index f14a456ffb..89f1b0f4db 100644 --- a/gtk/gtkdndprivate.h +++ b/gtk/gtkdndprivate.h @@ -43,7 +43,7 @@ struct _GtkDragDestSite G_BEGIN_DECLS GdkDragContext * gtk_drag_begin_internal (GtkWidget *widget, - GtkImageDefinition *icon, + gboolean *out_needs_icon, GtkTargetList *target_list, GdkDragAction actions, gint button, diff --git a/gtk/gtkdragsource.c b/gtk/gtkdragsource.c index 677281aa88..bbe3d2ecad 100644 --- a/gtk/gtkdragsource.c +++ b/gtk/gtkdragsource.c @@ -88,6 +88,8 @@ gtk_drag_source_event_cb (GtkWidget *widget, GdkEventSequence *sequence; GdkEvent *last_event; guint button; + gboolean needs_icon; + GdkDragContext *context; sequence = gtk_gesture_single_get_current_sequence (GTK_GESTURE_SINGLE (site->drag_gesture)); last_event = gdk_event_copy (gtk_gesture_get_last_event (site->drag_gesture, sequence)); @@ -95,9 +97,12 @@ gtk_drag_source_event_cb (GtkWidget *widget, button = gtk_gesture_single_get_current_button (GTK_GESTURE_SINGLE (site->drag_gesture)); gtk_event_controller_reset (GTK_EVENT_CONTROLLER (site->drag_gesture)); - gtk_drag_begin_internal (widget, site->image_def, site->target_list, - site->actions, button, last_event, - start_x, start_y); + context = gtk_drag_begin_internal (widget, &needs_icon, site->target_list, + site->actions, button, last_event, + start_x, start_y); + + if (context != NULL && needs_icon) + gtk_drag_set_icon_definition (context, site->image_def, 0, 0); gdk_event_free (last_event);