[spice-gtk,v4,2/2] gtk-session: clipboard: x11: owner-change improvement

Submitted by Victor Toso on Jan. 14, 2019, 12:33 p.m.

Details

Message ID 20190114123353.8039-2-victortoso@redhat.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso Jan. 14, 2019, 12:33 p.m.
From: Victor Toso <me@victortoso.com>

On X11, the release-grab message might end up clearing the
GtkClipboard which triggers the owner-changed callback having the
event owner as NULL.

We should not be calling gtk_clipboard_request_targets() in this
situation as is prone to errors as the intention here is request
clipboard information from changes made by client OS.

The fix is to avoid any such request while spice client is holding the
keyboard grab.

Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876

Changed in v4:
- Updated commit log

Signed-off-by: Victor Toso <victortoso@redhat.com>
Tested-by: James Harvey @jamespharvey20
---
 src/spice-gtk-session.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index abce43f..20df70d 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -674,6 +674,19 @@  static void clipboard_owner_change(GtkClipboard        *clipboard,
         return;
     }
 
+#ifdef GDK_WINDOWING_X11
+    /* In X11, while holding the keyboard-grab we are not interested in this
+     * event as it either came by grab or release messages from agent.  */
+    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
+        spice_gtk_session_get_keyboard_has_focus(self)) {
+        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
+                    "while holding focus");
+        return;
+    }
+#endif
+    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
+                spice_gtk_session_get_keyboard_has_focus(self));
+
     s->clipboard_by_guest[selection] = FALSE;
     s->clip_hasdata[selection] = TRUE;
     if (s->auto_clipboard_enable && !read_only(self))

Comments

Hi

On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@redhat.com> wrote:
>
> From: Victor Toso <me@victortoso.com>
>
> On X11, the release-grab message might end up clearing the
> GtkClipboard which triggers the owner-changed callback having the
> event owner as NULL.
>
> We should not be calling gtk_clipboard_request_targets() in this
> situation as is prone to errors as the intention here is request
> clipboard information from changes made by client OS.
>
> The fix is to avoid any such request while spice client is holding the
> keyboard grab.

Two things that bug me about this:
- it's x11 specific, weird
- the condition seems wrong: if an application has the keyboard grab,
that doesn't mean that another cannot update the clipboard. For
example, I suppose this can easily happen with multiple
pointers/inputs or some automation.

Other than that, if it fixes or workaround real clipboard issues, I am
not against it, but I think we should add more comments about the
"hack".


>
> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
>
> Changed in v4:
> - Updated commit log
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> Tested-by: James Harvey @jamespharvey20
> ---
>  src/spice-gtk-session.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index abce43f..20df70d 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
>          return;
>      }
>
> +#ifdef GDK_WINDOWING_X11
> +    /* In X11, while holding the keyboard-grab we are not interested in this
> +     * event as it either came by grab or release messages from agent.  */
> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> +                    "while holding focus");
> +        return;
> +    }
> +#endif
> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> +                spice_gtk_session_get_keyboard_has_focus(self));
> +
>      s->clipboard_by_guest[selection] = FALSE;
>      s->clip_hasdata[selection] = TRUE;
>      if (s->auto_clipboard_enable && !read_only(self))
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



--
Marc-André Lureau
Hi,

On Mon, Jan 14, 2019 at 04:40:35PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@redhat.com> wrote:
> >
> > From: Victor Toso <me@victortoso.com>
> >
> > On X11, the release-grab message might end up clearing the
> > GtkClipboard which triggers the owner-changed callback having the
> > event owner as NULL.
> >
> > We should not be calling gtk_clipboard_request_targets() in this
> > situation as is prone to errors as the intention here is request
> > clipboard information from changes made by client OS.
> >
> > The fix is to avoid any such request while spice client is holding the
> > keyboard grab.
> 
> Two things that bug me about this:
> - it's x11 specific, weird

You mean that GtkClipboard event's should behave the same way in
Wayland?

> - the condition seems wrong: if an application has the keyboard grab,
> that doesn't mean that another cannot update the clipboard. For
> example, I suppose this can easily happen with multiple
> pointers/inputs or some automation.

Yes but, do we wan't to support that? This is application-level
tweak on clipboard behavior so I don't think controlling it a bit
is bad if we agree that the rationale behind it is correct.

The other way to fix this is likely a major design in the code to
not touch the clipboard on grab/release (from guest messages)
while we have the focus. IMHO, also easy to get wrong.

> Other than that, if it fixes or workaround real clipboard
> issues, I am not against it, but I think we should add more
> comments about the "hack".

Calling it a hack is the same as saying you have a problem with
the rationale behind it, no?

I don't mind calling it a hack but I'm in favor of not having a
grab send by client to the guest by 3rd application running on
client OS *while* spice-gtk is holding the focus. Keeping the
behavior is bad, IMHO.

Not sure what you mean with multiple inputs. Are we targeting
something else other than Desktop clients?

> > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> >
> > Changed in v4:
> > - Updated commit log
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > Tested-by: James Harvey @jamespharvey20
> > ---
> >  src/spice-gtk-session.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index abce43f..20df70d 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >          return;
> >      }
> >
> > +#ifdef GDK_WINDOWING_X11
> > +    /* In X11, while holding the keyboard-grab we are not interested in this
> > +     * event as it either came by grab or release messages from agent.  */

Should I add:

    +   /* TODO: X11 clipboard specifics should be removed in the future */


Cheers,

> > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > +                    "while holding focus");
> > +        return;
> > +    }
> > +#endif
> > +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > +                spice_gtk_session_get_keyboard_has_focus(self));
> > +
> >      s->clipboard_by_guest[selection] = FALSE;
> >      s->clip_hasdata[selection] = TRUE;
> >      if (s->auto_clipboard_enable && !read_only(self))
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> 
> --
> Marc-André Lureau
Hi,

On Mon, Jan 14, 2019 at 1:41 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@redhat.com> wrote:
> >
> > From: Victor Toso <me@victortoso.com>
> >
> > On X11, the release-grab message might end up clearing the
> > GtkClipboard which triggers the owner-changed callback having the
> > event owner as NULL.
> >
> > We should not be calling gtk_clipboard_request_targets() in this
> > situation as is prone to errors as the intention here is request
> > clipboard information from changes made by client OS.
> >
> > The fix is to avoid any such request while spice client is holding the
> > keyboard grab.
>
> Two things that bug me about this:
> - it's x11 specific, weird

What's so weird about that?
It's also X11-specific that we can receive these "owner-change" events
while spice-gtk is holding the focus since on Wayland, apps need to
gain focus first to be able to set the clipboard, afaik.
The clipboard protocols on X11 and Wayland have simply been designed
in different ways. So although GTK tries to unify the behavior and
provide the same experience in both environments, some limitations
just cannot be overcome.

> - the condition seems wrong: if an application has the keyboard grab,
> that doesn't mean that another cannot update the clipboard. For
> example, I suppose this can easily happen with multiple
> pointers/inputs or some automation.

Sure, another app can update the clipboard, but imagine the following situation:
1) user copies something in the guest
2) an app in the client's system grabs the clipboard without having
keyboard focus -- this means that the grab likely wasn't directly
initiated by the user
3) spice-gtk receives "owner-change" event and subsequently sends grab
to the agent
4) based on the grab message from spice-gtk in step 3), vdagent grabs
the clipboard in the guest system -- and by that, it overwrites the
clipboard contents copied in step 1)

So the clipboard is suddenly changed, although the user didn't take any action.
I think this is unwanted and this patch solves it, so I'm all for
ack-ing this just for this reason. If this also solves the bug, it's
even better.

Nevertheless, I would be really happy if we could track down what
exactly causes the issues that have been reported and describe it in
more detail.
>
> Other than that, if it fixes or workaround real clipboard issues, I am
> not against it, but I think we should add more comments about the
> "hack".
>
>
> >
> > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> >
> > Changed in v4:
> > - Updated commit log
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > Tested-by: James Harvey @jamespharvey20

Does this mean it fixes the issues for him?
> > ---
> >  src/spice-gtk-session.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index abce43f..20df70d 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >          return;
> >      }
> >
> > +#ifdef GDK_WINDOWING_X11
> > +    /* In X11, while holding the keyboard-grab we are not interested in this
> > +     * event as it either came by grab or release messages from agent.  */

For completeness, it might be good to mention that the event can be
caused by other applications too (as Marc-André pointed out), even
though spice-gtk is holding the keyboard-grab.

> > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > +                    "while holding focus");
> > +        return;
> > +    }
> > +#endif
> > +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > +                spice_gtk_session_get_keyboard_has_focus(self));
> > +
> >      s->clipboard_by_guest[selection] = FALSE;
> >      s->clip_hasdata[selection] = TRUE;
> >      if (s->auto_clipboard_enable && !read_only(self))
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
Cheers,
Jakub
Hi

On Tue, Jan 15, 2019 at 12:40 AM Jakub Janku <jjanku@redhat.com> wrote:
>
> Hi,
>
> On Mon, Jan 14, 2019 at 1:41 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@redhat.com> wrote:
> > >
> > > From: Victor Toso <me@victortoso.com>
> > >
> > > On X11, the release-grab message might end up clearing the
> > > GtkClipboard which triggers the owner-changed callback having the
> > > event owner as NULL.
> > >
> > > We should not be calling gtk_clipboard_request_targets() in this
> > > situation as is prone to errors as the intention here is request
> > > clipboard information from changes made by client OS.
> > >
> > > The fix is to avoid any such request while spice client is holding the
> > > keyboard grab.
> >
> > Two things that bug me about this:
> > - it's x11 specific, weird
>
> What's so weird about that?
> It's also X11-specific that we can receive these "owner-change" events
> while spice-gtk is holding the focus since on Wayland, apps need to
> gain focus first to be able to set the clipboard, afaik.
> The clipboard protocols on X11 and Wayland have simply been designed
> in different ways. So although GTK tries to unify the behavior and
> provide the same experience in both environments, some limitations
> just cannot be overcome.

