From: Damjan Jovanovic Subject: [PATCH 6/7] winex11.drv: improve relaying drop effects to the drop target (try 3) Message-Id: Date: Sat, 18 Jul 2015 10:43:11 +0200 The XDND spec provides only a single action that can be offered to the drop target through the XdndPosition message. In practice this is too restrictive: many drag sources can offer multiple actions, of which only some will be suitable for a drop target. When dragging a file from a file manager, the safest single action to offer is COPY, but the trash can only accepts MOVE, so you'd have to Shift+drag a file to be able to drop it in the trash can. GTK+, Java and Mono all use an extension to the XDND spec, in which there is a property, XdndActionList, containing a list of all actions (as X11 atoms) that can be performed, while XdndPosition's single action is just the suggested or preferred action. On Windows the drop target gets provided with a set of allowed actions, and the key state that indicates the suggested action. There is no way to indicate key state on XDND - XdndPosition's data.l[1] contains event state, but doesn't contain enough of it to tell us a right drag is happening, and besides, absolutely nobody, not even the spec's reference implementation, actually reads or writes data.l[1] in XdndPosition. So what we do is translate XdndActionList into pdwEffect, and translate XdndPosition's suggested action into the key state that would have generated that action. Try 2 and 3: no changes. Damjan Jovanovic --- dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/x11drv_main.c | 1 + dlls/winex11.drv/xdnd.c | 124 +++++++++++++++++++++++++++++++++-------- 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 7135c73..5f277e3 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -459,6 +459,7 @@ enum x11drv_atoms XATOM_XdndSelection, XATOM_XdndTarget, XATOM_XdndTypeList, + XATOM_XdndActionList, XATOM_HTML_Format, XATOM_WCF_BITMAP, XATOM_WCF_DIB, diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 85c69bf..af730cd 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -168,6 +168,7 @@ static const char * const atom_names[NB_XATOMS - FIRST_XATOM] = "XdndSelection", "XdndTarget", "XdndTypeList", + "XdndActionList", "HTML Format", "WCF_BITMAP", "WCF_DIB", diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c index cdb79ad..9cd72da 100644 --- a/dlls/winex11.drv/xdnd.c +++ b/dlls/winex11.drv/xdnd.c @@ -61,6 +61,7 @@ static POINT XDNDxy = { 0, 0 }; static Time XDNDTimestamp; static IDataObject XDNDDataObject; static BOOL XDNDAccepted = FALSE; +static DWORD XDNDKeys = 0; static DWORD XDNDDropEffect = DROPEFFECT_NONE; /* the last window the mouse was over */ static HWND XDNDLastTargetWnd; @@ -139,31 +140,33 @@ static IDropTarget* get_droptarget_pointer(HWND hwnd) return droptarget; } +/* We use GTK+'s, Mono's and Java's extension to the XDND spec. The + * XdndActionList property is always set on the source window, and contains a + * list of all allowed actions. The single action in the XdndPosition event, + * chosen by the drag source based on the Ctrl/Shift/Alt key state, only + * chooses a preferred action among these. On the drop target side, + * the union of XdndActionList and the preferred action become the drop + * effects offered to the drop target, while the preferred action is translated + * into the keyboard buttons that indicate that action. */ + /************************************************************************** * X11DRV_XDND_XdndActionToDROPEFFECT */ -static DWORD X11DRV_XDND_XdndActionToDROPEFFECT(long action) +static DWORD X11DRV_XDND_XdndActionToDROPEFFECT(Atom action) { - /* In Windows, nothing but the given effects is allowed. - * In X the given action is just a hint, and you can always - * XdndActionCopy and XdndActionPrivate, so be more permissive. */ - if (action == x11drv_atom(XdndActionCopy)) + if (action == x11drv_atom(XdndActionCopy) || action == x11drv_atom(XdndActionPrivate)) return DROPEFFECT_COPY; else if (action == x11drv_atom(XdndActionMove)) - return DROPEFFECT_MOVE | DROPEFFECT_COPY; + return DROPEFFECT_MOVE; else if (action == x11drv_atom(XdndActionLink)) - return DROPEFFECT_LINK | DROPEFFECT_COPY; - else if (action == x11drv_atom(XdndActionAsk)) - /* FIXME: should we somehow ask the user what to do here? */ - return DROPEFFECT_COPY | DROPEFFECT_MOVE | DROPEFFECT_LINK; - FIXME("unknown action %ld, assuming DROPEFFECT_COPY\n", action); - return DROPEFFECT_COPY; + return DROPEFFECT_LINK; + return DROPEFFECT_NONE; } /************************************************************************** * X11DRV_XDND_DROPEFFECTToXdndAction */ -static long X11DRV_XDND_DROPEFFECTToXdndAction(DWORD effect) +static Atom X11DRV_XDND_DROPEFFECTToXdndAction(DWORD effect) { if (effect == DROPEFFECT_COPY) return x11drv_atom(XdndActionCopy); @@ -171,8 +174,77 @@ static long X11DRV_XDND_DROPEFFECTToXdndAction(DWORD effect) return x11drv_atom(XdndActionMove); else if (effect == DROPEFFECT_LINK) return x11drv_atom(XdndActionLink); - FIXME("unknown drop effect %u, assuming XdndActionCopy\n", effect); - return x11drv_atom(XdndActionCopy); + return None; +} + +static DWORD get_allowed_dropeffects(Display *display, Window window) +{ + Atom acttype; + int actfmt; + unsigned long bytesret; + unsigned long count; + Atom *actionList = NULL; + + XGetWindowProperty(display, window, x11drv_atom(XdndActionList), 0, 65535, + FALSE, XA_ATOM, &acttype, &actfmt, &count, &bytesret, (unsigned char**)&actionList); + if (actionList != NULL && acttype == XA_ATOM && actfmt == 32) + { + int i; + DWORD dropEffects = 0; + for (i = 0; i < count; i++) + dropEffects |= X11DRV_XDND_XdndActionToDROPEFFECT(actionList[i]); + XFree(actionList); + return dropEffects; + } + return 0; +} + +static void set_XdndActionList(DWORD dwOKEffect, Window window) +{ + int count = 0; + Atom actions[3]; + + if (dwOKEffect & DROPEFFECT_COPY) + actions[count++] = x11drv_atom(XdndActionCopy); + if (dwOKEffect & DROPEFFECT_MOVE) + actions[count++] = x11drv_atom(XdndActionMove); + if (dwOKEffect & DROPEFFECT_LINK) + actions[count++] = x11drv_atom(XdndActionLink); + XChangeProperty(thread_display(), window, x11drv_atom(XdndActionList), XA_ATOM, 32, + PropModeReplace, (const unsigned char*)actions, count); +} + +static Atom choose_action(DWORD allowedEffects, DWORD keyState) +{ + DWORD modifierState = keyState & (MK_CONTROL | MK_SHIFT | MK_ALT); + if ((modifierState == MK_CONTROL) && (allowedEffects & DROPEFFECT_COPY)) + return x11drv_atom(XdndActionCopy); + else if ((modifierState == MK_SHIFT) && (allowedEffects & DROPEFFECT_MOVE)) + return x11drv_atom(XdndActionMove); + /* Ctrl+Shift are used instead of Alt on X11, so allow either */ + else if (((modifierState == MK_ALT) || (modifierState == (MK_CONTROL | MK_SHIFT))) && + (allowedEffects & DROPEFFECT_LINK)) + return x11drv_atom(XdndActionLink); + + if (allowedEffects & DROPEFFECT_COPY) + return x11drv_atom(XdndActionCopy); + else if (allowedEffects & DROPEFFECT_MOVE) + return x11drv_atom(XdndActionMove); + else if (allowedEffects & DROPEFFECT_LINK) + return x11drv_atom(XdndActionLink); + else + return None; +} + +static DWORD get_keys_for_action(Atom action) +{ + if (action == x11drv_atom(XdndActionCopy) || action == x11drv_atom(XdndActionPrivate)) + return MK_CONTROL; + else if (action == x11drv_atom(XdndActionMove)) + return MK_SHIFT; + else if (action == x11drv_atom(XdndActionLink)) + return MK_ALT; + return 0; } /************************************************************************** @@ -286,7 +358,9 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) pointl.x = XDNDxy.x; pointl.y = XDNDxy.y; XDNDTimestamp = event->data.l[3]; - effect = X11DRV_XDND_XdndActionToDROPEFFECT(event->data.l[4]); + effect = get_allowed_dropeffects(event->display, event->data.l[0]) | + X11DRV_XDND_XdndActionToDROPEFFECT(event->data.l[4]); + XDNDKeys = MK_LBUTTON | get_keys_for_action(event->data.l[4]); if (!XDNDAccepted || XDNDLastTargetWnd != targetWindow) { @@ -314,7 +388,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) if (dropTarget) { hr = IDropTarget_DragEnter(dropTarget, &XDNDDataObject, - MK_LBUTTON, pointl, &effect); + XDNDKeys, pointl, &effect); if (SUCCEEDED(hr)) { if (effect != DROPEFFECT_NONE) @@ -336,7 +410,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) dropTarget = get_droptarget_pointer(XDNDLastDropTargetWnd); if (dropTarget) { - hr = IDropTarget_DragOver(dropTarget, MK_LBUTTON, pointl, &effect); + hr = IDropTarget_DragOver(dropTarget, XDNDKeys, pointl, &effect); if (SUCCEEDED(hr)) XDNDDropEffect = effect; else @@ -407,7 +481,7 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event ) pointl.x = XDNDxy.x; pointl.y = XDNDxy.y; - hr = IDropTarget_Drop(dropTarget, &XDNDDataObject, MK_LBUTTON, + hr = IDropTarget_Drop(dropTarget, &XDNDDataObject, XDNDKeys, pointl, &effect); if (SUCCEEDED(hr)) { @@ -1200,8 +1274,9 @@ static void next_action(BOOL sendPositionEvenIfNotQueued) { if (sendPositionEvenIfNotQueued || dragState.queuedNextPosition) { - send_XdndPosition(dragState.source, X11DRV_XDND_DROPEFFECTToXdndAction(dragState.dwOKEffect), - &dragState.curMousePos, dragState.timeXdndSelectionAcquired, dragState.target); + Atom action = choose_action(dragState.dwOKEffect, dragState.dwKeyState); + send_XdndPosition(dragState.source, action, &dragState.curMousePos, + dragState.timeXdndSelectionAcquired, dragState.target); dragState.timeXdndPositionSent = GetTickCount64(); dragState.awaitingStatus = TRUE; dragState.queuedNextPosition = FALSE; @@ -1225,8 +1300,9 @@ static void next_action(BOOL sendPositionEvenIfNotQueued) if (*dragState.pdwEffect != DROPEFFECT_NONE) { /* Send a final XdndPosition to set the mouse position and action before the drop */ - send_XdndPosition(dragState.source, X11DRV_XDND_DROPEFFECTToXdndAction(dragState.dwOKEffect), - &dragState.curMousePos, dragState.timeXdndSelectionAcquired, dragState.target); + Atom action = choose_action(dragState.dwOKEffect, dragState.dwKeyState); + send_XdndPosition(dragState.source, action, &dragState.curMousePos, + dragState.timeXdndSelectionAcquired, dragState.target); dragState.timeXdndPositionSent = GetTickCount64(); dragState.awaitingStatus = TRUE; dragState.isDropping = TRUE; @@ -1626,6 +1702,8 @@ HRESULT CDECL X11DRV_DoDragDrop ( return E_FAIL; } + set_XdndActionList(dragState.dwOKEffect, dragState.source); + /* * Capture the mouse input */