[PATCH v7 0/2] MR10451: comctl32/treeview: Implemented TVN_ITEMCHANGING & TVN_ITEMCHANGED
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59456 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55707 -- v7: comctl32/treeview: Implemented TVN_ITEMCHANGING and TVN_ITEMCHANGED. comctl32/tests: Add tests for TVN_ITEMCHANGING and TVN_ITEMCHANGED. https://gitlab.winehq.org/wine/wine/-/merge_requests/10451
From: Piotr Pawłowski <p@perkele.cc> --- dlls/comctl32/tests/treeview.c | 306 +++++++++++++++++++++++++++++++-- 1 file changed, 288 insertions(+), 18 deletions(-) diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index 997efa01a9d..0c17327af31 100644 --- a/dlls/comctl32/tests/treeview.c +++ b/dlls/comctl32/tests/treeview.c @@ -58,6 +58,7 @@ static HFONT g_customdraw_font; static BOOL g_v6; static int g_reject_tvn_itemexpanding = 0; static int g_click_delete_test = 0; +static BOOL g_skip_paint_messages = FALSE; #define NUM_MSG_SEQUENCES 3 #define TREEVIEW_SEQ_INDEX 0 @@ -104,12 +105,103 @@ static const struct message rootnone_select_seq[] = { { 0 } }; +/* + * TVN_ITEMCHANGINGW and TVN_ITEMCHANGEDW intended! + * TVN_ITEMCHANGINGA and TVN_ITEMCHANGEDA are never actually sent! + */ +static const struct message parent_rootnone_select_seq_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootnone_select_seq[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootchild_select_seq_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootchild_select_2changing_seq_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootchild_select_seq[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootchild_select_expand_seq_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message parent_rootchild_select_expand_seq[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 } +}; + +static const struct message select_previous_item_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 }, +}; + +static const struct message select_previous_item[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_SELCHANGEDA }, + { 0 }, +}; + static const struct message rootchild_select_seq[] = { - { TVM_SELECTITEM, sent|wparam, 9 }, - { TVM_SELECTITEM, sent|wparam, 9 }, - { TVM_SELECTITEM, sent|wparam, 9 }, - { TVM_SELECTITEM, sent|wparam, 9 }, - { TVM_SELECTITEM, sent|wparam, 9 }, { TVM_SELECTITEM, sent|wparam, 9 }, { 0 } }; @@ -187,6 +279,16 @@ static const struct message test_get_set_item_seq[] = { { TVM_SETITEMA, sent }, { TVM_GETITEMA, sent }, { TVM_SETITEMA, sent }, + { TVM_SETITEMA, sent }, + { TVM_SETITEMA, sent }, + { 0 } +}; + +static const struct message test_get_set_item_seq_parent_v6[] = { + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGINGW }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMCHANGEDW }, { 0 } }; @@ -425,7 +527,8 @@ static const struct message parent_right_click_seq[] = { static HWND hMainWnd; -static HTREEITEM hRoot, hChild; +static HTREEITEM hRoot, hChild, hBlockChange; +static BOOL bSelectPreviousItem = FALSE; static int pos = 0; static char sequence[256]; @@ -459,20 +562,29 @@ static void IdentifyItem(HTREEITEM hItem) AddItem('?'); } +static BOOL IsPaintMessage(UINT message) +{ + return message == WM_PAINT || message == WM_NCPAINT || message == WM_ERASEBKGND; +} + /* This function hooks in and records all messages to the treeview control */ static LRESULT WINAPI TreeviewWndProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) { static LONG defwndproc_counter = 0; LRESULT ret; WNDPROC lpOldProc = (WNDPROC)GetWindowLongPtrA(hwnd, GWLP_USERDATA); - struct message msg = { 0 }; - msg.message = message; - msg.flags = sent|wparam|lparam; - if (defwndproc_counter) msg.flags |= defwinproc; - msg.wParam = wParam; - msg.lParam = lParam; - add_message(sequences, TREEVIEW_SEQ_INDEX, &msg); + if (!(g_skip_paint_messages && IsPaintMessage(message))) + { + struct message msg = { 0 }; + + msg.message = message; + msg.flags = sent|wparam|lparam; + if (defwndproc_counter) msg.flags |= defwinproc; + msg.wParam = wParam; + msg.lParam = lParam; + add_message(sequences, TREEVIEW_SEQ_INDEX, &msg); + } defwndproc_counter++; ret = CallWindowProcA(lpOldProc, hwnd, message, wParam, lParam); @@ -688,6 +800,9 @@ static void test_select(void) BOOL r; HWND hTree; + /* suppress logging of painting related messages in this test */ + g_skip_paint_messages = TRUE; + hTree = create_treeview_control(0); fill_tree(hTree); @@ -713,36 +828,128 @@ static void test_select(void) expect(TRUE, r); AddItem('.'); ok(!strcmp(sequence, "1(nR)nR23(Rn)Rn45(nR)nR."), "root-none select test\n"); - ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootnone_select_seq, - "root-none select seq", FALSE); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootnone_select_seq, "root-none select seq", FALSE); + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootnone_select_seq_v6, "root-none parent select seq v6", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootnone_select_seq, "root-none parent select seq", FALSE); /* root-child select tests */ flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, 0); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); Clear(); AddItem('1'); + flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hRoot); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); + AddItem('2'); + flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hRoot); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, "root-child parent select seq", FALSE); + AddItem('3'); + flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hChild); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_expand_seq_v6, "root-child parent select seq v6", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_expand_seq, "root-child parent select seq", FALSE); + AddItem('4'); + flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hChild); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, "root-child parent select seq", FALSE); + AddItem('5'); + flush_sequences(sequences, NUM_MSG_SEQUENCES); r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hRoot); expect(TRUE, r); + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_2changing_seq_v6, "root-child parent select seq v6", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); + AddItem('.'); ok(!strcmp(sequence, "1(nR)nR23(RC)RC45(CR)CR."), "root-child select test\n"); - ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, - "root-child select seq", FALSE); DestroyWindow(hTree); + + g_skip_paint_messages = FALSE; +} + +static void test_itemchanging(void) +{ + BOOL r; + HWND hTree; + + /* suppress logging of painting related messages in this test */ + g_skip_paint_messages = TRUE; + + hTree = create_treeview_control(0); + fill_tree(hTree); + + /* Test if we can prevent item being selected; TVN_ITEMCHANGING logic only in v6 */ + r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hRoot); + expect(TRUE, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; + expect(TVIS_SELECTED, r); + hBlockChange = hChild; + r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hChild); + expect(TRUE, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hChild, TVIS_SELECTED) & TVIS_SELECTED; + if (g_v6) + todo_wine expect(0, r); + else + expect(TVIS_SELECTED, r); + + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; + expect(0, r); + hBlockChange = NULL; + r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hChild); + expect(TRUE, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hChild, TVIS_SELECTED) & TVIS_SELECTED; + if (g_v6) + todo_wine expect(0, r); + else + expect(TVIS_SELECTED, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; + expect(0, r); + + /* Test altering selection from TVN_ITEMCHANGING */ + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (WPARAM)hRoot); + bSelectPreviousItem = TRUE; + SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (WPARAM)hChild); + bSelectPreviousItem = FALSE; + + if (g_v6) + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item_v6, "select previous item seq", TRUE); + else + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", FALSE); + + DestroyWindow(hTree); + + g_skip_paint_messages = FALSE; } static void test_getitemtext(void) @@ -936,6 +1143,8 @@ static void test_get_set_item(void) HWND hTree, hTree2; DWORD ret; + g_skip_paint_messages = TRUE; + hTree = create_treeview_control(0); fill_tree(hTree); @@ -975,8 +1184,31 @@ static void test_get_set_item(void) ret = SendMessageA(hTree, TVM_SETITEMA, 0, (LPARAM)&tviRoot); expect(TRUE, ret); + tviRoot.mask = TVIF_STATE; + tviRoot.stateMask = TVIS_SELECTED; + tviRoot.state = TVIS_SELECTED; + tviRoot.cchTextMax = 0; + tviRoot.pszText = NULL; + ret = SendMessageA(hTree, TVM_SETITEMA, 0, (LPARAM)&tviRoot); + expect(TRUE, ret); + tviRoot.state = 0; + ret = SendMessageA(hTree, TVM_SETITEMA, 0, (LPARAM)&tviRoot); + expect(TRUE, ret); + + ok_sequence(sequences, TREEVIEW_SEQ_INDEX, test_get_set_item_seq, "test get set item", FALSE); + if (g_v6) + { + ok_sequence(sequences, PARENT_SEQ_INDEX, test_get_set_item_seq_parent_v6, + "test get set item notifications v6", TRUE); + } + else + { + ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, + "test get set item notifications", FALSE); + } + /* get item from a different tree */ hTree2 = create_treeview_control(0); @@ -997,6 +1229,8 @@ static void test_get_set_item(void) DestroyWindow(hTree); DestroyWindow(hTree2); + + g_skip_paint_messages = FALSE; } static void test_get_set_itemheight(void) @@ -1271,6 +1505,25 @@ static void test_get_set_unicodeformat(void) DestroyWindow(hTree); } +static BOOL IsParentPaintMessage(UINT message, WPARAM wParam, LPARAM lParam) +{ + if (message >= WM_CTLCOLORMSGBOX && message <= WM_CTLCOLORSTATIC) return TRUE; + else if (message == WM_NOTIFY) + { + return ((LPNMHDR)lParam)->code == NM_CUSTOMDRAW; + } + else return FALSE; +} + +static void HandleSelectPreviousItem(NMTREEVIEWA *nmtv) +{ + TVITEMA item = {0}; + item.mask = TVIF_STATE; + item.hItem = nmtv->itemOld.hItem; + item.stateMask = TVIS_SELECTED; + SendMessageA(nmtv->hdr.hwndFrom, TVM_SETITEMA, 0, (LPARAM)&item); +} + static LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { static LONG defwndproc_counter = 0; @@ -1295,7 +1548,9 @@ static LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam, message != WM_NCHITTEST && message != WM_GETTEXT && message != WM_GETICON && - message != WM_DEVICECHANGE) + message != WM_DEVICECHANGE && + !(g_skip_paint_messages && IsParentPaintMessage(message, wParam, lParam)) + ) { add_message(sequences, PARENT_SEQ_INDEX, &msg); } @@ -1318,7 +1573,19 @@ static LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam, NMTREEVIEWA *pTreeView = (LPNMTREEVIEWA) lParam; switch(pHdr->code) { + /* + * TVN_ITEMCHANGINGW (not TVN_ITEMCHANGINGA) intended. + * MS implementation appears to send TVN_ITEMCHANGINGW only. + * Available only in comctl32 v6. + */ + case TVN_ITEMCHANGINGW: + { + NMTVITEMCHANGE * pChange = (NMTVITEMCHANGE*) lParam; + if (pChange->hItem == hBlockChange) return TRUE; + } + break; case TVN_SELCHANGINGA: + if (bSelectPreviousItem) HandleSelectPreviousItem(pTreeView); AddItem('('); IdentifyItem(pTreeView->itemOld.hItem); IdentifyItem(pTreeView->itemNew.hItem); @@ -3411,6 +3678,7 @@ START_TEST(treeview) test_TVM_SORTCHILDREN(); test_right_click(); test_treeview_delete_midclick(); + test_itemchanging(); if (!load_v6_module(&ctx_cookie, &hCtx)) { @@ -3422,6 +3690,7 @@ START_TEST(treeview) g_v6 = TRUE; test_fillroot(); + test_select(); test_getitemtext(); test_get_set_insertmark(); test_get_set_item(); @@ -3447,6 +3716,7 @@ START_TEST(treeview) test_TVS_FULLROWSELECT(); test_TVM_SORTCHILDREN(); test_treeview_delete_midclick(); + test_itemchanging(); unload_v6_module(ctx_cookie, hCtx); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10451
From: Piotr Pawłowski <p@perkele.cc> --- dlls/comctl32/tests/treeview.c | 18 ++++---- dlls/comctl32/treeview.c | 83 ++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 23 deletions(-) diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index 0c17327af31..1a5f8c48eb1 100644 --- a/dlls/comctl32/tests/treeview.c +++ b/dlls/comctl32/tests/treeview.c @@ -830,7 +830,7 @@ static void test_select(void) ok(!strcmp(sequence, "1(nR)nR23(Rn)Rn45(nR)nR."), "root-none select test\n"); ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootnone_select_seq, "root-none select seq", FALSE); if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootnone_select_seq_v6, "root-none parent select seq v6", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootnone_select_seq_v6, "root-none parent select seq v6", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootnone_select_seq, "root-none parent select seq", FALSE); @@ -840,7 +840,7 @@ static void test_select(void) expect(TRUE, r); ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); @@ -851,7 +851,7 @@ static void test_select(void) expect(TRUE, r); ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq_v6, "root-child parent select seq v6", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); @@ -868,7 +868,7 @@ static void test_select(void) expect(TRUE, r); ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_expand_seq_v6, "root-child parent select seq v6", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_expand_seq_v6, "root-child parent select seq v6", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_expand_seq, "root-child parent select seq", FALSE); @@ -885,7 +885,7 @@ static void test_select(void) expect(TRUE, r); ok_sequence(sequences, TREEVIEW_SEQ_INDEX, rootchild_select_seq, "root-child select seq", FALSE); if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_2changing_seq_v6, "root-child parent select seq v6", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_2changing_seq_v6, "root-child parent select seq v6", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, parent_rootchild_select_seq, "root-child parent select seq", FALSE); @@ -918,7 +918,7 @@ static void test_itemchanging(void) expect(TRUE, r); r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hChild, TVIS_SELECTED) & TVIS_SELECTED; if (g_v6) - todo_wine expect(0, r); + expect(0, r); else expect(TVIS_SELECTED, r); @@ -929,7 +929,7 @@ static void test_itemchanging(void) expect(TRUE, r); r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hChild, TVIS_SELECTED) & TVIS_SELECTED; if (g_v6) - todo_wine expect(0, r); + expect(0, r); else expect(TVIS_SELECTED, r); r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; @@ -943,7 +943,7 @@ static void test_itemchanging(void) bSelectPreviousItem = FALSE; if (g_v6) - ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item_v6, "select previous item seq", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item_v6, "select previous item seq", FALSE); else ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", FALSE); @@ -1201,7 +1201,7 @@ static void test_get_set_item(void) if (g_v6) { ok_sequence(sequences, PARENT_SEQ_INDEX, test_get_set_item_seq_parent_v6, - "test get set item notifications v6", TRUE); + "test get set item notifications v6", FALSE); } else { diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c index 6332040b7f5..f1eec5f0dcb 100644 --- a/dlls/comctl32/treeview.c +++ b/dlls/comctl32/treeview.c @@ -517,6 +517,55 @@ TREEVIEW_SendRealNotify(const TREEVIEW_INFO *infoPtr, UINT code, NMHDR *hdr) return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); } +static BOOL TREEVIEW_SendItemChanging(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item, UINT uChanged, UINT uStateOld, UINT uStateNew) +{ +#if __WINE_COMCTL32_VERSION == 6 + NMTVITEMCHANGE change; + change.uChanged = uChanged; + change.hItem = item; + change.uStateOld = uStateOld; + change.uStateNew = uStateNew; + change.lParam = item->lParam; + return TREEVIEW_SendRealNotify(infoPtr, TVN_ITEMCHANGINGW, &change.hdr); +#endif + return FALSE; +} + +static BOOL TREEVIEW_SendItemChanged(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item, UINT uChanged, UINT uStateOld, UINT uStateNew) +{ +#if __WINE_COMCTL32_VERSION == 6 + NMTVITEMCHANGE change; + change.uChanged = uChanged; + change.hItem = item; + change.uStateOld = uStateOld; + change.uStateNew = uStateNew; + change.lParam = item->lParam; + return TREEVIEW_SendRealNotify(infoPtr, TVN_ITEMCHANGEDW, &change.hdr); +#endif + return FALSE; +} + +/* + * Alter state helper + * Returns a boolean value indicating whether the change was accepted (always TRUE for legacy commoncontrols) + */ +static BOOL TREEVIEW_AlterItemState(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM * item, UINT stateNew) +{ + UINT state = item->state; + if (state != stateNew) + { + if (TREEVIEW_SendItemChanging(infoPtr, item, TVIF_STATE, state, stateNew)) return FALSE; + item->state = stateNew; + if (TREEVIEW_SendItemChanged(infoPtr, item, TVIF_STATE, state, stateNew)) + { + /* roll back */ + item->state = state; + return FALSE; + } + } + return TRUE; +} + /* * Returns TRUE if the TREEVIEW is still valid after the notify. * The notification result is stored in *result if non-NULL. @@ -1212,10 +1261,8 @@ TREEVIEW_DoSetItemT(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item, if (tvItem->mask & TVIF_STATE) { - TRACE("prevstate 0x%x, state 0x%x, mask 0x%x\n", item->state, tvItem->state, - tvItem->stateMask); - item->state &= ~tvItem->stateMask; - item->state |= (tvItem->state & tvItem->stateMask); + UINT stateNew = (item->state & ~tvItem->stateMask) | (tvItem->state & tvItem->stateMask); + TREEVIEW_AlterItemState(infoPtr, item, stateNew); } if (tvItem->mask & TVIF_STATEEX) @@ -2338,7 +2385,7 @@ TREEVIEW_GetCount(const TREEVIEW_INFO *infoPtr) } static VOID -TREEVIEW_ToggleItemState(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item) +TREEVIEW_ToggleItemState(TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item) { if (infoPtr->dwStyle & TVS_CHECKBOXES) { @@ -2364,8 +2411,10 @@ TREEVIEW_ToggleItemState(const TREEVIEW_INFO *infoPtr, TREEVIEW_ITEM *item) TRACE("stateImage: 0x%x\n", stateImage); - item->state = state; - TREEVIEW_Invalidate(infoPtr, item); + if (state != item->state && TREEVIEW_AlterItemState(infoPtr, item, state)) + { + TREEVIEW_Invalidate(infoPtr, item); + } } } @@ -4565,6 +4614,7 @@ TREEVIEW_DoSelectItem(TREEVIEW_INFO *infoPtr, INT action, HTREEITEM newSelect, INT cause) { TREEVIEW_ITEM *prevSelect; + LRESULT ret = TRUE; assert(newSelect == NULL || TREEVIEW_ValidItem(infoPtr, newSelect)); @@ -4585,7 +4635,8 @@ TREEVIEW_DoSelectItem(TREEVIEW_INFO *infoPtr, INT action, HTREEITEM newSelect, case TVGN_CARET: prevSelect = infoPtr->selectedItem; - if (prevSelect == newSelect) { + if (prevSelect == newSelect) + { TREEVIEW_EnsureVisible(infoPtr, infoPtr->selectedItem, FALSE); break; } @@ -4596,12 +4647,16 @@ TREEVIEW_DoSelectItem(TREEVIEW_INFO *infoPtr, INT action, HTREEITEM newSelect, TVIF_TEXT | TVIF_HANDLE | TVIF_STATE | TVIF_PARAM, prevSelect, newSelect)) - return FALSE; + { + ret = FALSE; + break; + } - if (prevSelect) - prevSelect->state &= ~TVIS_SELECTED; - if (newSelect) - newSelect->state |= TVIS_SELECTED; + if (prevSelect == NULL || TREEVIEW_AlterItemState( infoPtr, prevSelect, prevSelect->state & ~TVIS_SELECTED)) + { + /* deselected previous */ + if (newSelect != NULL) TREEVIEW_AlterItemState( infoPtr, newSelect, newSelect->state | TVIS_SELECTED); + } infoPtr->selectedItem = newSelect; @@ -4644,7 +4699,7 @@ TREEVIEW_DoSelectItem(TREEVIEW_INFO *infoPtr, INT action, HTREEITEM newSelect, } TRACE("Leaving state 0x%x\n", newSelect ? newSelect->state : 0); - return TRUE; + return ret; } /* FIXME: handle NM_KILLFOCUS etc */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10451
On Sun May 3 21:03:49 2026 +0000, Zhiyi Zhang wrote:
Let's rephrase the commit subject as "comctl32/tests: Add tests for TVN_ITEMCHANGING and TVN_ITEMCHANGED.". OK
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_138709
On Sun May 3 21:03:58 2026 +0000, Zhiyi Zhang wrote:
Keep an empty line between variable declaration and the following code. OK
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_138710
On Sun May 3 21:02:12 2026 +0000, Piotr Pawłowski wrote:
changed this line in [version 7 of the diff](/wine/wine/-/merge_requests/10451/diffs?diff_id=265083&start_sha=e1550311969ecd0bff18ef45c12cc7e201d1e396#ea6661805bd26d4797d9aebb9c2d0e45a9d9acc7_4649_4650) OK
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_138711
On Tue Apr 28 03:33:34 2026 +0000, Zhiyi Zhang wrote:
I don't like that the TVN_ITEMCHANGING and TVN_ITEMCHANGED message tests are mixing with other tests. Please separate them into a new test function. For example, if selecting or setting one item triggers TVN_ITEMCHANGING and TVN_ITEMCHANGED, then you test specifically that, not adding the tests where there are multiple unnecessary child items. Moved new tests specifically testing TVN_ITEMCHANGING functionality to a new function.
The rest should be good as it is? I added parent message sequences to various existing tests instead of inventing new, I don't think there's any point in duplicating them with/without parent message sequences? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_138712
On Sun May 3 21:07:27 2026 +0000, Piotr Pawłowski wrote:
Moved new tests specifically testing TVN_ITEMCHANGING functionality to a new function. The rest should be good as it is? I added parent message sequences to various existing tests instead of inventing new, I don't think there's any point in duplicating them with/without parent message sequences? The tests for TVN_ITEMCHANGING and TVN_ITEMCHANGED are still mixed with other tests. So you have things like parent_rootnone_select_seq_v6 that include multiple pairs of TVN_ITEMCHANGING and TVN_ITEMCHANGED. The ideal way is to test TVN_ITEMCHANGING and TVN_ITEMCHANGED specifically. For example, when there is only one child item, there will be only **one pair** of TVN_ITEMCHANGING and TVN_ITEMCHANGED. This way, the tests would be much cleaner and clearly separated from existing tests.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_139401
On Mon May 11 09:56:08 2026 +0000, Zhiyi Zhang wrote:
The tests for TVN_ITEMCHANGING and TVN_ITEMCHANGED are still mixed with other tests. So you have things like parent_rootnone_select_seq_v6 that include multiple pairs of TVN_ITEMCHANGING and TVN_ITEMCHANGED. The ideal way is to test TVN_ITEMCHANGING and TVN_ITEMCHANGED specifically. For example, when there is only one child item, there will be only **one pair** of TVN_ITEMCHANGING and TVN_ITEMCHANGED. This way, the tests would be much cleaner and clearly separated from existing tests. Various treeview operations send TVN_ITEMCHANGING+TVM_ITEMCHANGED to parent in v6 mode but omit them in v5. Various tests must simply use different parent message sequences in v6 mode.
Additionally, test_select() wasn't done at all for v6 prior to my changes - presumably because it was broken (all recorded message sequences were valid for v5 but not v6). If this is what you want, I'll discard all changes I made to test_select() & test_get_set_item() - also drop test_select() from v6 tests. But I don't really think this is a good idea. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_139602
If this is what you want, I'll discard all changes I made to test_select() & test_get_set_item() - also drop test_select() from v6 tests. But I don't really think this is a good idea.
Yes, I think it should be better. This way, we can focus on TVN_ITEMCHANGING and TVM_ITEMCHANGED. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_139627
participants (3)
-
Piotr Pawłowski -
Piotr Pawłowski (＠DEATH)
-
Zhiyi Zhang (＠zhiyi)