The patch is not about integration with X11, the way for example we
have overlay, key locks, or mouse acceleration code that gtk doesn't
provide. It's about an API that gtk+ provides that should be uniform.
Sometime it fails to provide the require abstraction. In this case, I
don't undestand *yet* why the code should be specific to X11, it could
be equally "valid" on wayland or windows etc...

>
> > - the condition seems wrong: if an application has the keyboard grab,
> > that doesn't mean that another cannot update the clipboard. For
> > example, I suppose this can easily happen with multiple
> > pointers/inputs or some automation.
>
> Sure, another app can update the clipboard, but imagine the following situation:
> 1) user copies something in the guest
> 2) an app in the client's system grabs the clipboard without having
> keyboard focus -- this means that the grab likely wasn't directly
> initiated by the user

I don't know, you could have some automation framework changing the
clipboard. For testing, for example, or to remove swear words, or
other kind of protected content..

And I think you could have multiple keyboard/pointers on the same
seat, although this may be esoteric, I agree.

> 3) spice-gtk receives "owner-change" event and subsequently sends grab
> to the agent
> 4) based on the grab message from spice-gtk in step 3), vdagent grabs
> the clipboard in the guest system -- and by that, it overwrites the
> clipboard contents copied in step 1)

That would be what I expect indeed.

>
> So the clipboard is suddenly changed, although the user didn't take any action.

And this may also be what the user wanted.

> I think this is unwanted and this patch solves it, so I'm all for
> ack-ing this just for this reason. If this also solves the bug, it's
> even better.
>
> Nevertheless, I would be really happy if we could track down what
> exactly causes the issues that have been reported and describe it in
> more detail.

Yes, that sounds like a better course to me.

> >
> > Other than that, if it fixes or workaround real clipboard issues, I am
> > not against it, but I think we should add more comments about the
> > "hack".
> >
> >
> > >
> > > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> > >
> > > Changed in v4:
> > > - Updated commit log
> > >
> > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > Tested-by: James Harvey @jamespharvey20
>
> Does this mean it fixes the issues for him?
> > > ---
> > >  src/spice-gtk-session.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > index abce43f..20df70d 100644
> > > --- a/src/spice-gtk-session.c
> > > +++ b/src/spice-gtk-session.c
> > > @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> > >          return;
> > >      }
> > >
> > > +#ifdef GDK_WINDOWING_X11
> > > +    /* In X11, while holding the keyboard-grab we are not interested in this
> > > +     * event as it either came by grab or release messages from agent.  */
>
> For completeness, it might be good to mention that the event can be
> caused by other applications too (as Marc-André pointed out), even
> though spice-gtk is holding the keyboard-grab.
>
> > > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > > +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > > +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > > +                    "while holding focus");
> > > +        return;
> > > +    }
> > > +#endif
> > > +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > > +                spice_gtk_session_get_keyboard_has_focus(self));
> > > +
> > >      s->clipboard_by_guest[selection] = FALSE;
> > >      s->clip_hasdata[selection] = TRUE;
> > >      if (s->auto_clipboard_enable && !read_only(self))
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> >
> Cheers,
> Jakub
On Mon, Jan 14, 2019 at 3:40 PM Jakub Janku <jjanku@redhat.com> wrote:
> > >
> > > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> > >
> > > Changed in v4:
> > > - Updated commit log
> > >
> > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > Tested-by: James Harvey @jamespharvey20
>
> Does this mean it fixes the issues for him?

Yes.  v1, 3, and 4 fix the issues for me.  Otherwise, using the
clipboard even just within the guest causes constant problems and 10
second lockups.
Hi,

On Tue, Jan 15, 2019 at 01:40:48AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jan 15, 2019 at 12:40 AM Jakub Janku <jjanku@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 14, 2019 at 1:41 PM Marc-André Lureau
> > <marcandre.lureau@gmail.com> wrote:
> > >
> > > Hi
> > >
> > > On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@redhat.com> wrote:
> > > >
> > > > From: Victor Toso <me@victortoso.com>
> > > >
> > > > On X11, the release-grab message might end up clearing the
> > > > GtkClipboard which triggers the owner-changed callback having the
> > > > event owner as NULL.
> > > >
> > > > We should not be calling gtk_clipboard_request_targets() in this
> > > > situation as is prone to errors as the intention here is request
> > > > clipboard information from changes made by client OS.
> > > >
> > > > The fix is to avoid any such request while spice client is holding the
> > > > keyboard grab.
> > >
> > > Two things that bug me about this:
> > > - it's x11 specific, weird
> >
> > What's so weird about that?
> > It's also X11-specific that we can receive these "owner-change" events
> > while spice-gtk is holding the focus since on Wayland, apps need to
> > gain focus first to be able to set the clipboard, afaik.
> > The clipboard protocols on X11 and Wayland have simply been designed
> > in different ways. So although GTK tries to unify the behavior and
> > provide the same experience in both environments, some limitations
> > just cannot be overcome.
> 
> The patch is not about integration with X11, the way for
> example we have overlay, key locks, or mouse acceleration code
> that gtk doesn't provide. It's about an API that gtk+ provides
> that should be uniform.  Sometime it fails to provide the
> require abstraction. In this case, I don't undestand *yet* why
> the code should be specific to X11, it could be equally "valid"
> on wayland or windows etc...

