[PATCH weston] xwm: Don't clear the selection if it has no text type available

[Carlos Garnacho]

Hi!,

On Mon, Feb 1, 2016 at 9:36 PM, Derek Foreman <derekf at osg.samsung.com> wrote:
> weston maintains a copy of the most recently selected "thing" - it picks
> the first available type when it copies, and saves that one only.
>
> When an application quits weston will make the saved selection active.
>
> When xwm sees the selection set it will check if any of the offered types
> are text.  If no text type is offered it will clear the selection.
>
> weston then interprets this in the same way as an application exiting and
> causing the selection to be unset, and we get caught in a live lock with
> both weston and xwayland consuming as much cpu as they can.
>
> The simple fix is to just remove the test for text presence.
>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>
> This is kind of an alternate solution to the problem fixed in
> http://patchwork.freedesktop.org/patch/70435
>
> I don't understand why offers with no text type are cleared here,
> and couldn't find why with git blame either.  Anyone know why
> we've been doing this?

No idea myself, seems a rather arbitrary check. there shouldn't be any
reason this shouldn't work for other mimetypes offered.

>
> With this patch we can get somewhat odd behaviour - if terminology
> exits with a selection it can only be pasted into another terminology
> not a weston-terminal, because text wasn't the first offer and weston
> only saves the first offer.
>
> That seems better than a livelock though.  Maybe we can grab all available
> offers when cloning the selection instead of just the first (but that should
> be done in addition to this patch, I think, because we could have an app that
> doesn't offer text at all exit with a selection set and cause the same lock)

Reviewed-by: Carlos Garnacho <carlosg at gnome.org>

I agree with the conclusion and the approach in this patch. As you say
the next step is storing all offers, but I guess it depends on how
deep we want weston to get into the clipboard manager role. AFAICS
that'd be most similar to how toolkits/apps used to behave wrt
SAVE_TARGETS as defined in the clipboard manager spec [1], where
simply all mimetypes would be stored (see eg. gtk_clipboard_store())

Cheers,
  Carlos

[1] http://www.freedesktop.org/wiki/ClipboardManager/

>
>
>  xwayland/selection.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/xwayland/selection.c b/xwayland/selection.c
> index 25ec848..24aa324 100644
> --- a/xwayland/selection.c
> +++ b/xwayland/selection.c
> @@ -655,8 +655,6 @@ weston_wm_set_selection(struct wl_listener *listener, void *data)
>         struct weston_wm *wm =
>                 container_of(listener, struct weston_wm, selection_listener);
>         struct weston_data_source *source = seat->selection_data_source;
> -       const char **p, **end;
> -       int has_text_plain = 0;
>
>         if (source == NULL) {
>                 if (wm->selection_owner == wm->selection_window)
> @@ -670,28 +668,10 @@ weston_wm_set_selection(struct wl_listener *listener, void *data)
>         if (source->send == data_source_send)
>                 return;
>
> -       p = source->mime_types.data;
> -       end = (const char **)
> -               ((char *) source->mime_types.data + source->mime_types.size);
> -       while (p < end) {
> -               weston_log("  %s\n", *p);
> -               if (strcmp(*p, "text/plain") == 0 ||
> -                   strcmp(*p, "text/plain;charset=utf-8") == 0)
> -                       has_text_plain = 1;
> -               p++;
> -       }
> -
> -       if (has_text_plain) {
> -               xcb_set_selection_owner(wm->conn,
> -                                       wm->selection_window,
> -                                       wm->atom.clipboard,
> -                                       XCB_TIME_CURRENT_TIME);
> -       } else {
> -               xcb_set_selection_owner(wm->conn,
> -                                       XCB_ATOM_NONE,
> -                                       wm->atom.clipboard,
> -                                       XCB_TIME_CURRENT_TIME);
> -       }
> +       xcb_set_selection_owner(wm->conn,
> +                               wm->selection_window,
> +                               wm->atom.clipboard,
> +                               XCB_TIME_CURRENT_TIME);
>  }
>
>  void
> --
> 2.7.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel