[PATCH v3 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 -- v3: comctl32/treeview: Implemented TVN_ITEMCHANGING & TVN_ITEMCHANGED comctl32/tests/treeview: Enabled test_select() for comctl32 v6, made test for TVN_ITEMCHANGING & TVN_ITEMCHANGED https://gitlab.winehq.org/wine/wine/-/merge_requests/10451
From: Piotr Pawłowski <p@perkele.cc> --- dlls/comctl32/tests/treeview.c | 245 +++++++++++++++++++++++++++++++-- 1 file changed, 234 insertions(+), 11 deletions(-) diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index 997efa01a9d..db922fd7775 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 @@ -88,6 +89,10 @@ static void flush_events(void) } } +static const struct message blank_seq[] = { + { 0 } +}; + static const struct message FillRootSeq[] = { { TVM_INSERTITEMA, sent }, { TVM_INSERTITEMA, sent }, @@ -104,6 +109,90 @@ static const struct message rootnone_select_seq[] = { { 0 } }; +/* + * TVN_ITEMCHANGINGW and TVN_ITEMCHANGEDW intended! + * Verified against Windows versions from 7 up. + */ +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 }, + { 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_ITEMEXPANDINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDEDA }, + { 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_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 parent_rootchild_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_ITEMEXPANDINGA }, + { WM_NOTIFY, sent|id, 0, 0, TVN_ITEMEXPANDEDA }, + { 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 }, @@ -187,6 +276,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 +524,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 +559,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 +797,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,8 +825,17 @@ 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); @@ -742,7 +863,49 @@ static void test_select(void) 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); + } + + if (g_v6) + { + /* 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; + todo_wine expect(0, 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; + todo_wine expect(0, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; + expect(0, r); + + 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; + + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", TRUE); + } + DestroyWindow(hTree); + + g_skip_paint_messages = FALSE; } static void test_getitemtext(void) @@ -936,6 +1099,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 +1140,30 @@ 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, blank_seq, + "test get set item notifications", FALSE); + } + /* get item from a different tree */ hTree2 = create_treeview_control(0); @@ -997,6 +1184,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 +1460,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 +1503,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 +1528,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); @@ -3422,6 +3644,7 @@ START_TEST(treeview) g_v6 = TRUE; test_fillroot(); + test_select(); test_getitemtext(); test_get_set_insertmark(); test_get_set_item(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10451
From: Piotr Pawłowski <p@perkele.cc> --- dlls/comctl32/tests/treeview.c | 12 ++--- dlls/comctl32/treeview.c | 80 ++++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 20 deletions(-) diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c index db922fd7775..4cf9def8a12 100644 --- a/dlls/comctl32/tests/treeview.c +++ b/dlls/comctl32/tests/treeview.c @@ -830,7 +830,7 @@ static void test_select(void) 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 { @@ -865,7 +865,7 @@ static void test_select(void) 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 { @@ -883,14 +883,14 @@ static void test_select(void) r = SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (LPARAM)hChild); expect(TRUE, r); r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hChild, TVIS_SELECTED) & TVIS_SELECTED; - todo_wine expect(0, r); + expect(0, 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; - todo_wine expect(0, r); + expect(0, r); r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; expect(0, r); @@ -900,7 +900,7 @@ static void test_select(void) SendMessageA(hTree, TVM_SELECTITEM, TVGN_CARET, (WPARAM)hChild); bSelectPreviousItem = FALSE; - ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", FALSE); } DestroyWindow(hTree); @@ -1156,7 +1156,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..ded21155ca0 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,7 +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)); TRACE("Entering item %p (%s), flag 0x%x, cause 0x%x, state 0x%x\n", @@ -4596,12 +4645,15 @@ 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 +4696,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
Updated with more accurate emulation of Windows behavior. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_134351
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
} }
+static const struct message blank_seq[] = { + { 0 } +}; +
There is already an empty_seq. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135901
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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) + {
A small style nitpick. The curly braces are not necessary for one line of code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135903
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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); +
Avoid these white-space changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135902
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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;
Put `tviRoot.pszText = NULL;` on a new line. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135904
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
+ +static const struct message select_previous_item[] = { + { 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 parent_rootchild_select_seq[] = { Move `parent_rootchild_select_seq` to right after `parent_rootchild_select_seq_v6`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135905
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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)
You don't need to a space after `*` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135906
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
+ 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; + todo_wine expect(0, r); + r = SendMessageA(hTree, TVM_GETITEMSTATE, (WPARAM)hRoot, TVIS_SELECTED) & TVIS_SELECTED; + expect(0, r); + + 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; + + ok_sequence(sequences, PARENT_SEQ_INDEX, select_previous_item, "select previous item seq", TRUE); + } Let's include the tests for v5 as well. Even if the sequence is expected to be empty.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135907
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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);
Could you make these tests simpler? For example, I would expect that after getting an item to change, there is a sequence of TVN_ITEMCHANGING and TVN_ITEMCHANGED. Having a long sequence of messages after many operations is not exactly easy to understand. Keep the test as simple as possible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135908
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/treeview.c:
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:
This is a bit weird. Have you tried TVM_SETUNICODEFORMAT and see if it sends TVN_ITEMCHANGINGA? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_135909
On Sun Apr 12 10:28:05 2026 +0000, Zhiyi Zhang wrote:
This is a bit weird. Have you tried TVM_SETUNICODEFORMAT and see if it sends TVN_ITEMCHANGINGA? I tested various configurations, with TVM_SETUNICODEFORMAT too.
I can _not_ make Microsoft implementation send TVM_ITEMCHANGINGA, it always sends TVM_ITEMCHANGINGW even if created/configured as ANSI. This must have gone unnoticed because relying on common controls v6 features means your app requires at least Windows XP... then it shouldn't be built as ANSI. In fact I made the same 'bug' in my implementation at first (assumed v6 implies UNICODE), then found that MS does the same after adding tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10451#note_136067
participants (3)
-
Piotr Pawłowski -
Piotr Pawłowski (＠DEATH)
-
Zhiyi Zhang (＠zhiyi)