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

[Carlos Garnacho]

Hey Michael :),

On Thu, Oct 1, 2015 at 12:43 AM, Michael Catanzaro
<mcatanzaro at igalia.com> wrote:
> Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com>
>
> These are important fixes for the protocol that will allow drags to
> work between GTK+ and Chrome. Thanks for working on this, Carlos!

Thanks for the review!

>
> Nit #1: the existing documentation doesn't use the DnD abbreviation, so
> I would write out "drag-and-drop" in the documentation of the cancelled
> event.
>
> Nit #2: You have a comma splice in the documentation for drag_finished;
> you should change it to a semicolon, or split it into two sentences, or
> add a conjunction.

Right :), fixing these locally. Will send another patch when we've got
an outcome for the question below.

>
> Minor issue: this and the next patch introduce the requirement that
> accept is called on the data offer before the mouse grab is released. I
> now see this has always been required by mutter -- not sure why -- but
> it's new for Weston. (In fact, sending accepts has always been
> optional.) The new restriction seems a bit arbitrary and could be hard
> for some clients. Chrome's cross-platform DnD abstraction all but
> requires drag data to have been received before it can determine
> whether to accept a mime type, so Chrome doesn't send an accept until
> it's done receiving. Previously, that just meant the cursor would not
> be updated in response to motion events during that time, but now it
> means the drop will fail. This is not a huge deal, and the other
> changes in these patches are much more important, but it suggests we
> shouldn't add this restriction unless there's a good reason behind
> it... and I don't know of one, so I'd like to understand that. The
> change isn't necessary for the protocol as-is, and doesn't seem
> necessary for DnD actions, either?

This is in order to be able to emit .cancelled if there is no
agreement between source and destination about the data to be
transferred. Otherwise every drop on any client with a wl_data_device
would seem successful.

I know .accept has had no functional role in the DnD state machine
till now (besides UI feedback on the source-side, setting the
appropriate cursor/dnd surfaces mostly), it made sense to me to reuse
this though because compositor-side UI feedback and overall DnD
outcome would be in sync for virtually every usecase I can imagine. I
would expect .accept() to be called regularly on DnD motion over a
surface, the pointer moves across inner widgets that accept different
mimetypes or none.

Actually, IMO there should be checks somewhere about the mimetype
being one of the offered ones, in order to behave properly on wrong
requests, although that means keeping the mimetype list in memory
instead of just forwarding it.

However, I can imagine other usecases where we don't know any concrete
mimetype beforehand, eg. a "paste as html/plain text" popup menu on
.drop_performed, there'd be no other way in these cases than
preemptively picking one, as you say in your other mail. So perhaps it
makes sense to split into a request with a boolean after all?
Nonetheless, for the case of GTK+, both would be called from the same
place (and .accept is already invariably called with the first
mimetype from the list, hmm...)

Cheers,
  Carlos