On Fri, 19 Apr 2019 15:43:46 -0500 Zebediah Figura z.figura12@gmail.com wrote:
On 4/19/19 3:28 AM, John Found wrote:
On Thu, 18 Apr 2019 12:53:35 -0500 Zebediah Figura z.figura12@gmail.com wrote:
On 04/18/2019 12:35 PM, John Found wrote:
On Thu, 18 Apr 2019 12:09:02 -0500 Zebediah Figura z.figura12@gmail.com wrote:
Really? I'm looking at this test right here:
https://source.winehq.org/git/wine.git/blob/refs/heads/master:/dlls/user32/tests/win.c#l9398
Well, yes, I am wrong about the WindowFromPoint, but still only the first level of the children is checked.
The mentioned test checks exactly this in 3 cases:
- Main window without child created - main window returned.
- Main window with single STATIC child created - main window returned. (correct, according to MSDN)
- Main window with single BUTTON child created - the child returned.
There is no child-in-child cases (and IMHO, should not be).
Really? Why not? Our implementation is recursive, so it should return the deepest child. If it's wrong, we should add tests to prove it.
Well, I finished my extended tests and they are contradictory.
- Yes, WindowFromPoint searches recursively on random depth, both in Windows and WINE.
I was totally wrong about this. Excuse my ignorance.
- But WindowFromPoint still does not work properly in my drag&drop patch. The problem is that WindowFromPoint
fully ignores the STATIC controls (documented and correct behavior), but in Windows the STATIC controls are valid drop target if they have WS_EX_ACCEPTFILES set.
This way, the implemented in the patch "window_from_point_dnd" function correctly returns the STATIC windows drop targets. Comparing with the original Windows behavior, I can't see any differences.
(I will attach the improved test application I used, now it has STATIC as a drop targets and WindowFromPoint test tool as well.)
So, the question is what to do now?
Regards.
Thanks for testing, that makes sense. In that case it seems your original approach is necessary. I would recommend adding a comment to clarify this, so that someone doesn't make the mistake of trying to replace it with WindowFromPoint() in the future.
I also have some stylistic comments on your original patch:
+/* the recursive worker for window_from_point_dnd */ +HWND do_window_from_point_dnd(HWND hwnd, POINT* point) +{
- HWND w;
- w = ChildWindowFromPointEx(hwnd, *point, CWP_SKIPDISABLED | CWP_SKIPINVISIBLE);
- if (w && (w != hwnd))
- {
MapWindowPoints(hwnd, w, point, 1);w = do_window_from_point_dnd(w, point);- }
- return w;
+}
+/* Recursively search for the window on given coordinates in a drag&drop specific manner. */ +HWND window_from_point_dnd(HWND hwnd, POINT point) +{
- POINT p;
- p.x = point.x;
- p.y = point.y;
- ScreenToClient(hwnd, &p);
- return do_window_from_point_dnd(hwnd, &p);
+}
These functions should be static, since they're not used elsewhere.
I also feel like this would more readable formatted as a loop instead of a recursive function. Something like:
/* Find the deepest child window at the given point. We can't use WindowFromPoint() for this, because it skips HTTRANSPARENT windows, and those should still receive drop notifications. */ static HWND window_from_point( 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; }
while (targetWindow && !(GetWindowLongW(targetWindow, GWL_EXSTYLE) & WS_EX_ACCEPTFILES))targetWindow = GetParent(targetWindow);...
while (hwnd_drop && !(GetWindowLongW(hwnd_drop, GWL_EXSTYLE) & WS_EX_ACCEPTFILES))hwnd_drop = GetParent(hwnd_drop);Since you do this in both places, you could also factor it into the window_from_point helper, in which case it might be better to call it something like get_drop_target().
POINT pt;HWND hwnd_drop;pt.x = XDNDxy.x;pt.y = XDNDxy.y;hwnd_drop = window_from_point_dnd(hWnd, pt);You're passing pt by value, so you don't need to copy it into a local variable.
Thanks for the hints. My C skills are really not the best of my skills. Will fix it and submit as a v2.
BTW, what you think about the notice of Gabriel Ivăncescu in his post above, about the difference between GetParent (returns the owner as well) and GetAncestor(...,GA_PARENT) (returns only the parent)?