[PATCH wayland 2/2] protocol: Add DnD actions
Jonas Ådahl
jadahl at gmail.com
Wed Dec 16 17:08:49 PST 2015
On Wed, Dec 16, 2015 at 02:50:59PM +0100, Carlos Garnacho wrote:
> 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.
Ah right, makes sense. So should it instead be "the last message before
wl_data_source.send"? I'd like to have it more strictly specified than
"most recent" as well as at what point it will never be emitted again.
>
> >
> >> + </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.
I see. Do you think any of this should be put in the protocol in any
way? What the moved "data" means is a bit ambiguous
>
> >
> >> + </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 :)
I think I'd vote for leaving the action extension parts and leave the
rest to be undefined and meant for future core additions. In that way, I
don't think using a uint is an issue.
>
> >
> >> +
> >> + 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.
I suppose it's a personal preference, but as the XML is usually read
"raw" and & is hard to read, using immediately readable UTF-8 seems
like an improvement.
Jonas
>
> Cheers,
> Carlos
More information about the wayland-devel
mailing list