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.
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.
Signed-off-by: Angelo Haller angelo@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));
From: Angelo Haller angelo@szanni.org
Signed-off-by: Angelo Haller angelo@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.
From: Angelo Haller angelo@szanni.org
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53123 Signed-off-by: Angelo Haller angelo@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;
From: Angelo Haller angelo@szanni.org
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52534 Signed-off-by: Angelo Haller angelo@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);
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 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));
From: Angelo Haller angelo@szanni.org
Signed-off-by: Angelo Haller angelo@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); }
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.
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.
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?
On Fri Jul 29 14:12:39 2022 +0000, Angelo Haller wrote:
I moved things around. So now the sequence is
- flush()
- comment describing the following test.
- hold_key(), keydown, etc.
The other idea I had was to replace all the code with a structure and loop over that. Something like:
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.
-
Angelo Haller -
Angelo Haller (＠szanni)
-
Zhiyi Zhang (＠zhiyi)