My first attempt to make it generic was a failure because, on
Wayland, when we receive the owner-change event, we already had
the focus and would miss the event due my buggy code. I have it
fixed now in a more generic way, I have yet to test on windows.

> > > - the condition seems wrong: if an application has the keyboard grab,
> > > that doesn't mean that another cannot update the clipboard. For
> > > example, I suppose this can easily happen with multiple
> > > pointers/inputs or some automation.
> >
> > Sure, another app can update the clipboard, but imagine the following situation:
> > 1) user copies something in the guest
> > 2) an app in the client's system grabs the clipboard without having
> > keyboard focus -- this means that the grab likely wasn't directly
> > initiated by the user
> 
> I don't know, you could have some automation framework changing the
> clipboard. For testing, for example, or to remove swear words, or
> other kind of protected content..

Would be okay if we propose a gtk-session property to
enable/disable clipboard managers? If that's a use case you see
important, having it as a command line option should be enough

    eg: remote-viewer --allow-clipboard-manager

That would keep the code behavior as is.

> And I think you could have multiple keyboard/pointers on the same
> seat, although this may be esoteric, I agree.
> 
> > 3) spice-gtk receives "owner-change" event and subsequently sends grab
> > to the agent
> > 4) based on the grab message from spice-gtk in step 3), vdagent grabs
> > the clipboard in the guest system -- and by that, it overwrites the
> > clipboard contents copied in step 1)
> 
> That would be what I expect indeed.
> 
> >
> > So the clipboard is suddenly changed, although the user didn't take any action.
> 
> And this may also be what the user wanted.

I think the implication of 'not being what user wanted' be quite
severe. e.g: The user having his password stored in clipboard
temporally would have that sent to all guests that it is
connected at that moment.

> > I think this is unwanted and this patch solves it, so I'm all for
> > ack-ing this just for this reason. If this also solves the bug, it's
> > even better.
> >
> > Nevertheless, I would be really happy if we could track down what
> > exactly causes the issues that have been reported and describe it in
> > more detail.
> 
> Yes, that sounds like a better course to me.

Sorry, really not sure how to write it better myself. I tried.

I'm not taking in consideration that this is misbehavior of gtk
but expected behavior that we are not handling well, that's all.

> > > Other than that, if it fixes or workaround real clipboard issues, I am
> > > not against it, but I think we should add more comments about the
> > > "hack".
> > >
> > >
> > > >
> > > > Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > > > Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> > > >
> > > > Changed in v4:
> > > > - Updated commit log
> > > >
> > > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > > Tested-by: James Harvey @jamespharvey20
> >
> > Does this mean it fixes the issues for him?

Yes, some replies and also private chat with feedback.

    https://lists.freedesktop.org/archives/spice-devel/2018-December/046556.html

> > > > ---
> > > >  src/spice-gtk-session.c | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > > index abce43f..20df70d 100644
> > > > --- a/src/spice-gtk-session.c
> > > > +++ b/src/spice-gtk-session.c
> > > > @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> > > >          return;
> > > >      }
> > > >
> > > > +#ifdef GDK_WINDOWING_X11
> > > > +    /* In X11, while holding the keyboard-grab we are not interested in this
> > > > +     * event as it either came by grab or release messages from agent.  */
> >
> > For completeness, it might be good to mention that the event can be
> > caused by other applications too (as Marc-André pointed out), even
> > though spice-gtk is holding the keyboard-grab.

Well, that can be extended but it is duplicated doc. Here I'm
trying to point out why we are about to ignore the owner-change
event, that is, "while holding the keyboard-grab...". I tried to
put the whole scenario above the function.

I'll change the code nevertheless, if next version is worse than
this one, I can resend this with better info.

> >
> > > > +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > > > +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > > > +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > > > +                    "while holding focus");
> > > > +        return;
> > > > +    }
> > > > +#endif
> > > > +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > > > +                spice_gtk_session_get_keyboard_has_focus(self));
> > > > +
> > > >      s->clipboard_by_guest[selection] = FALSE;
> > > >      s->clip_hasdata[selection] = TRUE;
> > > >      if (s->auto_clipboard_enable && !read_only(self))
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> > >
> > Cheers,
> > Jakub

Many thanks,
Victor
Hi,

I tried to fix this bug in a less radical way, but my patch unfortunately
did not cover all the cases.

I obtained some logs from James Harvey which make the situation clearer -
it can be found here:
https://termbin.com/40un
So I'll try to explain what's happening:

James uses KDE which has a clipboard manager integrated in (klipper).

