[Spice-devel] [PATCH] client: support clipboard/selection-owner model

[Hans de Goede]

Hi,

On a second review I've found some more issues with the X11 client
copy paste code, note some of this were already present before
the patch in question.

1) You're using the XA_PRIMARY selection, this is the one which
    gets set as soon as you select anything with the mouse cursor,
    no need to press ctrl-c or ctrl-x to set it, so it gets changed
    all the time! Note this is also the one which usually gets pasted
    with the middle mouse button. I believe that rather then the
    XA_PRIMARY you want the selection identified by the clipboard
    atom, which uses the normal ctrl-c + ctrl-v copy / paste behavior

2) In the SelectionNotify event case in root_win_proc the following
    check is made:
         if (type != event.xselection.target) {

    But type can be the INCR atom as well and this is valid too, as the
    code acknowledges by checking for it later on:
         if (type == incr_atom) {

    Which will never be reached as the above check will fail in this
    case.

    Also when the type is INCR, there is no reason to do the second
    request with the indicated size, as the resulting data (which is an
    32 bit integer indicating the minimum size of the data which will
    be send) will get thrown away anyways.

3) In the none INCR case the property is never deleted, but it should
    be once we're done reading it, see:
    http://tronche.com/gui/x/icccm/sec-2.html#s-2.4

    At the end of the paragraph, I believe we can do this by simply
    setting the delete flag to true when the second XGetWindowProperty
    call is made.

4) If we get a SelectionRequest with a type which we do not support, we
    should send back a SelectionNotify event with the property set to none
    to indicate that we cannot fulfill the request.

    Like wise when Platform::request_clipboard_notification fails, or the
    client receives a SelectionNotify with a property of None in response
    to the Platform::request_clipboard_notification() the client should
    send a message to the agent that it could not get the desired data
    (so that under Linux the agent can generate a SelectionNotify with a
     property of None to let the requesting app inside the guest know
     we cannot give it the type it is asking for).

    I suggest we add a VD_AGENT_CLIPBOARD_NONE for this to the below enum,
enum {
     VD_AGENT_CLIPBOARD_UTF8_TEXT = 1,
     VD_AGENT_CLIPBOARD_BITMAP,
};

    We could make this have value 0.

5) When receiving a SelectionRequest event, we MUST always accept the
    TARGETS and MULTIPLE atoms as targets, see:
    http://tronche.com/gui/x/icccm/sec-2.html#s-2.6.2

    The handling of neither of this is really easy, for TARGETS we should
    ask the agent what types (VD_AGENT_CLIPBOARD_*) the current clipboard
    contents can be converted too, and then report that back. This
    requires some vd agent protocol enhancements.

    The handling of MULTIPLE is no fun either, as that will require
    multiple round trips to the agent, and we need to keep track
    where we are in the array of requests.

Regards,

Hans