[PATCH v2] winex11.drv: Search for the drop target in a correct manner.

[John Found]

Signed-off-by: John Found 
---
 dlls/winex11.drv/xdnd.c | 50 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c
index 2ab28e43bf..ac2fc644dc 100644
--- a/dlls/winex11.drv/xdnd.c
+++ b/dlls/winex11.drv/xdnd.c
@@ -254,6 +254,34 @@ void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event )
         XFree(xdndtypes);
 }

+/* Recursively searches for a window on given coordinates in a drag&drop specific manner.
+ *
+ * Don't use WindowFromPoint instead, because it omits the STATIC and transparent
+ * windows, but they can be a valid drop targets if have WS_EX_ACCEPTFILES set.
+ */
+static HWND window_from_point_dnd(HWND hwnd, POINT point)
+{
+    HWND child;
+    ScreenToClient(hwnd, &point);
+    while ((child = ChildWindowFromPointEx(hwnd, point, CWP_SKIPDISABLED | CWP_SKIPINVISIBLE)) && child != hwnd)
+    {
+       MapWindowPoints(hwnd, child, &point, 1);
+       hwnd = child;
+    }
+
+    return hwnd;
+}
+
+/* Returns the first window down the hierarchy that has WS_EX_ACCEPTFILES set or
+ * returns NULL, if such window does not exists.
+ */
+static HWND window_accepting_files(HWND hwnd)
+{
+    while (hwnd && !(GetWindowLongW(hwnd, GWL_EXSTYLE) & WS_EX_ACCEPTFILES))
+        hwnd = GetAncestor(hwnd, GA_PARENT);
+    return hwnd;
+}
+
 /**************************************************************************
  * X11DRV_XDND_PositionEvent
  *
@@ -270,7 +298,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )
     HRESULT hr;

XDNDxy = root_to_virtual_screen( event->data.l[2] >> 16, event->data.l[2] & 0xFFFF );
-    targetWindow = WindowFromPoint(XDNDxy);
+    targetWindow = window_from_point_dnd(hWnd, XDNDxy);

pointl.x = XDNDxy.x;
     pointl.y = XDNDxy.y;
@@ -334,11 +362,15 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )

if (XDNDAccepted)
         accept = 1;
-    else if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
-            X11DRV_XDND_HasHDROP())
+    else
     {
-        accept = 1;
-        effect = DROPEFFECT_COPY;
+        /* fallback search for window able to accept these files. */
+
+        if (window_accepting_files(targetWindow) && X11DRV_XDND_HasHDROP())
+        {
+            accept = 1;
+            effect = DROPEFFECT_COPY;
+        }
     }

