Hi, I see that I'm being late to the party as this has already been pushed upstream, but I'd like to comment anyway: On Thu, Jan 10, 2019 at 1:47 PM Victor Toso <victortoso@xxxxxxxxxx> wrote: > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > This patch improves the code style by adding braces, removing the > single case switch and add documentation to the handler of > GtkClipboard::owner-changed. > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > --- > src/spice-gtk-session.c | 54 +++++++++++++++++++++++++++++------------ > 1 file changed, 39 insertions(+), 15 deletions(-) > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > index 1ccae07..adc72a2 100644 > --- a/src/spice-gtk-session.c > +++ b/src/spice-gtk-session.c > @@ -618,6 +618,23 @@ static void clipboard_get_targets(GtkClipboard *clipboard, > s->nclip_targets[selection] = 0; > } > > +/* Callback for every owner-change event for given @clipboard. > + * This event is triggered in different ways depending on the environment of > + * the Client, some examples: > + * > + * Situation 1: When another application on the client machine is holding and > + * changing the clipboard. If client is on Wayland, spice-gtk only receives the > + * related GtkClipboard::owner-changed event after focus-in event on Spice > + * widget; On X11, we will receive it at the moment the clipboard data has been > + * changed in by other application. > + * > + * Situation 2: When spice-gtk holds the focus and is changing the clipboard by > + * either setting new content information with gtk_clipboard_set_with_owner() or > + * clearing up old content with gtk_clipboard_clear(). The main difference between > + * Wayland and X11 is that on X11, gtk_clipboard_clear() set the owner to none, which "set" -> "sets" ? > + * emits owner-change event; On Wayland that does not happen as spice-gtk still is > + * the owner of the clipboard. You must have come across this bug, since you commented on it: https://gitlab.gnome.org/GNOME/gtk/issues/715 I've created a simple test case which is attached. It seems like no "owner-change" event is emitted after gtk_clipboard_clear() is called, but after gtk_clipboard_set_with_owner() the event is emitted twice. So the Wayland behavior you described here might just be caused by the bug... > + */ > static void clipboard_owner_change(GtkClipboard *clipboard, > GdkEventOwnerChange *event, > gpointer user_data) > @@ -631,30 +648,37 @@ static void clipboard_owner_change(GtkClipboard *clipboard, > selection = get_selection_from_clipboard(s, clipboard); > g_return_if_fail(selection != -1); > > - if (s->main == NULL) > + if (s->main == NULL) { > return; > + } > > + /* In case we sent a grab to the agent, we need to release it now as > + * previous clipboard data should not be reachable anymore */ > if (s->clip_grabbed[selection]) { > s->clip_grabbed[selection] = FALSE; > - if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) > + if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) { > spice_main_channel_clipboard_selection_release(s->main, selection); > + } > } > > - switch (event->reason) { > - case GDK_OWNER_CHANGE_NEW_OWNER: > - if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self)) > - break; > - > - s->clipboard_by_guest[selection] = FALSE; > - s->clip_hasdata[selection] = TRUE; > - if (s->auto_clipboard_enable && !read_only(self)) > - gtk_clipboard_request_targets(clipboard, clipboard_get_targets, > - get_weak_ref(self)); > - break; > - default: > + /* We are mostly interested when owner has changed in which case > + * we would like to let agent know about new clipboard data. */ > + if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) { > s->clip_hasdata[selection] = FALSE; > - break; > + return; > } > + > + /* This situation happens when clipboard is being cleared by us, when agent > + * sends a release-grab for instance */ This is not true imho. It should only happen after a grab (after gtk_clipboard_set_with_owner() is called). After a release (gtk_clipboard_clear() call), gtk_clipboard_get_owner(clipboard) should already return NULL. > + if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self)) { > + return; > + } > + > + s->clipboard_by_guest[selection] = FALSE; > + s->clip_hasdata[selection] = TRUE; > + if (s->auto_clipboard_enable && !read_only(self)) > + gtk_clipboard_request_targets(clipboard, clipboard_get_targets, > + get_weak_ref(self)); > } > > typedef struct > -- > 2.20.1 > Cheers, Jakub
#include <gtk/gtk.h> GtkClipboard *clipboard; static void clipboard_owner_change_cb(GtkClipboard *clipboard, GdkEventOwnerChange *event, gpointer user_data) { g_message("owner_change_cb: owner=%p", gtk_clipboard_get_owner(clipboard)); } static void clipboard_get_cb(GtkClipboard *clipboard, GtkSelectionData *selection_data, guint info, gpointer user_data) { gtk_selection_data_set_text(selection_data, "abcd", -1); } static void clipboard_clear_cb(GtkClipboard *clipboard, gpointer user_data) { g_message("clear_cb"); GObject *owner = user_data; g_object_unref(owner); } static void btn_grab_click(GtkButton *button, gpointer user_data) { GObject *owner; GtkTargetList *list; GtkTargetEntry *table; gint n_targets; g_message("grabbing clipboard"); list = gtk_target_list_new(NULL, 0); gtk_target_list_add_text_targets(list, 0); table = gtk_target_table_new_from_list(list, &n_targets); owner = g_object_new(G_TYPE_OBJECT, NULL); gtk_clipboard_set_with_owner(clipboard, table, n_targets, clipboard_get_cb, clipboard_clear_cb, owner); gtk_target_table_free(table, n_targets); gtk_target_list_unref(list); } static void btn_release_click(GtkButton *button, gpointer user_data) { g_message("clearing clipboard"); gtk_clipboard_clear(clipboard); } static void create_gui() { GtkWidget *window, *box, *btn_grab, *btn_release; window = gtk_window_new(GTK_WINDOW_TOPLEVEL); box = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 10); btn_grab = gtk_button_new_with_label("Grab"); btn_release = gtk_button_new_with_label("Release"); g_signal_connect(btn_grab, "clicked", G_CALLBACK(btn_grab_click), NULL); g_signal_connect(btn_release, "clicked", G_CALLBACK(btn_release_click), NULL); gtk_box_pack_start((GtkBox *)box, btn_grab, TRUE, TRUE, 0); gtk_box_pack_start((GtkBox *)box, btn_release, TRUE, TRUE, 0); gtk_container_add(GTK_CONTAINER(window), box); gtk_widget_show_all(window); } int main(int argc, char *argv[]) { gtk_init(&argc, &argv); clipboard = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD); g_signal_connect(clipboard, "owner-change", G_CALLBACK(clipboard_owner_change_cb), NULL); create_gui(); gtk_main (); return 0; }
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel