From: Damjan Jovanovic Subject: [PATCH 6/7] winex11.drv: improve XDND action negotiations Message-Id: Date: Mon, 25 Apr 2016 03:01:13 +0200 Since XdndPosition can only carry 1 action, and in many cases drag sources allow multiple actions, there's an extension to the XDND spec supported by at least GTK+, Java, and Mono (discovered by a thorough audit of XDND implementations in popular X11 frameworks) where the drag source exposes all allowed actions in its XdndActionList window property, and the single action in XdndPosition is just a suggested action, based on the key state (Ctrl for copy, Shift for move, and both for link) or if nothing is pressed, chosen arbitrarily among available actions (eg. GTK+ chooses copy then move then link, Java move then copy then link). Since Windows allows multiple actions, and vanilla XDND doesn't, let's benefit from this extension when possible. We translate the XdndActionList actions ORed with the XdndPosition action into the drop effects offered to the drop target, but since Windows can only suggest actions through the key state, and we cannot tell whether the drag source suggested an action because Ctrl and Shift keys were pressed or because it was an arbitrary choice, we cannot translate the ambiguously strong proposed action into key state. Rather, obtain key state from GetKeyboardState(), which should be the same as what it was on the drag source. This is backward compatible, as drag sources without XdndActionList can still send an action in XdndPosition, in which case that becomes the only allowed action. Signed-off-by: Damjan Jovanovic --- dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/x11drv_main.c | 1 + dlls/winex11.drv/xdnd.c | 81 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 31f6ec3..6d5d940 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -448,6 +448,7 @@ enum x11drv_atoms XATOM_XdndActionCopy, XATOM_XdndActionMove, XATOM_XdndActionLink, + XATOM_XdndActionList, XATOM_XdndActionAsk, XATOM_XdndActionPrivate, XATOM_XdndSelection, diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index d4f5c84..4f3a72a 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -164,6 +164,7 @@ static const char * const atom_names[NB_XATOMS - FIRST_XATOM] = "XdndActionCopy", "XdndActionMove", "XdndActionLink", + "XdndActionList", "XdndActionAsk", "XdndActionPrivate", "XdndSelection", diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c index 19a1b50..d411537 100644 --- a/dlls/winex11.drv/xdnd.c +++ b/dlls/winex11.drv/xdnd.c @@ -62,6 +62,7 @@ static Time XDNDTimestamp; static IDataObject XDNDDataObject; static BOOL XDNDAccepted = FALSE; static DWORD XDNDDropEffect = DROPEFFECT_NONE; +static DWORD XDNDKeyState = 0; /* the last window the mouse was over */ static HWND XDNDLastTargetWnd; /* might be an ancestor of XDNDLastTargetWnd */ @@ -144,20 +145,15 @@ static IDropTarget* get_droptarget_pointer(HWND hwnd) */ static DWORD X11DRV_XDND_XdndActionToDROPEFFECT(long 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)) 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; + else if (action == x11drv_atom(XdndActionPrivate)) + return DROPEFFECT_COPY; + return 0; } /************************************************************************** @@ -176,6 +172,62 @@ static long X11DRV_XDND_DROPEFFECTToXdndAction(DWORD effect) } /************************************************************************** + * X11DRV_XDND_GetKeyState + */ +static DWORD X11DRV_XDND_GetKeyState(void) +{ + /* On both Windows DND and XDND, we get a set of allowed actions, and + * the suggested action - via key state in Windows, and via XdndPosition + * in XDND. However on XDND this suggestion is weaker, as drag sources + * can send that action in XdndPosition because the user is pressing + * Ctrl/Shift keys, or they can send it as an arbitrary choice among the + * allowed actions (eg. if the user isn't pressing any keys, GTK+ + * sends copy, then move, then link; Java move, then copy, then link). + * We can't distinguish between those cases, so instead of trying to + * translate an ambiguously strong XdndPosition action into key state, + * look up the state of Ctrl/Shift directly, and give it to the drop + * target. + */ + BYTE keyboard[256]; + DWORD keyState = MK_LBUTTON; + if (GetKeyboardState(keyboard)) + { + if (keyboard[VK_CONTROL] & 0x80) + keyState |= MK_CONTROL; + if (keyboard[VK_SHIFT] & 0x80) + keyState |= MK_SHIFT; + } + else + ERR("GetKeyboardState() failed, error %u\n", GetLastError()); + return keyState; +} + +/************************************************************************** + * X11DRV_XDND_ReadAllowedEffects + */ +static DWORD X11DRV_XDND_ReadAllowedEffects(Display *display, Window window, Atom suggestedAction) +{ + Atom acttype; + int actfmt; + unsigned long bytesret; + unsigned long actionCount; + Atom *actions; + DWORD effects = 0; + + if (XGetWindowProperty(display, window, x11drv_atom(XdndActionList), + 0, 65535, FALSE, AnyPropertyType, &acttype, &actfmt, &actionCount, + &bytesret, (unsigned char**)&actions) == Success) + { + unsigned long i; + for (i = 0; i < actionCount; i++) + effects |= X11DRV_XDND_XdndActionToDROPEFFECT(actions[i]); + XFree(actions); + } + effects |= X11DRV_XDND_XdndActionToDROPEFFECT(suggestedAction); + return effects; +} + +/************************************************************************** * X11DRV_XDND_EnterEvent * * Handle an XdndEnter event. @@ -278,7 +330,8 @@ 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 = X11DRV_XDND_ReadAllowedEffects(event->display, XDNDDropSourceWindow, event->data.l[4]); + XDNDKeyState = X11DRV_XDND_GetKeyState(); if (!XDNDAccepted || XDNDLastTargetWnd != targetWindow) { @@ -307,7 +360,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) { DWORD effect_ignore = effect; hr = IDropTarget_DragEnter(dropTarget, &XDNDDataObject, - MK_LBUTTON, pointl, &effect_ignore); + XDNDKeyState, pointl, &effect_ignore); if (hr == S_OK) { XDNDAccepted = TRUE; @@ -327,7 +380,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, XDNDKeyState, pointl, &effect); if (hr == S_OK) XDNDDropEffect = effect; else @@ -396,7 +449,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, XDNDKeyState, pointl, &effect); if (hr == S_OK) {