[PATCH] comctl32: LVM_INSERTITEM handler should send the LVN_ITEMCHANGING/LVN_ITEMCHANGED notifications.

[Dmitry Timoshkov]

Signed-off-by: Dmitry Timoshkov 
---
 dlls/comctl32/listview.c       |  9 +++------
 dlls/comctl32/tests/listview.c | 22 +++++++++++++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c
index 9e35f7879b7..414bd686f27 100644
--- a/dlls/comctl32/listview.c
+++ b/dlls/comctl32/listview.c
@@ -4259,11 +4259,11 @@ static BOOL set_main_item(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL
     nmlv.uChanged = uChanged ? uChanged : lpLVItem->mask;
     nmlv.lParam = item.lParam;

-    /* Send LVN_ITEMCHANGING notification, if the item is not being inserted
-       and we are _NOT_ virtual (LVS_OWNERDATA), and change notifications
+    /* Send LVN_ITEMCHANGING notification:
+       if we are _NOT_ virtual (LVS_OWNERDATA), and change notifications
        are enabled. Even nothing really changed we still need to send this,
        in this case uChanged mask is just set to passed item mask. */
-    if (lpItem && !isNew && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE))
+    if (lpItem && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE))
     {
       HWND hwndSelf = infoPtr->hwndSelf;

@@ -4351,9 +4351,6 @@ static BOOL set_main_item(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL
    }
     }

-    /* if we're inserting the item, we're done */
-    if (isNew) return TRUE;
-
     /* send LVN_ITEMCHANGED notification */
     if (lpLVItem->mask & LVIF_PARAM) nmlv.lParam = lpLVItem->lParam;
     if (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)
diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c
index e95b81f5bb1..ea3a8b9bcde 100644
--- a/dlls/comctl32/tests/listview.c
+++ b/dlls/comctl32/tests/listview.c
@@ -422,6 +422,13 @@ static const struct message parent_insert_focused_seq[] = {
     { 0 }
 };

+static const struct message parent_insert_selected_seq[] = {
+    { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGING },
+    { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
+    { WM_NOTIFY, sent|id, 0, 0, LVN_INSERTITEM },
+    { 0 }
+};
+
 static const struct message parent_report_cd_seq[] = {
     { WM_NOTIFY, sent|id|custdraw, 0, 0, NM_CUSTOMDRAW, CDDS_PREPAINT },
     { WM_NOTIFY, sent|id|custdraw, 0, 0, NM_CUSTOMDRAW, CDDS_ITEMPREPAINT },
@@ -5942,7 +5949,7 @@ static void test_insertitem(void)
     ret = SendMessageA(hwnd, LVM_INSERTITEMA, 0, (LPARAM)&item);
     ok(ret == 1, "got %d\n", ret);

-    ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused_seq, "insert focused", TRUE);
+    ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused_seq, "insert focused", FALSE);

state = SendMessageA(hwnd, LVM_GETITEMSTATE, 1, LVIS_FOCUSED);
     ok(state == LVIS_FOCUSED, "got %x\n", state);
@@ -5959,6 +5966,19 @@ static void test_insertitem(void)
     state = SendMessageA(hwnd, LVM_GETITEMSTATE, 1, LVIS_FOCUSED);
     ok(state == LVIS_FOCUSED, "got %x\n", state);

+    flush_sequences(sequences, NUM_MSG_SEQUENCES);
+
+    /* insert item 3 */
+    item.mask = LVIF_STATE;
+    item.state = LVIS_SELECTED;
+    item.stateMask = LVIS_SELECTED;
+    item.iItem = 3;
+    item.iSubItem = 0;
+    ret = SendMessageA(hwnd, LVM_INSERTITEMA, 0, (LPARAM)&item);
+    ok(ret == 3, "got %d\n", ret);
+
+    ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_selected_seq, "insert selected", FALSE);
+
     DestroyWindow(hwnd);
 }

[Nikolay Sivov]

The test is not adequate for the change you're making. Just one example, 
inserting with just LVIF_PARAM set now produces incomplete 
(LVN_ITEMCHANGING, LVN_INSERTITEM), I don't see the same on Windows.

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> The test is not adequate for the change you're making. Just one example, 
> inserting with just LVIF_PARAM set now produces incomplete 
> (LVN_ITEMCHANGING, LVN_INSERTITEM), I don't see the same on Windows.

What kind of tests would be more appropriate in this case?