[PATCH v3 0/6] MR550: comctl32/listview: Fix LVS_OWNERDATA list view multi select.
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 v5, with the added comment in 3/6 as requested on the mailing list. These are the patches sent as a response to v4 by Zhiyi Zhang on the mailing list, unmodified apart from 3/6 or f24f4b7fd. --- 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. -- v3: comctl32/tests: Add more ownerdata listview tests. comctl32/listview: Don't send LVN_ODSTATECHANGED for empty ranges. comctl32/listview: Send LVN_ODSTATECHANGED notification for LVS_OWNERDATA listview on selection changes. comctl32/listview: Send LVN_ODSTATECHANGED only for LVS_OWNERDATA listviews. comctl32/listview: Move sending LVN_ODSTATECHANGED notifications to a function. comctl32/listview: Send one deselect all items notification for LVS_OWNERDATA listviews. https://gitlab.winehq.org/wine/wine/-/merge_requests/550
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. Signed-off-by: Angelo Haller <angelo(a)szanni.org> --- dlls/comctl32/listview.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index ab328b3e798..3761a61286e 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3406,7 +3406,14 @@ static BOOL LISTVIEW_DeselectAllSkipItems(LISTVIEW_INFO *infoPtr, RANGES toSkip) lvItem.state = 0; lvItem.stateMask = LVIS_SELECTED; - + + /* Only send one deselect all (-1) notification for 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; iterator_rangesitems(&i, ranges_diff(clone, toSkip)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
From: Angelo Haller <angelo(a)szanni.org> Signed-off-by: Angelo Haller <angelo(a)szanni.org> --- dlls/comctl32/listview.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 3761a61286e..a12b19b8083 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -438,6 +438,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); @@ -3565,7 +3566,6 @@ 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; @@ -3587,13 +3587,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; @@ -9023,6 +9018,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. -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
From: Angelo Haller <angelo(a)szanni.org> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53123 Signed-off-by: Angelo Haller <angelo(a)szanni.org> --- dlls/comctl32/listview.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index a12b19b8083..bf22a90e987 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3587,7 +3587,8 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) for (i = nFirst; i <= nLast; i++) LISTVIEW_SetItemState(infoPtr,i,&item); - LISTVIEW_SetOwnerDataState(infoPtr, nFirst, nLast, &item); + if (infoPtr->dwStyle & LVS_OWNERDATA) + LISTVIEW_SetOwnerDataState(infoPtr, nFirst, nLast, &item); if (!IsWindow(hwndSelf)) return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
From: Angelo Haller <angelo(a)szanni.org> 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 ++++++++++++---- dlls/comctl32/tests/listview.c | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index bf22a90e987..e48b1fa2847 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3610,6 +3610,7 @@ static BOOL LISTVIEW_AddGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) */ static void LISTVIEW_SetGroupSelection(LISTVIEW_INFO *infoPtr, INT nItem) { + INT nFirst = -1, nLast = -1; RANGES selection; DWORD old_mask; LVITEMW item; @@ -3661,21 +3662,28 @@ 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); + { + /* Find the range for LVN_ODSTATECHANGED */ + if (nFirst == -1) + nFirst = i.nItem; + nLast = i.nItem; + LISTVIEW_SetItemState(infoPtr, i.nItem, &item); + } /* this will also destroy the selection */ iterator_destroy(&i); + if (infoPtr->dwStyle & LVS_OWNERDATA) + LISTVIEW_SetOwnerDataState(infoPtr, nFirst, nLast, &item); + infoPtr->notify_mask |= old_mask; LISTVIEW_SetItemFocus(infoPtr, nItem); } diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index ed5222a5ee8..fec1a83de4d 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3577,7 +3577,7 @@ static void test_ownerdata_multiselect(void) ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", TRUE); + "ownerdata select multiple notification", FALSE); res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); @@ -3594,7 +3594,7 @@ static void test_ownerdata_multiselect(void) ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", TRUE); + "ownerdata select multiple notification", FALSE); res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
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 e48b1fa2847..38d43a9f742 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -9032,6 +9032,7 @@ static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT nFirst, INT n { NMLVODSTATECHANGE nmlv; + if (nFirst == nLast) return; if (!item) return; ZeroMemory(&nmlv, sizeof(nmlv)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
From: Angelo Haller <angelo(a)szanni.org> Signed-off-by: Angelo Haller <angelo(a)szanni.org> --- dlls/comctl32/tests/listview.c | 241 +++++++++++++++++++++++++++++++-- 1 file changed, 231 insertions(+), 10 deletions(-) diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index fec1a83de4d..c8a38ec9037 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -256,11 +256,77 @@ 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_odstatechanged_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_odstatechanged_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_select_0_modkey_odstatechanged_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_move_0_to_1_odstatechanged_seq[] = { + { 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_odstatechanged_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_select_3_odstatechanged_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_select_3_modkey_odstatechanged_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 }, + { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_select_3_to_2_odstatechanged_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED }, + { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_move_3_to_2_odstatechanged_seq[] = { + { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, + { 0 } +}; + +static const struct message ownerdata_multiselect_select_3_to_1_odstatechanged_seq[] = { + { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED }, + { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED }, + { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED }, { 0 } }; @@ -3567,43 +3633,198 @@ static void test_ownerdata_multiselect(void) res = SendMessageA(hwnd, LVM_SETSELECTIONMARK, 0, 0); expect(0, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* First down then up */ + /* Select multiple items via SHIFT+DOWN */ hold_key(VK_SHIFT); + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_to_1_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+DOWN", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(2, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select one item via SHIFT+UP */ + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_modkey_odstatechanged_seq, + "ownerdata multiselect: select one via SHIFT+UP", TRUE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select multiple items via SHIFT+CONTROL+DOWN */ + hold_key(VK_CONTROL); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_to_1_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+CONTROL+DOWN", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(2, res); flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Select one item via SHIFT+CONTROL+UP */ + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_modkey_odstatechanged_seq, + "ownerdata multiselect: select one via SHIFT+CONTROL+UP", TRUE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Keep selection but move cursor via CONTROL+DOWN */ + release_key(VK_SHIFT); + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_move_0_to_1_odstatechanged_seq, + "ownerdata multiselect: keep selection but move cursor via CONTROL+DOWN", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Select multiple via SHIFT+CONTROL+DOWN after moving cursor over an item without selecting */ + hold_key(VK_SHIFT); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, - ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", FALSE); + ownerdata_multiselect_select_0_to_2_odstatechanged_seq, + "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(3, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Deselect all items, select item 3 via DOWN */ + release_key(VK_CONTROL); + release_key(VK_SHIFT); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_odstatechanged_seq, + "ownerdata multiselect: deselect all, select item 3 via DOWN", FALSE); res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* First up then down */ + /* Select multiple items via SHIFT+UP */ + hold_key(VK_SHIFT); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_to_2_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+UP", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); expect(2, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Select one item via SHIFT+DOWN */ + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_modkey_odstatechanged_seq, + "ownerdata multiselect: select one via SHIFT+DOWN", TRUE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select multiple items via SHIFT+CONTROL+UP */ hold_key(VK_CONTROL); + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_to_2_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+CONTROL+UP", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(2, res); flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Select one item via SHIFT+CONTROL+DOWN */ res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_modkey_odstatechanged_seq, + "ownerdata multiselect: select one via SHIFT+CONTROL+DOWN", TRUE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Keep selection but move cursor via CONTROL+UP */ + release_key(VK_SHIFT); + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, - ownerdata_multiselect_odstatechanged_seq, - "ownerdata select multiple notification", FALSE); + ownerdata_multiselect_move_3_to_2_odstatechanged_seq, + "ownerdata multiselect: keep selection but move cursor via CONTROL+UP ", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(1, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); - res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + /* Select multiple via SHIFT+CONTROL+UP after moving cursor over an item without selecting */ + hold_key(VK_SHIFT); + + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_3_to_1_odstatechanged_seq, + "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+UP", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(3, res); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* Deselect all items, select item 3 via UP */ release_key(VK_CONTROL); release_key(VK_SHIFT); + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_odstatechanged_seq, + "ownerdata multiselect: deselect all, select item 0 via UP", FALSE); + res = SendMessageA(hwnd, WM_KEYUP, VK_UP, 0); + expect(0, res); res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); - expect(3, res); + expect(1, res); DestroyWindow(hwnd); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/550
On Fri Jul 29 13:40:40 2022 +0000, Angelo Haller wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/550/diffs?diff_id=6451&start_sha=fe74dc5a00cdedbbcf0e226479d77a7f1213c245#42d3400c132634d04e82cefe16f31c77eeeaa67b_3675_3675) Yup, no C99 comments. Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/550#note_5323
On Fri Jul 29 13:40:43 2022 +0000, Angelo Haller wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/550/diffs?diff_id=6451&start_sha=fe74dc5a00cdedbbcf0e226479d77a7f1213c245#0251bfdc5c488ffd767c45cd3a965188becc33f7_3809_3804) Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/550#note_5324
On Fri Jul 29 02:37:46 2022 +0000, Zhiyi Zhang wrote:
> For consistency, let's move hold_key() to the start of each test. Right
> now some of them are at the start and some are at the up.
I moved things around. So now the sequence is
1. flush()
2. comment describing the following test.
3. hold_key(), keydown, etc.
The other idea I had was to replace all the code with a structure and loop over that. Something like:
```c
struct kbd_selection_sequence {
BOOL hold_shift;
BOOL hold_control;
UINT press_key;
int selected_count;
const struct message *sequence;
};
struct kbd_selection_sequence sequence[] = {
/* Select multiple items via SHIFT+DOWN */
{1, 0, VK_DOWN, 2, ownerdata_multiselect_select_0_to_1_odstatechanged_seq},
/* Select one item via SHIFT+UP */
{1, 0, VK_UP, 1, ownerdata_multiselect_select_0_modkey_odstatechanged_seq},
/* ... */
};
for (i = 0; i < ARRAY_SIZE(sequence); i++) {
if (sequence[i].hold_shift)
hold_key(VK_SHIFT);
/* ... */
}
```
Maybe that would make things a little easier to read and possibly even repurpose for non LVS_OWNERDATA listviews?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/550#note_5327
On Fri Jul 29 14:12:39 2022 +0000, Angelo Haller wrote:
I moved things around. So now the sequence is 1. flush() 2. comment describing the following test. 3. hold_key(), keydown, etc. The other idea I had was to replace all the code with a structure and loop over that. Something like: ```c struct kbd_selection_sequence { BOOL hold_shift; BOOL hold_control; UINT press_key; int selected_count; const struct message *sequence; }; struct kbd_selection_sequence sequence[] = { /* Select multiple items via SHIFT+DOWN */ {1, 0, VK_DOWN, 2, ownerdata_multiselect_select_0_to_1_odstatechanged_seq}, /* Select one item via SHIFT+UP */ {1, 0, VK_UP, 1, ownerdata_multiselect_select_0_modkey_odstatechanged_seq}, /* ... */ }; for (i = 0; i < ARRAY_SIZE(sequence); i++) { if (sequence[i].hold_shift) hold_key(VK_SHIFT); /* ... */ } ``` Maybe that would make things a little easier to read and possibly even repurpose for non LVS_OWNERDATA listviews? Yeah, if you can structure them into a loop it would make it much clearer. I prefer TRUE/FALSE instead of 1/0 for BOOL. The test sequence can be marked as static const. You can also add a current key state to avoid calling hold_key() if a key is already being held.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/550#note_5328
participants (3)
-
Angelo Haller -
Angelo Haller (＠szanni)
-
Zhiyi Zhang (＠zhiyi)