[PATCH v2 0/2] MR10451: comctl32/treeview: Implemented TVN_ITEMCHANGING

[Piotr Pawłowski (＠DEATH)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59456

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55707

--
  v2: comctl32/treeview: Implemented TVN_ITEMCHANGING & TVN_ITEMCHANGED
      comctl32/tests: Enabled test_select() for comctl32 v6, made test for TVN_ITEMCHANGING & TVN_ITEMCHANGED
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10451

[PATCH v2 1/2] comctl32/tests: Enabled test_select() for comctl32 v6, made test for TVN_ITEMCHANGING & TVN_ITEMCHANGED

[Piotr Pawłowski]

From: Piotr Pawłowski <[e-mail]>

---
 dlls/comctl32/tests/treeview.c | 173 ++++++++++++++++++++++++++++++---
 1 file changed, 162 insertions(+), 11 deletions(-)

diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c
index 997efa01a9d..da8568af94c 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,6 +105,76 @@ 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 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 },
@@ -425,7 +496,7 @@ static const struct message parent_right_click_seq[] = {
 
 static HWND hMainWnd;
 
-static HTREEITEM hRoot, hChild;
+static HTREEITEM hRoot, hChild, hBlockChange;
 
 static int pos = 0;
 static char sequence[256];
@@ -459,20 +530,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 +768,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 +796,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 +834,42 @@ 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);
+    }
+    
+    
     DestroyWindow(hTree);
+
+    g_skip_paint_messages = FALSE;
 }
 
 static void test_getitemtext(void)
@@ -1271,6 +1398,16 @@ 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 LRESULT CALLBACK parent_wnd_proc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
     static LONG defwndproc_counter = 0;
@@ -1295,7 +1432,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,6 +1457,17 @@ 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:
                 AddItem('(');
                 IdentifyItem(pTreeView->itemOld.hItem);
@@ -3422,6 +3572,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

[PATCH v2 2/2] 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

[Piotr Pawłowski]

From: Piotr Pawłowski <[e-mail]>

---
 dlls/comctl32/tests/treeview.c |  8 ++---
 dlls/comctl32/treeview.c       | 66 ++++++++++++++++++++++++++++++----
 2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c
index da8568af94c..9662509c443 100644
--- a/dlls/comctl32/tests/treeview.c
+++ b/dlls/comctl32/tests/treeview.c
@@ -801,7 +801,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
     {
@@ -836,7 +836,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
     {
@@ -854,14 +854,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);
     }
diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c
index 6332040b7f5..7c95d4d8dcd 100644
--- a/dlls/comctl32/treeview.c
+++ b/dlls/comctl32/treeview.c
@@ -517,6 +517,57 @@ 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);
+#else
+    return FALSE;
+#endif
+}
+
+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);
+#else
+    return FALSE;
+#endif
+}
+
+/*
+ * 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.
@@ -2364,8 +2415,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);
+        }
     }
 }
 
@@ -4598,10 +4651,11 @@ TREEVIEW_DoSelectItem(TREEVIEW_INFO *infoPtr, INT action, HTREEITEM newSelect,
 					newSelect))
 	    return FALSE;
 
-	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;
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10451