[PATCH v2 0/6] Fix LVS_OWNERDATA list view multi select.
From: Angelo Haller <angelo(a)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(a)szanni.org> -- 2.36.0
From: Angelo Haller <angelo(a)szanni.org> Add more test cases to ownderdata listviews. Check LVN_ITEMCHANGED IDs. Signed-off-by: Angelo Haller <angelo(a)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); } -- 2.36.0
From: Angelo Haller <angelo(a)szanni.org> Signed-off-by: Angelo Haller <angelo(a)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. -- 2.36.0
From: Angelo Haller <angelo(a)szanni.org> The LVN_ODSTATECHANGED notification should only be sent to lists that have LVS_OWNERDATA set. Signed-off-by: Angelo Haller <angelo(a)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)); -- 2.36.0
From: Angelo Haller <angelo(a)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(a)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); } -- 2.36.0
From: Angelo Haller <angelo(a)szanni.org> Signed-off-by: Angelo Haller <angelo(a)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)); -- 2.36.0
From: Angelo Haller <angelo(a)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(a)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); } -- 2.36.0
participants (1)
-
Angelo Haller