(1) user copies something in the guest, grab is sent to the spice-gtk
(2) clipboard manager immediately requests the data, data is retrieved from
the vdagent
(3) user pastes the copied data in guest, this causes a quick re-grab in
the guest = a clipboard release message is sent to spice-gtk and it is
immediately followed by a new grab
(4) spice-gtk receives clipboard release, so it clears the clipboard
(5) clipboard manager detects that the clipboard has no owner, so it grabs
it itself, advertising the data it originally obtained from us in step (2)
(this normally enables the user to paste the data even after the source app
has been closed)
(6) spice-gtk receives "owner-change" signal caused by the grab in step
(5), requests clipboard targets and sends a grab to vdagent
(7) spice-gtk finally receives the grab from step (3), so it sets the
clipboard using gtk_clipboard_set_with_owner()
(8) vdagent receives grab from step (6), so it too sets the clipboard using
gtk_clipboard_set_with_owner()

This is clearly a race. We ended up in a state where both spice-gtk and
vdagent thinks the other component can provide the data, but in reality
none of them can.

(This does not happen always, as can be seen in the log, the first paste
succeeds.)

Given current spice protocol, I don't see a way to detect the race on
either side, but I'd love to be wrong. So I'd update the commit log and
comment in the code to explain the situation and then it's ack from me.

Cheers,
Jakub

On Mon, Jan 14, 2019, 1:34 PM Victor Toso <victortoso@redhat.com wrote:

> From: Victor Toso <me@victortoso.com>
>
> On X11, the release-grab message might end up clearing the
> GtkClipboard which triggers the owner-changed callback having the
> event owner as NULL.
>
> We should not be calling gtk_clipboard_request_targets() in this
> situation as is prone to errors as the intention here is request
> clipboard information from changes made by client OS.
>
> The fix is to avoid any such request while spice client is holding the
> keyboard grab.
>
> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
>
> Changed in v4:
> - Updated commit log
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> Tested-by: James Harvey @jamespharvey20
> ---
>  src/spice-gtk-session.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index abce43f..20df70d 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard
>   *clipboard,
>          return;
>      }
>
> +#ifdef GDK_WINDOWING_X11
> +    /* In X11, while holding the keyboard-grab we are not interested in
> this
> +     * event as it either came by grab or release messages from agent.  */
> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting
> client's target "
> +                    "while holding focus");
> +        return;
> +    }
> +#endif
> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> +                spice_gtk_session_get_keyboard_has_focus(self));
> +
>      s->clipboard_by_guest[selection] = FALSE;
>      s->clip_hasdata[selection] = TRUE;
>      if (s->auto_clipboard_enable && !read_only(self))
> --
> 2.20.1
>
>
ping?

On Mon, Jan 28, 2019 at 10:29 AM Jakub Janku <jjanku@redhat.com> wrote:
>
> Hi,
>
> I tried to fix this bug in a less radical way, but my patch unfortunately did not cover all the cases.
>
> I obtained some logs from James Harvey which make the situation clearer - it can be found here:
> https://termbin.com/40un
> So I'll try to explain what's happening:
>
> James uses KDE which has a clipboard manager integrated in (klipper).
>
> (1) user copies something in the guest, grab is sent to the spice-gtk
> (2) clipboard manager immediately requests the data, data is retrieved from the vdagent
> (3) user pastes the copied data in guest, this causes a quick re-grab in the guest = a clipboard release message is sent to spice-gtk and it is immediately followed by a new grab
> (4) spice-gtk receives clipboard release, so it clears the clipboard
> (5) clipboard manager detects that the clipboard has no owner, so it grabs it itself, advertising the data it originally obtained from us in step (2) (this normally enables the user to paste the data even after the source app has been closed)
> (6) spice-gtk receives "owner-change" signal caused by the grab in step (5), requests clipboard targets and sends a grab to vdagent
> (7) spice-gtk finally receives the grab from step (3), so it sets the clipboard using gtk_clipboard_set_with_owner()
> (8) vdagent receives grab from step (6), so it too sets the clipboard using gtk_clipboard_set_with_owner()
>
> This is clearly a race. We ended up in a state where both spice-gtk and vdagent thinks the other component can provide the data, but in reality none of them can.
>
> (This does not happen always, as can be seen in the log, the first paste succeeds.)
>
> Given current spice protocol, I don't see a way to detect the race on either side, but I'd love to be wrong. So I'd update the commit log and comment in the code to explain the situation and then it's ack from me.
>
> Cheers,
> Jakub
>
> On Mon, Jan 14, 2019, 1:34 PM Victor Toso <victortoso@redhat.com wrote:
>>
>> From: Victor Toso <me@victortoso.com>
>>
>> On X11, the release-grab message might end up clearing the
>> GtkClipboard which triggers the owner-changed callback having the
>> event owner as NULL.
>>
>> We should not be calling gtk_clipboard_request_targets() in this
>> situation as is prone to errors as the intention here is request
>> clipboard information from changes made by client OS.
>>
>> The fix is to avoid any such request while spice client is holding the
>> keyboard grab.
>>
>> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
>> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
>> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
>>
>> Changed in v4:
>> - Updated commit log
>>
>> Signed-off-by: Victor Toso <victortoso@redhat.com>
>> Tested-by: James Harvey @jamespharvey20
>> ---
>>  src/spice-gtk-session.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
>> index abce43f..20df70d 100644
>> --- a/src/spice-gtk-session.c
>> +++ b/src/spice-gtk-session.c
>> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
>>          return;
>>      }
>>
>> +#ifdef GDK_WINDOWING_X11
>> +    /* In X11, while holding the keyboard-grab we are not interested in this
>> +     * event as it either came by grab or release messages from agent.  */
>> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
>> +        spice_gtk_session_get_keyboard_has_focus(self)) {
>> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
>> +                    "while holding focus");
>> +        return;
>> +    }
>> +#endif
>> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
>> +                spice_gtk_session_get_keyboard_has_focus(self));
>> +
>>      s->clipboard_by_guest[selection] = FALSE;
>>      s->clip_hasdata[selection] = TRUE;
>>      if (s->auto_clipboard_enable && !read_only(self))
>> --
>> 2.20.1
>>
Hi,

