<div dir="auto"><div>Hi,</div><div dir="auto"><br><br><div class="gmail_quote" dir="auto"><div dir="ltr">On Wed, Jan 2, 2019, 5:31 PM Victor Toso <<a href="mailto:victortoso@redhat.com">victortoso@redhat.com</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Thanks for taking a look!<br>
<br>
On Sun, Dec 30, 2018 at 10:23:02PM +0100, Jakub Janku wrote:<br>
> Hi,<br>
> <br>
> On Wed, Dec 19, 2018 at 3:30 PM Victor Toso <<a href="mailto:victortoso@redhat.com" target="_blank" rel="noreferrer">victortoso@redhat.com</a>> wrote:<br>
> ><br>
> > From: Victor Toso <<a href="mailto:me@victortoso.com" target="_blank" rel="noreferrer">me@victortoso.com</a>><br>
> ><br>
> > If SpiceGtkSession is holding the keyboard, that's huge indication<br>
> > that we should not be requesting clipboard data yet. The proper time<br>
> > to request it is when another application in the client machine is<br>
> > asking for it, which means the user would switch to another<br>
> > application to paste the guest's clipboard data.<br>
> ><br>
<br>
> I'm having a rather hard time understanding this commit message.<br>
> I read it the following way:<br>
> "spice-gtk should not request clipboard data from vdagent, while<br>
> spice-gtk holds the keyboard focus"<br>
> However, what the patch actually does is that it makes sure that<br>
> spice-gtk doesn't send clipboard grab to vdagent, while spice-gtk<br>
> holds the keyboard focus.<br>
> <br>
> These 2 things seem rather unrelated. What am I missing?<br>
<br>
Client request for grab will make agent to look into clipboard<br>
data in the guest which would lead to VD_AGENT_CLIPBOARD_REQUEST<br>
being sent from agent to client.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Exactly! So it's the agent requesting the CLIENT's data. But in the commit message, you state "do not request GUEST's clipboard data unnecessarily". Or isn't it?</div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
I tried to avoid the grab first; I can also delay the clipboard<br>
data request to a later time (like when the keyboard is out of<br>
focus, as I suggested in the patch and explained below)<br>
<br>
> > This is default behavior over wayland.<br>
> <br>
> I can confirm that on Wayland, this patch breaks copy&paste<br>
> from client to guest.<br>
> According to the Wayland docs, the windows should receive the<br>
> owner-change event before the focus-in event (<br>
> <a href="https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-data-sharing" rel="noreferrer noreferrer" target="_blank">https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-data-sharing</a><br>
> have a look at Data devices.Selection: "This event is also generated<br>
> on a client immediately before it receives keyboard focus.").<br>
> But as it turns out, gtk+ delays the event so that the focus-in event<br>
> comes first ( <a href="https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gdk/wayland/gdkdevice-wayland.c" rel="noreferrer noreferrer" target="_blank">https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gdk/wayland/gdkdevice-wayland.c</a><br>
> check out the keyboard_handle_enter() ).<br>
> So this patch will unfortunately need some changes in order to<br>
> work on Wayland.<br>
<br>
Yep!<br>
<br>
> > Related: <a href="https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6" rel="noreferrer noreferrer" target="_blank">https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6</a><br>
> > Related: <a href="https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9" rel="noreferrer noreferrer" target="_blank">https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9</a><br>
> > Related: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1594876" rel="noreferrer noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=1594876</a><br>
> <br>
> Could you please elaborate a bit more on why this patch solves these issues?<br>
<br>
I can reproduce the client requesting guest's clipboard data on<br>
X11/KDE multiple times while the user is just copy-paste between<br>
applications in the guest. The idea of this patch was to avoid<br>
those requests till the keyboard focus is lost, which means that<br>
user is trying to paste guest's clipboard data into another<br>
application in the client's OS.<br>
<br>
Let me know if it isn't clear!<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Makes sense to me to try to avoid such requests. But why are those requests causing the issues described?</div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Cheers,<br>
> Jakub<br>
<br>
Thanks again,<br>
Victor<br>
<br>
<br>
> ><br>
> > Signed-off-by: Victor Toso <<a href="mailto:victortoso@redhat.com" target="_blank" rel="noreferrer">victortoso@redhat.com</a>><br>
> > Tested-by: James Harvey @jamespharvey20<br>
> > ---<br>
> >  src/spice-gtk-session.c | 4 +++-<br>
> >  1 file changed, 3 insertions(+), 1 deletion(-)<br>
> ><br>
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c<br>
> > index 1ccae07..0d3438c 100644<br>
> > --- a/src/spice-gtk-session.c<br>
> > +++ b/src/spice-gtk-session.c<br>
> > @@ -645,9 +645,11 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,<br>
> >          if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self))<br>
> >              break;<br>
> ><br>
> > +<br>
> >          s->clipboard_by_guest[selection] = FALSE;<br>
> >          s->clip_hasdata[selection] = TRUE;<br>
> > -        if (s->auto_clipboard_enable && !read_only(self))<br>
> > +        if (s->auto_clipboard_enable && !read_only(self) &&<br>
> > +            !spice_gtk_session_get_keyboard_has_focus(self))<br>
> >              gtk_clipboard_request_targets(clipboard, clipboard_get_targets,<br>
> >                                            get_weak_ref(self));<br>
> >          break;<br>
> > --<br>
> > 2.19.2<br>
> ><br>
> > _______________________________________________<br>
> > Spice-devel mailing list<br>
> > <a href="mailto:Spice-devel@lists.freedesktop.org" target="_blank" rel="noreferrer">Spice-devel@lists.freedesktop.org</a><br>
> > <a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a><br>
</blockquote></div></div></div>