From: Angelo Haller angelo@szanni.org
The following patches fix sending of the LVN_ODSTATECHANGED notification for LVS_OWNERDATA list views, adding more refined tests in the process and fixing various bugs.
This is v2 addressing some of the issues raised in the first patch series.
Warning: I have had access to the Windows Research Kernel (WRK) 1.2 ~10 years ago. These changes are regarding comctrl32 & tests which are NOT part of the WRK. As outlined in https://wiki.winehq.org/Developer_FAQ this should therefore satisfy the requirement of ONLY submitting patches to components I have NOT had access to.
Angelo Haller (6): comctl32/tests: Expand ownerdata listview tests. comctl32/listview: Move LVN_ODSTATECHANGED notification to function. comctl32/listview: Send LVN_ODSTATECHANGED only for virtual lists. comctl32/listview: Send LVN_ODSTATECHANGED notification. comctl32/listview: Send LVN_ODSTATECHANGED only for true ranges. comctl32/listview: Fix deselect on LVS_OWNERDATA.
dlls/comctl32/listview.c | 55 +++++++++++++++++++++---------- dlls/comctl32/tests/listview.c | 59 +++++++++++++++++++++++++++++----- 2 files changed, 89 insertions(+), 25 deletions(-)
Signed-off-by: Angelo Haller angelo@szanni.org
From: Angelo Haller angelo@szanni.org
Add more test cases to ownderdata listviews. Check LVN_ITEMCHANGED IDs.
Signed-off-by: Angelo Haller angelo@szanni.org --- dlls/comctl32/tests/listview.c | 59 +++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index 6ac7f53137d..78b3e3ae069 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -255,11 +255,33 @@ static const struct message ownerdata_deselect_all_parent_seq[] = { { 0 } };
-static const struct message ownerdata_multiselect_odstatechanged_seq[] = { - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED }, +static const struct message ownerdata_multiselect_select_0_to_1_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED }, - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED }, - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_select_0_to_2_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED }, + { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_deselect_all_select_3_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_deselect_3_select_2_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, { 0 } };
@@ -3575,8 +3597,8 @@ static void test_ownerdata_multiselect(void) expect(0, res);
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, - ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", TRUE); + ownerdata_multiselect_select_0_to_1_seq, + "ownerdata multiselect: select multiple via SHIFT", TRUE);
res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); @@ -3592,8 +3614,8 @@ static void test_ownerdata_multiselect(void) expect(0, res);
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, - ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", TRUE); + ownerdata_multiselect_select_0_to_2_seq, + "ownerdata multiselect: select multiple via SHIFT+CONTROL", TRUE);
res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); @@ -3604,6 +3626,27 @@ static void test_ownerdata_multiselect(void) res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); expect(3, res);
+ flush_sequences(sequences, NUM_MSG_SEQUENCES); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_deselect_all_select_3_seq, + "ownerdata multiselect: deselect all, select item 3", TRUE); + + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_deselect_3_select_2_seq, + "ownerdata multiselect: deselect item 3, select item 2", TRUE); + DestroyWindow(hwnd); }
From: Angelo Haller angelo@szanni.org
Signed-off-by: Angelo Haller angelo@szanni.org --- v2: Remove header and change return to VOID. --- dlls/comctl32/listview.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 730bf4aaddd..72ade724313 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -437,6 +437,7 @@ static INT LISTVIEW_GetStringWidthT(const LISTVIEW_INFO *, LPCWSTR, BOOL); static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *, INT, BOOL); static UINT LISTVIEW_GetItemState(const LISTVIEW_INFO *, INT, UINT); static BOOL LISTVIEW_SetItemState(LISTVIEW_INFO *, INT, const LVITEMW *); +static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *, INT, INT, const LVITEMW *); static LRESULT LISTVIEW_VScroll(LISTVIEW_INFO *, INT, INT); static LRESULT LISTVIEW_HScroll(LISTVIEW_INFO *, INT, INT); static BOOL LISTVIEW_EnsureVisible(LISTVIEW_INFO *, INT, BOOL); @@ -3557,16 +3558,11 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) INT nFirst = min(infoPtr->nSelectionMark, nItem); INT nLast = max(infoPtr->nSelectionMark, nItem); HWND hwndSelf = infoPtr->hwndSelf; - NMLVODSTATECHANGE nmlv; DWORD old_mask; LVITEMW item; INT i;
- /* Temporarily disable change notification - * If the control is LVS_OWNERDATA, we need to send - * only one LVN_ODSTATECHANGED notification. - * See MSDN documentation for LVN_ITEMCHANGED. - */ + /* disable per item notifications on LVS_OWNERDATA style */ old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE; @@ -3579,13 +3575,8 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) for (i = nFirst; i <= nLast; i++) LISTVIEW_SetItemState(infoPtr,i,&item);
- ZeroMemory(&nmlv, sizeof(nmlv)); - nmlv.iFrom = nFirst; - nmlv.iTo = nLast; - nmlv.uOldState = 0; - nmlv.uNewState = item.state; + LISTVIEW_SetOwnerDataState(infoPtr, nFirst, nLast, &item);
- notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv); if (!IsWindow(hwndSelf)) return FALSE; infoPtr->notify_mask |= old_mask; @@ -8950,6 +8941,22 @@ static BOOL LISTVIEW_SetItemPosition(LISTVIEW_INFO *infoPtr, INT nItem, const PO return LISTVIEW_MoveIconTo(infoPtr, nItem, &Pt, FALSE); }
+/* Make sure to also disable per item notifications via the notification mask. */ +static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT nFirst, INT nLast, const LVITEMW *item) +{ + NMLVODSTATECHANGE nmlv; + + if (!item) return; + + ZeroMemory(&nmlv, sizeof(nmlv)); + nmlv.iFrom = nFirst; + nmlv.iTo = nLast; + nmlv.uOldState = 0; + nmlv.uNewState = item->state; + + notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv); +} + /*** * DESCRIPTION: * Sets the state of one or many items.
From: Angelo Haller angelo@szanni.org
The LVN_ODSTATECHANGED notification should only be sent to lists that have LVS_OWNERDATA set.
Signed-off-by: Angelo Haller angelo@szanni.org --- dlls/comctl32/listview.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 72ade724313..318df0a4093 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -8946,6 +8946,7 @@ static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT nFirst, INT n { NMLVODSTATECHANGE nmlv;
+ if (!(infoPtr->dwStyle & LVS_OWNERDATA)) return; if (!item) return;
ZeroMemory(&nmlv, sizeof(nmlv));
From: Angelo Haller angelo@szanni.org
Send LVN_ODSTATECHANGED notification on selection change for listviews when LVS_OWNERDATA is set.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52534 Signed-off-by: Angelo Haller angelo@szanni.org --- dlls/comctl32/listview.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 318df0a4093..3c3f5af2a27 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3597,6 +3597,8 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) */ static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) { + INT nFirst = -1; + INT nLast = -1; RANGES selection; DWORD old_mask; LVITEMW item; @@ -3648,21 +3650,25 @@ static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) iterator_destroy(&i); }
- /* disable per item notifications on LVS_OWNERDATA style - FIXME: single LVN_ODSTATECHANGED should be used */ + /* disable per item notifications on LVS_OWNERDATA style */ old_mask = infoPtr->notify_mask & NOTIFY_MASK_ITEM_CHANGE; if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->notify_mask &= ~NOTIFY_MASK_ITEM_CHANGE;
LISTVIEW_DeselectAllSkipItems(infoPtr, selection);
- iterator_rangesitems(&i, selection); - while(iterator_next(&i)) - LISTVIEW_SetItemState(infoPtr, i.nItem, &item); + while(iterator_next(&i)) { + if (nFirst == -1) + nFirst = i.nItem; + nLast = i.nItem; + LISTVIEW_SetItemState(infoPtr, i.nItem, &item); + } /* this will also destroy the selection */ iterator_destroy(&i);
+ LISTVIEW_SetOwnerDataState(infoPtr, nFirst, nLast, &item); + infoPtr->notify_mask |= old_mask; LISTVIEW_SetItemFocus(infoPtr, nItem); }
From: Angelo Haller angelo@szanni.org
Signed-off-by: Angelo Haller angelo@szanni.org --- dlls/comctl32/listview.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 3c3f5af2a27..bb394974906 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -8953,6 +8953,7 @@ static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT nFirst, INT n NMLVODSTATECHANGE nmlv;
if (!(infoPtr->dwStyle & LVS_OWNERDATA)) return; + if (nFirst == nLast) return; if (!item) return;
ZeroMemory(&nmlv, sizeof(nmlv));
From: Angelo Haller angelo@szanni.org
Send one "deselect all items" notification on selection change for LVS_OWNERDATA listviews instead of notifying about each individual item change.
Enable LVS_OWNERDATA multi select tests for wine.
Signed-off-by: Angelo Haller angelo@szanni.org --- dlls/comctl32/listview.c | 6 ++++++ dlls/comctl32/tests/listview.c | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index bb394974906..a71e34b99d9 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3406,6 +3406,12 @@ static BOOL LISTVIEW_DeselectAllSkipItems(LISTVIEW_INFO *infoPtr, RANGES toSkip)
lvItem.state = 0; lvItem.stateMask = LVIS_SELECTED; + + /* notify deselect of all items (-1) on LVS_OWNERDATA style */ + if (infoPtr->dwStyle & LVS_OWNERDATA) { + LISTVIEW_SetItemState(infoPtr, -1, &lvItem); + return TRUE; + }
/* need to clone the DPA because callbacks can change it */ if (!(clone = ranges_clone(infoPtr->selectionRanges))) return FALSE; diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index 78b3e3ae069..066858ac7e8 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3598,7 +3598,7 @@ static void test_ownerdata_multiselect(void)
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_1_seq, - "ownerdata multiselect: select multiple via SHIFT", TRUE); + "ownerdata multiselect: select multiple via SHIFT", FALSE);
res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); @@ -3615,7 +3615,7 @@ static void test_ownerdata_multiselect(void)
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_seq, - "ownerdata multiselect: select multiple via SHIFT+CONTROL", TRUE); + "ownerdata multiselect: select multiple via SHIFT+CONTROL", FALSE);
res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); @@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void)
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_deselect_all_select_3_seq, - "ownerdata multiselect: deselect all, select item 3", TRUE); + "ownerdata multiselect: deselect all, select item 3", FALSE);
res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); expect(1, res); @@ -3645,7 +3645,7 @@ static void test_ownerdata_multiselect(void)
ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_deselect_3_select_2_seq, - "ownerdata multiselect: deselect item 3, select item 2", TRUE); + "ownerdata multiselect: deselect item 3, select item 2", FALSE);;
DestroyWindow(hwnd); }
-
Angelo Haller