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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Mon, Jan 14, 2019 at 4:34 PM Victor Toso <victortoso@xxxxxxxxxx> wrote:
>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]