TRACE("actionRequested(%ld) accept(%d) chosen(0x%x) at x(%d),y(%d)\n",
@@ -423,10 +455,12 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event )
     {
         /* Only send WM_DROPFILES if Drop didn't succeed or DROPEFFECT_NONE was set.
          * Doing both causes winamp to duplicate the dropped files (#29081) */
-        if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
-                X11DRV_XDND_HasHDROP())
+
+        HWND hwnd_drop = window_accepting_files(window_from_point_dnd(hWnd, XDNDxy));
+
+        if (hwnd_drop && X11DRV_XDND_HasHDROP())
         {
-            HRESULT hr = X11DRV_XDND_SendDropFiles( hWnd );
+            HRESULT hr = X11DRV_XDND_SendDropFiles(hwnd_drop);
             if (SUCCEEDED(hr))
             {
                 accept = 1;

[John Found]

After some more tests with a real Windows it turned, 
that using GetAncestor() instead of GetParent() is a mistake.

In fact, the popup window, owned by a window with WS_EX_ACCEPTFILES also becomes a drop target and the 
WM_DROPFILES is sent to the **owner** window. This way the proper solution is to use exactly GetParent 
that returns the owner window (only for the popup windows) as well.

So, patch v3 is to be released soon.

On Sat, 20 Apr 2019 10:20:27 +0300
"John Found"  wrote:
> Signed-off-by: John Found
>
> dlls/winex11.drv/xdnd.c | 50 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c
> index 2ab28e43bf..ac2fc644dc 100644
> --- a/dlls/winex11.drv/xdnd.c
> +++ b/dlls/winex11.drv/xdnd.c
> @@ -254,6 +254,34 @@ void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event )
>          XFree(xdndtypes);
>  }
>
> +/* Recursively searches for a window on given coordinates in a drag&drop specific manner.
> -
> -
> -
> - Don't use WindowFromPoint instead, because it omits the STATIC and transparent
> -
> - windows, but they can be a valid drop targets if have WS_EX_ACCEPTFILES set.
> - */
>
> +static HWND window_from_point_dnd(HWND hwnd, POINT point)
> +{
> - HWND child;
> - ScreenToClient(hwnd, &point);
> - while ((child = ChildWindowFromPointEx(hwnd, point, CWP_SKIPDISABLED | CWP_SKIPINVISIBLE)) && child != hwnd)
> - {
> -
>   MapWindowPoints(hwnd, child, &point, 1);
> -
>   hwnd = child;
> - }
> -
> - return hwnd;
>
> +}
> -
>
> +/* Returns the first window down the hierarchy that has WS_EX_ACCEPTFILES set or
> -
> - returns NULL, if such window does not exists.
> - */
>
> +static HWND window_accepting_files(HWND hwnd)
> +{
> - while (hwnd && !(GetWindowLongW(hwnd, GWL_EXSTYLE) & WS_EX_ACCEPTFILES))
> -
>    hwnd = GetAncestor(hwnd, GA_PARENT);
> - return hwnd;
>
> +}
> -
>
> /**************************************************************************
> - X11DRV_XDND_PositionEvent
> -
>
> @@ -270,7 +298,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )
>      HRESULT hr;
>  XDNDxy = root_to_virtual_screen( event->data.l[2] >> 16, event->data.l[2] & 0xFFFF );
> - targetWindow = WindowFromPoint(XDNDxy);
> -
>
> targetWindow = window_from_point_dnd(hWnd, XDNDxy);
>
> pointl.x = XDNDxy.x;
> pointl.y = XDNDxy.y;
>
> @@ -334,11 +362,15 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )
>  if (XDNDAccepted)
>      accept = 1;
> - else if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
> -
>        X11DRV_XDND_HasHDROP())
> - else
> {
> -
>    accept = 1;
> -
>    effect = DROPEFFECT_COPY;
> -
>    /* fallback search for window able to accept these files. */
> -
> -
>    if (window_accepting_files(targetWindow) && X11DRV_XDND_HasHDROP())
> -
>    {
> -
>        accept = 1;
> -
>        effect = DROPEFFECT_COPY;
> -
>    }
>
> }
>
> TRACE("actionRequested(%ld) accept(%d) chosen(0x%x) at x(%d),y(%d)\n",
>
> @@ -423,10 +455,12 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event )
>      {
>          /* Only send WM_DROPFILES if Drop didn't succeed or DROPEFFECT_NONE was set.
>           * Doing both causes winamp to duplicate the dropped files (#29081) */
> -
>    if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
> -
>            X11DRV_XDND_HasHDROP())
> -
> -
>    HWND hwnd_drop = window_accepting_files(window_from_point_dnd(hWnd, XDNDxy));
> -
> -
>    if (hwnd_drop && X11DRV_XDND_HasHDROP())
>    {
> -
>        HRESULT hr = X11DRV_XDND_SendDropFiles( hWnd );
> -
>        HRESULT hr = X11DRV_XDND_SendDropFiles(hwnd_drop);
>        if (SUCCEEDED(hr))
>        {
>            accept = 1;
>
> -- 
> 2.21.0

[John Found]

Ah, yes - attached is the test application I am using for the tests. It is interesting
whether such behavior is only in Windows 10 (where I tested) or on older versions as well.

If someone has an access to older windows and want to try - check what is displayed in the 
indicator label on dropping files on the popup window.

Sorry for the strange sources of the test application. :)

On Tue, 23 Apr 2019 14:21:56 +0300
John Found  wrote:
> After some more tests with a real Windows it turned, 
> that using GetAncestor() instead of GetParent() is a mistake.
>
> In fact, the popup window, owned by a window with WS_EX_ACCEPTFILES also becomes a drop target and the 
> WM_DROPFILES is sent to the **owner** window. This way the proper solution is to use exactly GetParent 
> that returns the owner window (only for the popup windows) as well.
>
> So, patch v3 is to be released soon.
>
> On Sat, 20 Apr 2019 10:20:27 +0300
> "John Found"  wrote:
> > Signed-off-by: John Found
> >
> > dlls/winex11.drv/xdnd.c | 50 ++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 42 insertions(+), 8 deletions(-)
> >
> > diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c
> > index 2ab28e43bf..ac2fc644dc 100644
> > --- a/dlls/winex11.drv/xdnd.c
> > +++ b/dlls/winex11.drv/xdnd.c
> > @@ -254,6 +254,34 @@ void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event )
> >          XFree(xdndtypes);
> >  }
> >
> > +/* Recursively searches for a window on given coordinates in a drag&drop specific manner.
> > -
> > -
> > -
> > - Don't use WindowFromPoint instead, because it omits the STATIC and transparent
> > -
> > - windows, but they can be a valid drop targets if have WS_EX_ACCEPTFILES set.
> > - */
> >
> > +static HWND window_from_point_dnd(HWND hwnd, POINT point)
> > +{
> > - HWND child;
> > - ScreenToClient(hwnd, &point);
> > - while ((child = ChildWindowFromPointEx(hwnd, point, CWP_SKIPDISABLED | CWP_SKIPINVISIBLE)) && child != hwnd)
> > - {
> > -
> >   MapWindowPoints(hwnd, child, &point, 1);
> > -
> >   hwnd = child;
> > - }
> > -
> > - return hwnd;
> >
> > +}
> > -
> >
> > +/* Returns the first window down the hierarchy that has WS_EX_ACCEPTFILES set or
> > -
> > - returns NULL, if such window does not exists.
> > - */
> >
> > +static HWND window_accepting_files(HWND hwnd)
> > +{
> > - while (hwnd && !(GetWindowLongW(hwnd, GWL_EXSTYLE) & WS_EX_ACCEPTFILES))
> > -
> >    hwnd = GetAncestor(hwnd, GA_PARENT);
> > - return hwnd;
> >
> > +}
> > -
> >
> > /**************************************************************************
> > - X11DRV_XDND_PositionEvent
> > -
> >
> > @@ -270,7 +298,7 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )
> >      HRESULT hr;
> >  XDNDxy = root_to_virtual_screen( event->data.l[2] >> 16, event->data.l[2] & 0xFFFF );
> > - targetWindow = WindowFromPoint(XDNDxy);
> > -
> >
> > targetWindow = window_from_point_dnd(hWnd, XDNDxy);
> >
> > pointl.x = XDNDxy.x;
> > pointl.y = XDNDxy.y;
> >
> > @@ -334,11 +362,15 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event )
> >  if (XDNDAccepted)
> >      accept = 1;
> > - else if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
> > -
> >        X11DRV_XDND_HasHDROP())
> > - else
> > {
> > -
> >    accept = 1;
> > -
> >    effect = DROPEFFECT_COPY;
> > -
> >    /* fallback search for window able to accept these files. */
> > -
> > -
> >    if (window_accepting_files(targetWindow) && X11DRV_XDND_HasHDROP())
> > -
> >    {
> > -
> >        accept = 1;
> > -
> >        effect = DROPEFFECT_COPY;
> > -
> >    }
> >
> > }
> >
> > TRACE("actionRequested(%ld) accept(%d) chosen(0x%x) at x(%d),y(%d)\n",
> >
> > @@ -423,10 +455,12 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event )
> >      {
> >          /* Only send WM_DROPFILES if Drop didn't succeed or DROPEFFECT_NONE was set.
> >           * Doing both causes winamp to duplicate the dropped files (#29081) */
> > -
> >    if ((GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) &&
> > -
> >            X11DRV_XDND_HasHDROP())
> > -
> > -
> >    HWND hwnd_drop = window_accepting_files(window_from_point_dnd(hWnd, XDNDxy));
> > -
> > -
> >    if (hwnd_drop && X11DRV_XDND_HasHDROP())
> >    {
> > -
> >        HRESULT hr = X11DRV_XDND_SendDropFiles( hWnd );
> > -
> >        HRESULT hr = X11DRV_XDND_SendDropFiles(hwnd_drop);
> >        if (SUCCEEDED(hr))
> >        {
> >            accept = 1;
> >
> > -- 
> > 2.21.0
>
> -- 
> John Found