On Mon, Feb 11, 2019 at 10:12:42AM +0100, Jakub Janku wrote:
> ping?

Should be fixed by:

commit 214f8fd969127c41a7d53def196118ee0549a411
Author: Jakub Janků <jjankuatredhat.com>
Date:   Sun Jan 27 18:14:20 2019 +0100

    clipboard: don't request targets without owner on X11

    On X11, if the owner in GdkEventOwnerChange is set to NULL,
    it means there's no data in the clipboard, so it's pointless to
    request targets as the request will fail anyway.

    On Wayland, owner is always NULL, so don't do anything there.

    Signed-off-by: Jakub Janků <jjanku at redhat.com>
    Acked-by: Victor Toso <victortoso@redhat.com>

The fact that the patch in this mail thread is related to keyboard-grab
was the reason to be nacked. If you want to discuss that, we might move
along to that thread on clibpoard-managers, sent as RFC

    https://lists.freedesktop.org/archives/spice-devel/2019-January/047259.html

> On Mon, Jan 28, 2019 at 10:29 AM Jakub Janku <jjanku@redhat.com> wrote:
> >
> > Hi,
> >
> > I tried to fix this bug in a less radical way, but my patch unfortunately did not cover all the cases.
> >
> > I obtained some logs from James Harvey which make the situation clearer - it can be found here:
> > https://termbin.com/40un
> > So I'll try to explain what's happening:
> >
> > James uses KDE which has a clipboard manager integrated in (klipper).
> >
> > (1) user copies something in the guest, grab is sent to the
> > spice-gtk
> > (2) clipboard manager immediately requests the data, data is
> > retrieved from the vdagent
> > (3) user pastes the copied data in guest, this causes a quick
> > re-grab in the guest = a clipboard release message is sent to
> > spice-gtk and it is immediately followed by a new grab
> > (4) spice-gtk receives clipboard release, so it clears the clipboard
> > (5) clipboard manager detects that the clipboard has no owner, so it
> > grabs it itself, advertising the data it originally obtained from us
> > in step (2) (this normally enables the user to paste the data even
> > after the source app has been closed)
> > (6) spice-gtk receives "owner-change" signal caused by the grab in
> > step (5), requests clipboard targets and sends a grab to vdagent
> > (7) spice-gtk finally receives the grab from step (3), so it sets
> > the clipboard using gtk_clipboard_set_with_owner()
> > (8) vdagent receives grab from step (6), so it too sets the
> > clipboard using gtk_clipboard_set_with_owner()
> >
> > This is clearly a race. We ended up in a state where both spice-gtk
> > and vdagent thinks the other component can provide the data, but in
> > reality none of them can.
> >
> > (This does not happen always, as can be seen in the log, the first
> > paste succeeds.)
> >
> > Given current spice protocol, I don't see a way to detect the
> > race on either side, but I'd love to be wrong. So I'd update
> > the commit log and comment in the code to explain the
> > situation and then it's ack from me.

I think we can still improve documentation to clarify the race and
behaviors. There are a few things that I thought were obvious and in
fact Marc-André pointed out I was wrong.

Either way, let me know if I'm missing something.

Cheers!

> > Cheers,
> > Jakub
> >
> > On Mon, Jan 14, 2019, 1:34 PM Victor Toso <victortoso@redhat.com wrote:
> >>
> >> From: Victor Toso <me@victortoso.com>
> >>
> >> On X11, the release-grab message might end up clearing the
> >> GtkClipboard which triggers the owner-changed callback having the
> >> event owner as NULL.
> >>
> >> We should not be calling gtk_clipboard_request_targets() in this
> >> situation as is prone to errors as the intention here is request
> >> clipboard information from changes made by client OS.
> >>
> >> The fix is to avoid any such request while spice client is holding the
> >> keyboard grab.
> >>
> >> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> >> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> >> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> >>
> >> Changed in v4:
> >> - Updated commit log
> >>
> >> Signed-off-by: Victor Toso <victortoso@redhat.com>
> >> Tested-by: James Harvey @jamespharvey20
> >> ---
> >>  src/spice-gtk-session.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> >> index abce43f..20df70d 100644
> >> --- a/src/spice-gtk-session.c
> >> +++ b/src/spice-gtk-session.c
> >> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> >>          return;
> >>      }
> >>
> >> +#ifdef GDK_WINDOWING_X11
> >> +    /* In X11, while holding the keyboard-grab we are not interested in this
> >> +     * event as it either came by grab or release messages from agent.  */
> >> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> >> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> >> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> >> +                    "while holding focus");
> >> +        return;
> >> +    }
> >> +#endif
> >> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> >> +                spice_gtk_session_get_keyboard_has_focus(self));
> >> +
> >>      s->clipboard_by_guest[selection] = FALSE;
> >>      s->clip_hasdata[selection] = TRUE;
> >>      if (s->auto_clipboard_enable && !read_only(self))
> >> --
> >> 2.20.1
> >>
On Mon, Feb 11, 2019 at 10:30 AM Victor Toso <victortoso@redhat.com> wrote:
>
> Hi,
>
> On Mon, Feb 11, 2019 at 10:12:42AM +0100, Jakub Janku wrote:
> > ping?
>
> Should be fixed by:

