[PATCH wayland 1/2] protocol: Improve data source notification around DnD progress

[Carlos Garnacho]

Hey,

On Wed, Dec 23, 2015 at 5:47 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> Hi again,
>
> I was reading an E-mail in another thread that brought up different
> types of backward compatibility promises, and it made me think of a
> potential issue. I'm commenting inline close to the relevant change this
> patch introduces.
>
> On Tue, Dec 22, 2015 at 02:33:32AM +0100, Carlos Garnacho wrote:
>> Currently, there's no means for the DnD origin to know whether the
>> destination is actually finished with the DnD transaction, short of
>> finalizing it after the first transfer finishes, or leaking it forever.
>>
>> But this poses other interoperation problems, drag destinations might
>> be requesting several mimetypes at once, might be just poking to find
>> out the most suitable format, might want to defer the action to a popup,
>> might be poking contents early before the selection was dropped...
>>
>> In addition, data_source.cancelled is suitable for the situations where
>> the DnD operation fails (not on a drop target, no matching mimetypes,
>> etc..), but seems undocumented for that use (and unused in weston's DnD).
>>
>> In order to improve the situation, the drag source should be notified
>> of all stages of DnD. In addition to documenting the "cancelled" event
>> for DnD purposes, The following 2 events have been added:
>>
>> - wl_data_source.dnd_drop_performed: Happens when the operation has been
>>   physically finished (eg. the button is released), it could be the right
>>   place to reset the pointer cursor back and undo any other state resulting
>>   from the initial button press.
>> - wl_data_source.dnd_finished: Happens when the destination side destroys
>>   the wl_data_offer, at this point the source can just forget all data
>>   related to the DnD selection as well, plus optionally deleting the data
>>   on move operations.
>>
>> Changes since v4:
>>   - Applied rewording suggestions from Jonas Ådahl. Added new
>>     wl_data_offer.finish request to allow explicit finalization on the
>>     destination side.
>>
>> Changes since v3:
>>   - Renamed dnd_performed to a more descriptive dnd_drop_performed,
>>     documented backwards compatible behavior on wl_data_offer.accept and
>>     wl_data_source.cancelled.
>>
>> Changes since v2:
>>   - Minor rewording.
>>
>> Changes since v1:
>>   - Renamed events to have a common "dnd" namespace. Made dnd_performed to
>>     happen invariably, data_device.cancelled may still happen afterwards.
>>
>> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
>> Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com>
>> Reviewed-by: Jonas Ådahl <jadahl at gmail.com>
>> Reviewed-by: Mike Blumenkrantz <zmike at samsung.com>
>> ---
>>  protocol/wayland.xml | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 69 insertions(+), 6 deletions(-)
>>
>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>> index df8ed19..ae5ef21 100644
>> --- a/protocol/wayland.xml
>> +++ b/protocol/wayland.xml
>> @@ -408,7 +408,7 @@
>>    </interface>
>>
>>
>> -  <interface name="wl_data_offer" version="1">
>> +  <interface name="wl_data_offer" version="3">
>>      <description summary="offer to transfer data">
>>        A wl_data_offer represents a piece of data offered for transfer
>>        by another client (the source client).  It is used by the
>> @@ -423,7 +423,17 @@
>>       Indicate that the client can accept the given mime type, or
>>       NULL for not accepted.
>>
>> -     Used for feedback during drag-and-drop.
>> +     For objects of version 2 or older, this request is used by the
>> +     client to give feedback whether the client can receive the given
>> +     mime type, or NULL if none is accepted; the feedback does not
>> +     determine whether drag-and-drop operation succeeds or not.
>> +
>> +     For objects of version 3 or newer, this request determines the
>> +     final result of the drag-and-drop operation. If the end result
>> +     is that no mime types was accepted, the drag-and-drop operation
>> +     will be cancelled and the corresponding drag source will receive
>> +     wl_data_source.cancelled. Clients may still use this event in
>> +     conjunction with wl_data_source.action for feedback.
>
> As Pekka (CC:ed) wrote in a reply to the proxy version tracking thread
> [0], the kind of change that is introduced above is not really backward
> compatible. The possible issue would be, if I understand things
> correctly, that if we have a client split up into two separate parts (an
> application and a third party library); if the application creates a
> wl_data_device of version 3 and then passes it (or a wl_data_offer) to
> the library that only support version 1 or 2, the client will break
> because according to version < 3 there is no requirement to reason to
> reply with "accept" other than wishing to provide optional feedback, and
> the library may rely on this.

Uhm, I see no way out of these misbehaviors. If an app manages a v3
wl_data_offer (or source) with v2-style, it will simply not honor
negotiation, we'll get a 0 action mask and drag-and-drop will fail. I
see more trouble though when adding listeners, if the one doing it
thinks this is v2....

It would fail even if we take mimetype acknowledgement out of .accept,
because the caller wouldn't be calling the new request either. In all
honesty, I consider v2 broken and something we should hide under the
rug soon, there is no way we can stay both backward compatible and
sane at the same time. If such library is more than a proof of
concept, I'd urge it to update to v3 ASAP.

>
> I think the "wl_data_source.cancelled" change is backward compatible
> though, since it doesn't alter any expectations from client.

And I guess the answer to this is "we don't know"... we used to only
send it on clipboard sources. If that expectation cristalized in some
client, we might trigger crashes or unexpected code paths.

Cheers,
  Carlos