[PATCH wayland 2/2] protocol: Add DnD actions
Carlos Garnacho
carlosg at gnome.org
Wed Dec 16 05:50:59 PST 2015
Hey Jonas!
On Wed, Dec 16, 2015 at 2:40 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Tue, Dec 15, 2015 at 06:56:24PM +0100, Carlos Garnacho wrote:
>> These 2 requests have been added:
>>
>> - wl_data_source.set_actions: Notifies the compositor of the available
>> actions on the data source.
>> - wl_data_offer.set_actions: Notifies the compositor of the available
>> actions on the destination side, plus the preferred action.
>>
>> Out of the data from these requests, the compositor can determine the action
>> both parts agree on (and let the user play a role through eg. keyboard
>> modifiers). The chosen option will be notified to both parties
>> through the following two requests:
>>
>> - wl_data_source.action
>> - wl_data_offer.action
>>
>> In addition, the destination side can peek the source side actions through
>> wl_data_offer.source_actions.
>>
>> Compared to the XDND protocol, there's two notable changes:
>>
>> - XDND lets the source suggest an action, whereas wl_data_device lets
>> the destination prefer a given action. The difference is subtle here,
>> it comes off as convenience because it is the drag destination which
>> receives the motion events (unlike in X) and can perform action updates.
>>
>> The drag destination seems also in a better position to update the
>> preferred action based on things like the data being transferred, the
>> place being dropped, and whether the drag is client-local.
>>
>> - That same source-side preferred action is used in XDND to convey the
>> modifier-induced action to the drag destination, which would then ack
>> it, or reply with another action that's accepted (or none), this makes
>> the XdndPosition/XdndStatus messaging very verbose, and synchronous
>> because the drag source always needs to know the latest status/action
>> for every position+action sent.
>>
>> Here it's the compositor which takes care of modifiers and matching
>> available/accepted actions, this allows for the signaling to happen
>> only whenever the actions/modifiers change for real.
>>
>> Roughly based on previous work by Giulio Camuffo <giuliocamuffo at gmail.com>
>>
>> Changes since v4:
>> - Minor rewording.
>>
>> Changes since v3:
>> - Splitted from DnD progress notification changes.
>> - Further rationales in commit log.
>>
>> Changes since v2:
>> - Renamed notify_actions to set_actions on both sides, seems more consistent
>> with the rest of the protocol.
>> - Spelled out better which events may be triggered on the compositor side
>> by the requests, the circumstances in which events are emitted, and
>> what are events useful for in clients.
>> - Defined a minimal common ground wrt compositor-side action picking and
>> keybindings.
>> - Acknowledge the possibility of compositor/toolkit defined actions, even
>> though none are used at the moment.
>> Changes since v1:
>> - Added wl_data_offer.source_actions to let know of the actions offered
>> by a data source.
>> - Renamed wl_data_source.finished to "drag_finished" for clarity
>> - Improved wording as suggested by Bryce
>>
>> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
>> Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com>
>> Reviewed-by: Mike Blumenkrantz <zmike at samsung.com>
>
> Sorry for taking my sweet time, but anyhow, overall I think it looks
> good, but obligatory bikeshedding follows.
Thanks for the review :)
>
>> ---
>> protocol/wayland.xml | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 129 insertions(+)
>>
>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>> index b54bcd0..ab64762 100644
>> --- a/protocol/wayland.xml
>> +++ b/protocol/wayland.xml
>> @@ -468,6 +468,56 @@
>>
>> <arg name="mime_type" type="string"/>
>> </event>
>> +
>> + <!-- Version 3 additions -->
>
> bikeshed: For consistency, I think added requests should always come before
> added events in the XML.
Ah sure, didn't know there was a pattern here.
>
>> +
>> + <event name="source_actions" since="3">
>> + <description summary="notify the source-side available actions">
>> + This event indicates the actions offered by the data source. It
>> + will be sent right after data_device.enter, or anytime the source
>
> wl_data_device.enter.
>
>> + side changes its offered actions through data_source.set_actions.
>> + </description>
>> + <arg name="source_actions" type="uint"/>
>> + </event>
>> +
>> + <event name="action" since="3">
>> + <description summary="notify the selected action">
>> + This event indicates the action selected by the compositor after
>> + matching the source/destination side actions. Only one action (or
>> + none) will be offered here.
>> +
>> + This event can be emitted multiple times during the drag-and-drop
>> + operation, mainly in response to source side changes (through
>> + data_source.set_actions), destination side changes (through
>> + data_offer.set_actions), and as pointer enters/leaves surfaces.
>
> wl_data_source.set_actions and wl_data_offer.set_actions.
>
>> +
>> + Compositors may also change the selected action on the fly, mainly
>> + in response to keyboard modifier changes during the drag-and-drop
>> + operation.
>> +
>> + The most recent action received is always the valid one.
>
> I think it would be good to mention the last action emitted before the
> wl_data_device.drop event is the action that should be used when the
> completing the operation. Also that no "action" event will be emitted
> after the "wl_data_device.drop" event.
Hmm, although that may not be true. In "ask" operations, the drag
destination may switch either mimetype or action after .drop happened.
Examples I can think of:
- [Move|Copy|Link|Cancel] menu in a file manager (this one's not made
up, nautilus has this)
- [Paste as plain text|Paste with format] menu in some wysiwyg editor
In these cases the final action will be possibly chosen well after
.drop happened, and will be different to the one happening at that
time (ask). But the drag source won't receive .drag_finished/cancelled
until that last action is chosen and dealt with, so it is that one
which would be honored. It is a good idea to explain the possible
"ask" behavior further here.
>
>> + </description>
>> + <arg name="dnd_action" type="uint"/>
>> + </event>
>> +
>> + <request name="set_actions" since="3">
>> + <description summary="set the available/preferred drag-and-drop actions">
>> + Sets the actions that the destination side client supports for
>> + this operation. This request may trigger the emission of
>> + data_source.action and data_offer.action events if the compositor
>
> wl_data_source.action and wl_data_offer.action.
>
>> + needs changing the selected action.
>> +
>> + This request can be called multiple times throughout the
>> + drag-and-drop operation, typically in response to data_device.enter
>> + or data_device.motion events.
>
> wl_data_device.enter and wl_data_device.motion.
>
>> +
>> + This request determines the final result of the drag-and-drop
>> + operation. If the end result is that no action is accepted,
>> + the drag source will receive drag_source.cancelled.
>
> wl_data_source.cancelled.
>
>> + </description>
>> + <arg name="dnd_actions" type="uint"/>
>> + <arg name="preferred_action" type="uint"/>
>> + </request>
>> </interface>
>>
>> <interface name="wl_data_source" version="3">
>> @@ -524,6 +574,9 @@
>> - The drag-and-drop operation was performed, but the drop destination
>> did not accept any of the mimetypes offered through
>> data_source.target.
>
> I know this is the wrong patch for commenting on this, but here was
> spaces instead of tabs as well (the same for the point below the added
> one).
Meh, sorry about these. Too many different indentation schemes around,
I open the file with the wrong emacs window and boom.
>
>> + - The drag-and-drop operation was performed, but the drop destination
>> + did not select any action present in the mask offered through
>> + data_source.action.
>
> Mixed tabs and spaces (indentation).
>
>> - The drag-and-drop operation was performed but didn't happen over a
>> surface.
>> - The compositor cancelled the drag-and-drop operation
>> @@ -558,8 +611,44 @@
>> The drop destination finished interoperating with this data
>> source, the client is now free to destroy this data source and
>> free all associated data.
>> +
>> + Clients can also trigger the deletion of source-side data on
>> + "move" drag-and-drop operations.
>
> This wording seems a bit off to me. At first it sounded like I was back
> at the wl_data_offer side and it could trigger the deletion of client
> side things, but then I remembered what interface it was in. A
> suggestion:
>
> If the action used to perform the operation was "move", the
> source can now delete the transferred data.
Right, I agree it sounds clearer.
>
> A side thought; how is this supposed to work when move does not require
> any deletion? I.e. a file moved within a filesystem.
In a file manager there's indeed special considerations around "move"
operations, rename(2) is only possible if files move across the same
filesystem, otherwise unlink(2) must happen after the file was fully
copied. Because at the DnD level everything they transfer is URIs, and
copy/move operations happen off-band, I'd expect the final delete to
happen off-band too.
At the DnD level, what AFAICT file managers usually do is:
- Never explicitly delete as a result of "move" operations
- Disallow/ignore "move" operations on drags to a different process,
because in these cases you know nothing about the operation performed
afterwards, only that the URIs were transferred correctly.
It makes some sense if you think of the DnD transfer being purely
about the URIs, the file manager transfers an URI list and an intent,
and it does succeed at that, regardless of the copy operation being
cancelled afterwards by the user, the destination partition being
full, the ssh connection dropping... in terms of DnD, the URI list
triggered something on the destination, anything that happens after
that belongs somewhere else.
>
>> + </description>
>> + </event>
>> +
>> + <event name="action" since="3">
>> + <description summary="notify the selected action">
>> + This event indicates the action selected by the compositor after
>> + matching the source/destination side actions. Only one action (or
>> + none) will be offered here.
>> +
>> + This event can be emitted multiple times during the drag-and-drop
>> + operation, mainly in response to source side changes (through
>> + data_source.set_actions), destination side changes (through
>> + data_offer.set_actions), and as pointer enters/leaves surfaces.
>
> wl_data_source.set-actions and wl_data_offer.set_actions.
>
> and
>
> "..., and as the pointer enter/leaves ...".
Oops, thanks for spotting
>
>> +
>> + Compositors may also change the selected action on the fly, mainly
>> + in response to keyboard modifier changes during the drag-and-drop
>> + operation.
>> +
>> + The most recent action received is always the valid one.
>> +
>> + Clients can trigger cursor surface changes from this point, so
>> + they reflect the current action.
>
> Here as well I think it'd be good to add a note about what action should
> be used to perform the operation, i.e. the last action emitted before
> the "dnd_drop_performed" event. Maybe even specify that no events will
> ever be emitted after the "dnd_drop_performed" event.
>
>> </description>
>> + <arg name="dnd_action" type="uint"/>
>> </event>
>> +
>> + <request name="set_actions" since="3">
>> + <description summary="set the available drag-and-drop actions">
>> + Sets the actions that the source side client supports for this
>> + operation. This request may trigger a data_source.action event and
>> + data_offer.action events if the compositor needs changing the
>
> wl_data_source.action and wl_data_offer.action.
>
>> + selected action.
>> + </description>
>> + <arg name="dnd_actions" type="uint"/>
>> + </request>
>> </interface>
>>
>> <interface name="wl_data_device" version="3">
>> @@ -727,6 +816,46 @@
>> <arg name="id" type="new_id" interface="wl_data_device"/>
>> <arg name="seat" type="object" interface="wl_seat"/>
>> </request>
>> +
>> + <!-- Version 3 additions -->
>> +
>> + <enum name="dnd_action" since="3">
>> + <description summary="drag and drop actions">
>> + This is a bitmask of the available/preferred actions in a
>> + drag-and-drop operation.
>> +
>> + The current reserved ranges are:
>> +
>> + 0x0000 - 0x00FF: Reserved for the wayland core protocol.
>> + 0x01FF - 0xFFFF: Reserved for future toolkit-specific use. Slots
>> + may be reserved.
>
> This means we'll have 8 possible core actions, and 24 that may be
> reserved. 8 I suppose is enough for core actions, but is 24 really
> enough for all eternity if they are meant to be allocated by different
> toolkits and applications? If we're to make it reservable it seems to be
> better to make it more long term sustainable, i.e. not making it a bit
> mask but a regular enum.
>
> This means that some of the events and requests would have to use array
> instead of uint, which doesn't seem that bad.
Uhm, it looks like the matching of actions would be harder then...
At the expense of sounding too optimistic here, I think this will be
mostly unused. XDND defines the following actions:
- XdndActionCopy
- XdndActionMove
- XdndActionAsk
- XdndActionLink
- XdndActionPrivate
It has stayed like that for ~17 years, and I dare say that the latter
2 are mostly unused. In the end I think that the two things that
matter in the context of DnD are:
- Whether the destination found the offered data satisfactory
- Whether the source has to signal deletion
And none/move/copy cover all the combinations of these conditions we
care about (ask being a sort of extension point), everything else IMO
belongs to drag destination app-specific context (thus custom
mimetypes, local-only DnD, etc)
In that scheme, I think slots should be reserved with real
actions/arguments behind every requested bit, and we should fold into
the core actions anything we see popular enough. Although I'd be just
as happy to drop the action extension blurbs and wait for others to
contend about extra actions in the future :)
>
>> +
>> + In the compositor, the selected action comes out as a result of
>> + matching the actions offered by the source and destination sides,
>
> Full stop here.
>
>> + "action" events with a "none" action will be sent to both source
>> + and destination if there is no match. All further checks will
>> + effectively happen on (source_actions & dest_actions).
>
> s/&/∩/ (we're using UTF-8 so can just use the unicode character for
> intersection here).
>
> I also think there is no need variable name like things, i.e. I think it
> can just be (source actions ∩ destination actions).
Sure, didn't know set algebra symbols were preferred here.
Cheers,
Carlos
More information about the wayland-devel
mailing list