You can double-check with James to be sure, but I don't think that's true.

If you look at the logs ( https://termbin.com/40un ), there's the
following line:
    |   (virt-viewer:25767): GSpice-DEBUG: 21:29:06.743:
spice-gtk-session.c:556 Retrieving the clipboard data has failed
And the commit you mention below fixes exactly that and that's all,
the race is still there.

Cheers,
Jakub
>
> commit 214f8fd969127c41a7d53def196118ee0549a411
> Author: Jakub Janků <jjankuatredhat.com>
> Date:   Sun Jan 27 18:14:20 2019 +0100
>
>     clipboard: don't request targets without owner on X11
>
>     On X11, if the owner in GdkEventOwnerChange is set to NULL,
>     it means there's no data in the clipboard, so it's pointless to
>     request targets as the request will fail anyway.
>
>     On Wayland, owner is always NULL, so don't do anything there.
>
>     Signed-off-by: Jakub Janků <jjanku at redhat.com>
>     Acked-by: Victor Toso <victortoso@redhat.com>
>
> The fact that the patch in this mail thread is related to keyboard-grab
> was the reason to be nacked. If you want to discuss that, we might move
> along to that thread on clibpoard-managers, sent as RFC
>
>     https://lists.freedesktop.org/archives/spice-devel/2019-January/047259.html
>
> > On Mon, Jan 28, 2019 at 10:29 AM Jakub Janku <jjanku@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > I tried to fix this bug in a less radical way, but my patch unfortunately did not cover all the cases.
> > >
> > > I obtained some logs from James Harvey which make the situation clearer - it can be found here:
> > > https://termbin.com/40un
> > > So I'll try to explain what's happening:
> > >
> > > James uses KDE which has a clipboard manager integrated in (klipper).
> > >
> > > (1) user copies something in the guest, grab is sent to the
> > > spice-gtk
> > > (2) clipboard manager immediately requests the data, data is
> > > retrieved from the vdagent
> > > (3) user pastes the copied data in guest, this causes a quick
> > > re-grab in the guest = a clipboard release message is sent to
> > > spice-gtk and it is immediately followed by a new grab
> > > (4) spice-gtk receives clipboard release, so it clears the clipboard
> > > (5) clipboard manager detects that the clipboard has no owner, so it
> > > grabs it itself, advertising the data it originally obtained from us
> > > in step (2) (this normally enables the user to paste the data even
> > > after the source app has been closed)
> > > (6) spice-gtk receives "owner-change" signal caused by the grab in
> > > step (5), requests clipboard targets and sends a grab to vdagent
> > > (7) spice-gtk finally receives the grab from step (3), so it sets
> > > the clipboard using gtk_clipboard_set_with_owner()
> > > (8) vdagent receives grab from step (6), so it too sets the
> > > clipboard using gtk_clipboard_set_with_owner()
> > >
> > > This is clearly a race. We ended up in a state where both spice-gtk
> > > and vdagent thinks the other component can provide the data, but in
> > > reality none of them can.
> > >
> > > (This does not happen always, as can be seen in the log, the first
> > > paste succeeds.)
> > >
> > > Given current spice protocol, I don't see a way to detect the
> > > race on either side, but I'd love to be wrong. So I'd update
> > > the commit log and comment in the code to explain the
> > > situation and then it's ack from me.
>
> I think we can still improve documentation to clarify the race and
> behaviors. There are a few things that I thought were obvious and in
> fact Marc-André pointed out I was wrong.
>
> Either way, let me know if I'm missing something.
>
> Cheers!
>
> > > Cheers,
> > > Jakub
> > >
> > > On Mon, Jan 14, 2019, 1:34 PM Victor Toso <victortoso@redhat.com wrote:
> > >>
> > >> From: Victor Toso <me@victortoso.com>
> > >>
> > >> On X11, the release-grab message might end up clearing the
> > >> GtkClipboard which triggers the owner-changed callback having the
> > >> event owner as NULL.
> > >>
> > >> We should not be calling gtk_clipboard_request_targets() in this
> > >> situation as is prone to errors as the intention here is request
> > >> clipboard information from changes made by client OS.
> > >>
> > >> The fix is to avoid any such request while spice client is holding the
> > >> keyboard grab.
> > >>
> > >> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> > >> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> > >> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
> > >>
> > >> Changed in v4:
> > >> - Updated commit log
> > >>
> > >> Signed-off-by: Victor Toso <victortoso@redhat.com>
> > >> Tested-by: James Harvey @jamespharvey20
> > >> ---
> > >>  src/spice-gtk-session.c | 13 +++++++++++++
> > >>  1 file changed, 13 insertions(+)
> > >>
> > >> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > >> index abce43f..20df70d 100644
> > >> --- a/src/spice-gtk-session.c
> > >> +++ b/src/spice-gtk-session.c
> > >> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
> > >>          return;
> > >>      }
> > >>
> > >> +#ifdef GDK_WINDOWING_X11
> > >> +    /* In X11, while holding the keyboard-grab we are not interested in this
> > >> +     * event as it either came by grab or release messages from agent.  */
> > >> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > >> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > >> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > >> +                    "while holding focus");
> > >> +        return;
> > >> +    }
> > >> +#endif
> > >> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > >> +                spice_gtk_session_get_keyboard_has_focus(self));
> > >> +
> > >>      s->clipboard_by_guest[selection] = FALSE;
> > >>      s->clip_hasdata[selection] = TRUE;
> > >>      if (s->auto_clipboard_enable && !read_only(self))
> > >> --
> > >> 2.20.1
> > >>
On Mon, Feb 11, 2019 at 11:19:02AM +0100, Jakub Janku wrote:
> On Mon, Feb 11, 2019 at 10:30 AM Victor Toso <victortoso@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 11, 2019 at 10:12:42AM +0100, Jakub Janku wrote:
> > > ping?
> >
> > Should be fixed by:
> 
> You can double-check with James to be sure, but I don't think that's true.
> 
> If you look at the logs ( https://termbin.com/40un ), there's the
> following line:
>     |   (virt-viewer:25767): GSpice-DEBUG: 21:29:06.743:
> spice-gtk-session.c:556 Retrieving the clipboard data has failed
> And the commit you mention below fixes exactly that and that's all,
> the race is still there.

(...)

> > > >
> > > > (1) user copies something in the guest, grab is sent to the
> > > > spice-gtk
> > > > (2) clipboard manager immediately requests the data, data is
> > > > retrieved from the vdagent
> > > > (3) user pastes the copied data in guest, this causes a quick
> > > > re-grab in the guest = a clipboard release message is sent to
> > > > spice-gtk and it is immediately followed by a new grab
> > > > (4) spice-gtk receives clipboard release, so it clears the clipboard
> > > > (5) clipboard manager detects that the clipboard has no owner, so it
> > > > grabs it itself, advertising the data it originally obtained from us
> > > > in step (2) (this normally enables the user to paste the data even
> > > > after the source app has been closed)
> > > > (6) spice-gtk receives "owner-change" signal caused by the grab in
> > > > step (5), requests clipboard targets and sends a grab to vdagent
> > > > (7) spice-gtk finally receives the grab from step (3), so it sets
> > > > the clipboard using gtk_clipboard_set_with_owner()
> > > > (8) vdagent receives grab from step (6), so it too sets the
> > > > clipboard using gtk_clipboard_set_with_owner()
> > > >
> > > > This is clearly a race. We ended up in a state where both spice-gtk
> > > > and vdagent thinks the other component can provide the data, but in
> > > > reality none of them can.
> > > >
> > > > (This does not happen always, as can be seen in the log, the first
> > > > paste succeeds.)
> > > >
> > > > Given current spice protocol, I don't see a way to detect the
> > > > race on either side, but I'd love to be wrong. So I'd update
> > > > the commit log and comment in the code to explain the
> > > > situation and then it's ack from me.

The race is unavoidable with current protocol. What we need is
documentation on what behaviors we expect when such situations
happen... That's what I think but I may be wrong :)

(...)

> > > >> +#ifdef GDK_WINDOWING_X11
> > > >> +    /* In X11, while holding the keyboard-grab we are not interested in this
> > > >> +     * event as it either came by grab or release messages from agent.  */
> > > >> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > > >> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > > >> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > > >> +                    "while holding focus");
> > > >> +        return;
> > > >> +    }
> > > >> +#endif
> > > >> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > > >> +                spice_gtk_session_get_keyboard_has_focus(self));
> > > >> +

This patch was nacked by Marc-André because it discards the
situation where Clipboard Managers are getting clipboard data
(from the guest!). This is a behavior that Marc-André would like
to keep for some reason.

My suggestion is to add some configurable property to give users
some choice as there is none nowadays...

   https://lists.freedesktop.org/archives/spice-devel/2019-January/047259.html 


> > > >>      s->clipboard_by_guest[selection] = FALSE;
> > > >>      s->clip_hasdata[selection] = TRUE;
> > > >>      if (s->auto_clipboard_enable && !read_only(self))
> > > >> --
> > > >> 2.20.1
> > > >>