[PATCH v2 4/5] comctl32: LVM_INSERTITEM handler should send LVN_ITEMCHANGING/LVN_ITEMCHANGED notifications when the item state is being set.
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/comctl32/listview.c | 9 +++++---- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index cd7fec28e27..137ca7c6529 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -4260,11 +4260,12 @@ 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 + /* Send LVN_ITEMCHANGING notification: + if the item is not being inserted or its state is being set, and 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 && (!isNew || (uChanged & LVIF_STATE)) && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) { HWND hwndSelf = infoPtr->hwndSelf; @@ -4352,8 +4353,8 @@ 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; + /* if we're inserting the item and its state is not being set, we're done */ + if (isNew && !(uChanged & LVIF_STATE)) return TRUE; /* send LVN_ITEMCHANGED notification */ if (lpLVItem->mask & LVIF_PARAM) nmlv.lParam = lpLVItem->lParam; diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index a53fec81c90..4521158d141 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -6017,7 +6017,7 @@ static void test_LVM_INSERTITEM(void) if ((insert_item[i].mask & LVIF_STATE) && (insert_item[i].state & (LVIS_FOCUSED | LVIS_SELECTED))) { sprintf(buf, "%d: insert focused", i); - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused0_seq, buf, TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused0_seq, buf, insert_item[i].mask != LVIF_STATE); } else { @@ -6054,7 +6054,7 @@ static void test_insertitem(void) item.iSubItem = 0; ret = SendMessageA(hwnd, LVM_INSERTITEMA, 0, (LPARAM)&item); ok(ret == 0, "got %d\n", ret); - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused0_seq, "insert focused 0", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused0_seq, "insert focused 0", FALSE); state = SendMessageA(hwnd, LVM_GETITEMSTATE, 0, LVIS_FOCUSED); ok(state == LVIS_FOCUSED, "got %x\n", state); @@ -6067,7 +6067,7 @@ static void test_insertitem(void) item.iSubItem = 0; ret = SendMessageA(hwnd, LVM_INSERTITEMA, 0, (LPARAM)&item); ok(ret == 1, "got %d\n", ret); - ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused1_seq, "insert focused 1", TRUE); + ok_sequence(sequences, PARENT_SEQ_INDEX, parent_insert_focused1_seq, "insert focused 1", FALSE); state = SendMessageA(hwnd, LVM_GETITEMSTATE, 1, LVIS_FOCUSED); ok(state == LVIS_FOCUSED, "got %x\n", state); -- 2.34.1
On 2/11/22 15:36, Dmitry Timoshkov wrote:
- if (lpItem && !isNew && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) + if (lpItem && (!isNew || (uChanged & LVIF_STATE)) && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) { HWND hwndSelf = infoPtr->hwndSelf;
@@ -4352,8 +4353,8 @@ 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; + /* if we're inserting the item and its state is not being set, we're done */ + if (isNew && !(uChanged & LVIF_STATE)) return TRUE;
This isn't right either. On Windows, inserting with e.g. LVIF_STATE/LVIS_CUT does not produce a change notification.
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 2/11/22 15:36, Dmitry Timoshkov wrote:
- if (lpItem && !isNew && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) + if (lpItem && (!isNew || (uChanged & LVIF_STATE)) && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) { HWND hwndSelf = infoPtr->hwndSelf;
@@ -4352,8 +4353,8 @@ 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; + /* if we're inserting the item and its state is not being set, we're done */ + if (isNew && !(uChanged & LVIF_STATE)) return TRUE;
This isn't right either. On Windows, inserting with e.g. LVIF_STATE/LVIS_CUT does not produce a change notification.
Thanks for the quick review, looks like I need to add a test for this case as well. -- Dmitry.
On 2/11/22 15:54, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 2/11/22 15:36, Dmitry Timoshkov wrote:
- if (lpItem && !isNew && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) + if (lpItem && (!isNew || (uChanged & LVIF_STATE)) && (infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE)) { HWND hwndSelf = infoPtr->hwndSelf;
@@ -4352,8 +4353,8 @@ 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; + /* if we're inserting the item and its state is not being set, we're done */ + if (isNew && !(uChanged & LVIF_STATE)) return TRUE; This isn't right either. On Windows, inserting with e.g. LVIF_STATE/LVIS_CUT does not produce a change notification. Thanks for the quick review, looks like I need to add a test for this case as well.
I haven't tested properly, but I suspect it's only considered a change if it's (LVIS_SELECTED|LVIS_FOCUSED).
participants (2)
-
Dmitry Timoshkov -
Nikolay Sivov