[PATCH v11 0/2] MR8258: Draft: comctl32/treeview: Return from TREEVIEW_LButtonDown when the treeview handle is invalid.
comctl32/treeview: Return from TREEVIEW_LButtonDown when the treeview handle is invalid.
LButtonDown should return when the treeview handle is invalid (e.g. destroyed) after NM_CLICK to prevent further message processing.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58211
-- v11: comctl32/treeview: Return from TREEVIEW_LButtonDown when the treeview handle is invalid. comctl32/tests: Add a test for treeview deletion during NM_CLICK in LBUTTONDOWN.
From: Jacob Czekalla jczekalla@codeweavers.com
--- dlls/comctl32/tests/treeview.c | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index 5b53a32f51b..ae445bdf9ae 100644 --- a/dlls/comctl32/tests/treeview.c +++ b/dlls/comctl32/tests/treeview.c @@ -52,6 +52,7 @@ static char *g_endedit_overwrite_ptr; static HFONT g_customdraw_font; static BOOL g_v6; static int g_reject_tvn_itemexpanding = 0; +static int g_click_delete_test = 0;
#define NUM_MSG_SEQUENCES 3 #define TREEVIEW_SEQ_INDEX 0 @@ -243,6 +244,20 @@ static const struct message test_right_click_seq[] = { { 0 } };
+static const struct message test_click_delete_seq[] = { + { WM_LBUTTONDOWN, sent|wparam, MK_LBUTTON }, + { WM_CAPTURECHANGED, sent|defwinproc }, + { 0x90, sent|defwinproc|optional }, + { WM_SHOWWINDOW, sent|defwinproc|wparam|lparam, 0, 0 }, + { WM_WINDOWPOSCHANGING, sent|defwinproc }, + { WM_WINDOWPOSCHANGED, sent|defwinproc }, + { WM_KILLFOCUS, sent|defwinproc }, + { WM_IME_SETCONTEXT, sent|defwinproc|wparam, 0 }, + { WM_DESTROY, sent|defwinproc }, + { WM_NCDESTROY, sent|defwinproc }, + { 0 } +}; + static const struct message parent_expand_seq[] = { { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDINGA }, { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDEDA }, @@ -1478,6 +1493,15 @@ static LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam, ok(selected == hChild, "child item should still be selected\n"); break; } + case NM_CLICK: + { + if (g_click_delete_test) + { + DestroyWindow(pHdr->hwndFrom); + return FALSE; + } + break; + } } } break; @@ -3123,6 +3147,42 @@ static void test_right_click(void) DestroyWindow(hTree); }
+static void test_treeview_delete_midclick(void) +{ + HWND treeview; + RECT rc; + POINT pt, orig_pos; + + g_click_delete_test = 1; + treeview = create_treeview_control(0); + fill_tree(treeview); + ShowWindow(hMainWnd, SW_SHOW); + UpdateWindow(hMainWnd); + + *(HTREEITEM *)&rc = hRoot; + SendMessageA(treeview, TVM_GETITEMRECT, TRUE, (LPARAM)&rc); + + pt.x = (rc.left + rc.right) / 2; + pt.y = (rc.top + rc.bottom) / 2; + ClientToScreen(hMainWnd, &pt); + GetCursorPos(&orig_pos); + SetCursorPos(pt.x, pt.y); + ScreenToClient(treeview, &pt); + + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + PostMessageA(treeview, WM_LBUTTONDOWN, MK_LBUTTON, MAKELPARAM(pt.x, pt.y)); + PostMessageA(treeview, WM_LBUTTONUP, 0, MAKELPARAM(pt.x, pt.y)); + + flush_events(); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, test_click_delete_seq, "treeview click and destroy sequence", TRUE); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SetCursorPos(orig_pos.x, orig_pos.y); + g_click_delete_test = 0; +} + static void init_functions(void) { HMODULE hComCtl32 = LoadLibraryA("comctl32.dll"); @@ -3206,6 +3266,7 @@ START_TEST(treeview) test_TVS_FULLROWSELECT(); test_TVM_SORTCHILDREN(); test_right_click(); + test_treeview_delete_midclick();
if (!load_v6_module(&ctx_cookie, &hCtx)) { @@ -3241,6 +3302,7 @@ START_TEST(treeview) test_WM_KEYDOWN(); test_TVS_FULLROWSELECT(); test_TVM_SORTCHILDREN(); + test_treeview_delete_midclick();
unload_v6_module(ctx_cookie, hCtx); }
From: Jacob Czekalla jczekalla@codeweavers.com
LButtonDown should return when the treeview handle is invalid (e.g. destroyed) after NM_CLICK to prevent further message processing.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58211 --- dlls/comctl32/tests/treeview.c | 2 +- dlls/comctl32/treeview.c | 51 ++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index ae445bdf9ae..790f3300807 100644 --- a/dlls/comctl32/tests/treeview.c +++ b/dlls/comctl32/tests/treeview.c @@ -3176,7 +3176,7 @@ static void test_treeview_delete_midclick(void) PostMessageA(treeview, WM_LBUTTONUP, 0, MAKELPARAM(pt.x, pt.y));
flush_events(); - ok_sequence(sequences, TREEVIEW_SEQ_INDEX, test_click_delete_seq, "treeview click and destroy sequence", TRUE); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, test_click_delete_seq, "treeview click and destroy sequence", FALSE);
flush_sequences(sequences, NUM_MSG_SEQUENCES); SetCursorPos(orig_pos.x, orig_pos.y); diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c index bf9bad4715e..e353fda34af 100644 --- a/dlls/comctl32/treeview.c +++ b/dlls/comctl32/treeview.c @@ -522,11 +522,22 @@ TREEVIEW_SendRealNotify(const TREEVIEW_INFO *infoPtr, UINT code, NMHDR *hdr) return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); }
+/* + * Returns TRUE if the TREEVIEW is still valid after the notify. + * The notification result is stored in *result if non-NULL. + */ static BOOL -TREEVIEW_SendSimpleNotify(const TREEVIEW_INFO *infoPtr, UINT code) +TREEVIEW_SendSimpleNotify(const TREEVIEW_INFO *infoPtr, UINT code, BOOL *result) { NMHDR hdr; - return TREEVIEW_SendRealNotify(infoPtr, code, &hdr); + HWND hwnd; + BOOL notify_val; + + hwnd = infoPtr->hwnd; + notify_val = TREEVIEW_SendRealNotify(infoPtr, code, &hdr); + if (result) + *result = notify_val; + return IsWindow(hwnd); }
static VOID @@ -4193,6 +4204,7 @@ TREEVIEW_LButtonDoubleClick(TREEVIEW_INFO *infoPtr, LPARAM lParam) { TREEVIEW_ITEM *item; TVHITTESTINFO hit; + BOOL notify_result;
TRACE("\n"); SetFocus(infoPtr->hwnd); @@ -4211,7 +4223,9 @@ TREEVIEW_LButtonDoubleClick(TREEVIEW_INFO *infoPtr, LPARAM lParam) return 0; TRACE("item %d\n", TREEVIEW_GetItemIndex(infoPtr, item));
- if (TREEVIEW_SendSimpleNotify(infoPtr, NM_DBLCLK) == FALSE) + if (TREEVIEW_SendSimpleNotify(infoPtr, NM_DBLCLK, ¬ify_result) == FALSE) + return 0; + if (notify_result == FALSE) { /* FIXME! */ switch (hit.flags) { @@ -4258,7 +4272,7 @@ TREEVIEW_LButtonDoubleClick(TREEVIEW_INFO *infoPtr, LPARAM lParam) static LRESULT TREEVIEW_LButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) { - BOOL do_track, do_select, bDoLabelEdit; + BOOL do_track, do_select, bDoLabelEdit, notify_result; HWND hwnd = infoPtr->hwnd; TVHITTESTINFO ht;
@@ -4315,8 +4329,13 @@ TREEVIEW_LButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) (ht.flags & TVHT_ONITEMLABEL) && (infoPtr->selectedItem == ht.hItem);
/* Send NM_CLICK right away */ - if (!do_track && TREEVIEW_SendSimpleNotify(infoPtr, NM_CLICK)) - goto setfocus; + if (!do_track) + { + if (!TREEVIEW_SendSimpleNotify(infoPtr, NM_CLICK, ¬ify_result)) + return 0; + if (notify_result) + goto setfocus; + }
if (ht.flags & TVHT_ONITEMBUTTON) { @@ -4345,8 +4364,13 @@ TREEVIEW_LButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) } }
- if (do_track && TREEVIEW_SendSimpleNotify(infoPtr, NM_CLICK)) - goto setfocus; + if (do_track) + { + if (!TREEVIEW_SendSimpleNotify(infoPtr, NM_CLICK, ¬ify_result)) + return 0; + if (notify_result) + goto setfocus; + }
if (TREEVIEW_IsFullRowSelect(infoPtr)) do_select = ht.flags & (TVHT_ONITEMINDENT | TVHT_ONITEMICON | TVHT_ONITEMLABEL | TVHT_ONITEMRIGHT); @@ -4385,6 +4409,7 @@ TREEVIEW_LButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) static LRESULT TREEVIEW_RButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) { + BOOL notify_result; TVHITTESTINFO ht;
if (infoPtr->hwndEdit) @@ -4414,7 +4439,9 @@ TREEVIEW_RButtonDown(TREEVIEW_INFO *infoPtr, LPARAM lParam) else { SetFocus(infoPtr->hwnd); - if(!TREEVIEW_SendSimpleNotify(infoPtr, NM_RCLICK)) + if (!TREEVIEW_SendSimpleNotify(infoPtr, NM_RCLICK, ¬ify_result)) + return 0; + if (!notify_result) { /* Send a WM_CONTEXTMENU message in response to the RBUTTONUP */ SendMessageW(infoPtr->hwndNotify, WM_CONTEXTMENU, @@ -5340,7 +5367,7 @@ TREEVIEW_KeyDown(TREEVIEW_INFO *infoPtr, WPARAM wParam) break;
case VK_RETURN: - TREEVIEW_SendSimpleNotify(infoPtr, NM_RETURN); + TREEVIEW_SendSimpleNotify(infoPtr, NM_RETURN, NULL); break;
case VK_HOME: @@ -5666,7 +5693,7 @@ TREEVIEW_SetFocus(TREEVIEW_INFO *infoPtr) }
TREEVIEW_Invalidate(infoPtr, infoPtr->selectedItem); - TREEVIEW_SendSimpleNotify(infoPtr, NM_SETFOCUS); + TREEVIEW_SendSimpleNotify(infoPtr, NM_SETFOCUS, NULL); return 0; }
@@ -5677,7 +5704,7 @@ TREEVIEW_KillFocus(const TREEVIEW_INFO *infoPtr)
TREEVIEW_Invalidate(infoPtr, infoPtr->selectedItem); UpdateWindow(infoPtr->hwnd); - TREEVIEW_SendSimpleNotify(infoPtr, NM_KILLFOCUS); + TREEVIEW_SendSimpleNotify(infoPtr, NM_KILLFOCUS, NULL); return 0; }
This merge request was approved by Zhiyi Zhang.
The CI didn't run for the update. And the first patch introduced assertion failures, although the failures are fixed in the second patch. Please see https://testbot.winehq.org/JobDetails.pl?Key=159495&f401=task.log#k401 ``` 00f0:err:msvcrt:_wassert (L"newSelect == NULL || TREEVIEW_ValidItem(infoPtr, newSelect)",L"../dlls/comctl32/treeview.c",4515) Assertion failed: newSelect == NULL || TREEVIEW_ValidItem(infoPtr, newSelect), file ../dlls/comctl32/treeview.c, line 4515 ```
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Zhiyi Zhang (＠zhiyi)