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 v4 fixing the log buffer overflow on the debian test machine by moving some of the up/down key sequences to a later patch. Patch 1/7 still includes the necessary up/down key sequences to validate the fixes in the following patches. The matching up/down key sequences for 1/7 are then introduced in 7/7. I was sadly unable to introduce the test sequence with each fix, as this would require inventing entirely new test sequences, just to work around the debian 32KB log buffer limit.
I hope this is ok as it stands.
I am also attaching the my remarks from v3, as I don't know if they got ignored due to failing tests:
I was sadly not able to trigger any deselect sequences through emulating mouse clicks. I was actually completely unable even send a lef mouse button down at all. Both SendMessage and SendInput fail, on wind and windows.
I am unsure if programmatic mouse clicks are even supported in list views? Some forums seem to suggest, that this is only the case for buttons and similar elements. I was following the code snippets in other parts of the comctl32 tests.
The other thing might be that the signal is getting caught somewhere in the test code. If anybody has any more insight in this regard, I'd be happy to add additional mouse click tests as well.
The other thing I was unable to do is activate the single select tests via SHIFT/+COMMAND to show we need patch 6/6. Windows weirdly informs about the selected item twice, once to inform the item has been selected and then in another call later about the item being focused as well. This seemingly only affects LVS_OWNERDATA listviews from my tests.
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 (7): Add more test cases to ownderdata listviews: comctl32/listview: Fix deselect on LVS_OWNERDATA. comctl32/listview: Move sending LVN_ODSTATECHANGED notifications to a 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/tests: Expand ownerdata listview tests.
dlls/comctl32/listview.c | 55 ++++--- dlls/comctl32/tests/listview.c | 253 +++++++++++++++++++++++++++++++-- 2 files changed, 281 insertions(+), 27 deletions(-)
Signed-off-by: Angelo Haller angelo@szanni.org
From: Angelo Haller angelo@szanni.org
- Check LVN_ITEMCHANGED IDs - Multi select via key down while pressing SHIFT/SHIFT+CONTROL - Single select via key up while pressing SHIFT/SHIFT+CONTROL - Deselect via key down
Signed-off-by: Angelo Haller angelo@szanni.org
--- v4: Move some up/down key sequences from to 7/7 to account for 32KB Debian log buffer limit. v3: Add missing WM_KEYUP events. Add more combinations of up/down keys. --- dlls/comctl32/tests/listview.c | 118 +++++++++++++++++++++++++++++---- 1 file changed, 106 insertions(+), 12 deletions(-)
diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index 6ac7f53137d..ec76984175f 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -255,11 +255,40 @@ 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_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 } };
@@ -3571,38 +3600,103 @@ static void test_ownerdata_multiselect(void)
flush_sequences(sequences, NUM_MSG_SEQUENCES);
+ /* Select multiple items via SHIFT+DOWN */ 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", TRUE); - + ownerdata_multiselect_select_0_to_1_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+DOWN", TRUE); 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); + hold_key(VK_CONTROL);
flush_sequences(sequences, NUM_MSG_SEQUENCES);
+ /* Select multiple items 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_0_to_1_odstatechanged_seq, + "ownerdata multiselect: select multiple via SHIFT+CONTROL+DOWN", TRUE); + 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); + + release_key(VK_SHIFT);
+ flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Keep selection but move cursor via CONTROL+DOWN */ + 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", TRUE); + 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); + + hold_key(VK_SHIFT); + + flush_sequences(sequences, NUM_MSG_SEQUENCES);
+ /* Select multiple via SHIFT+CONTROL+DOWN after moving cursor over an item without selecting */ + res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + ownerdata_multiselect_select_0_to_2_odstatechanged_seq, + "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE); res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); expect(0, res); + res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); + expect(3, res);
release_key(VK_CONTROL); release_key(VK_SHIFT);
+ flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Deselect all items, select item 3 via DOWN */ + 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", TRUE); + res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0); + expect(0, res); res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); - expect(3, res); + expect(1, res);
DestroyWindow(hwnd); }
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
--- v3: Remove double `;;` and clarify comment. Move patch to the front to enable tests in a more fine grained manner. --- dlls/comctl32/listview.c | 6 ++++++ dlls/comctl32/tests/listview.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 730bf4aaddd..6fc58c933a9 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3405,6 +3405,12 @@ static BOOL LISTVIEW_DeselectAllSkipItems(LISTVIEW_INFO *infoPtr, RANGES toSkip)
lvItem.state = 0; lvItem.stateMask = LVIS_SELECTED; + + /* Only send one deselect all (-1) notification 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 ec76984175f..7243a1858cd 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3692,7 +3692,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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);
From: Angelo Haller angelo@szanni.org
Signed-off-by: Angelo Haller angelo@szanni.org --- v2: Remove header and change return to VOID. v3: Comment capitalization and better subject line. --- 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 6fc58c933a9..03ce801e4cc 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); @@ -3563,16 +3564,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; @@ -3585,13 +3581,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; @@ -8956,6 +8947,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.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53123 Signed-off-by: Angelo Haller angelo@szanni.org
--- v3: Add wine bug reference. Use function call guard instead of early return. --- 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 03ce801e4cc..5ba1924cbd7 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3581,7 +3581,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
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
--- v3: Merge nFirst & nLast definition into a single line. Add function call guard due to preceding patch changes. --- dlls/comctl32/listview.c | 16 +++++++++++----- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 5ba1924cbd7..0243d9a84ce 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3604,6 +3604,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; @@ -3655,21 +3656,26 @@ 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);
+ 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 7243a1858cd..d8a27ea18f4 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3605,7 +3605,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3676,7 +3676,7 @@ static void test_ownerdata_multiselect(void) expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_odstatechanged_seq, - "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE); + "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);
On 6/29/22 05:16, Angelo Haller wrote:
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
v3: Merge nFirst & nLast definition into a single line. Add function call guard due to preceding patch changes.
dlls/comctl32/listview.c | 16 +++++++++++----- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 5ba1924cbd7..0243d9a84ce 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3604,6 +3604,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;
@@ -3655,21 +3656,26 @@ 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);- }
Hi Angelo,
This looks like a separate change than what you describe in the subject. Please put it in a separate patch.
Other than the above. There are some typos in 1/7 but I took care of it. I made some changes to the patch series overall and attached them in the mail attachments. Mostly some style fix. You can make your changes on top of them.
Then there are still some message sequences in test_ownerdata_multiselect() that don't pass. It looks like one issue for the rest of the todo_wines and they're probably existing failures. But could you fix them as well?
Thanks for your work. It's much better than the last revision.
Best Regards, Zhiyi
/* 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 7243a1858cd..d8a27ea18f4 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3605,7 +3605,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE);
"ownerdata multiselect: select multiple via SHIFT+DOWN", FALSE);@@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE);
"ownerdata multiselect: select multiple via SHIFT+CONTROL+DOWN", FALSE);@@ -3676,7 +3676,7 @@ static void test_ownerdata_multiselect(void) expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_odstatechanged_seq,
"ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE);
"ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", FALSE);
On 30/06/2022 04.41, Zhiyi Zhang wrote:
On 6/29/22 05:16, Angelo Haller wrote:
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
v3: Merge nFirst & nLast definition into a single line. Add function call guard due to preceding patch changes.
dlls/comctl32/listview.c | 16 +++++++++++----- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 5ba1924cbd7..0243d9a84ce 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3604,6 +3604,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;
@@ -3655,21 +3656,26 @@ 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);- }
Hi Angelo,
This looks like a separate change than what you describe in the subject. Please put it in a separate patch.
Hi Zhiyi,
thanks for the review.
I believe the patch does exactly what is described in the subject. Maybe I could add another line describing in detail what is being done, as it does not seem entirely clear? Maybe something like:
`Compute the range by determining the first and last item of the selection, then send one LVN_ODSTATECHANGED notification.` ?
The above code changes are for determining what the first and last items are. These are needed for making the call to `LISTVIEW_SetOwnerDataState()`, which is done in the code below. No other function calls are introduced in the patch.
Moving the computation of nFirst and nLast to a different patch would trigger a "set but not used" warning by the compiler.
Other than the above. There are some typos in 1/7 but I took care of it. I made some changes to the patch series overall and attached them in the mail attachments. Mostly some style fix. You can make your changes on top of them.
Thanks, I guess I must have missed those even with my spell checker on.
Then there are still some message sequences in test_ownerdata_multiselect() that don't pass. It looks like one issue for the rest of the todo_wines and they're probably existing failures. But could you fix them as well?
Yes, these are existing failures. I believe these should be addressed in a different patch series though.
Windows seems to send some very strange message sequences on ownerdata lists. The current behavior in wine is correct for non ownerdata lists. I believe more rigorous testing for non ownerdata listviews would be needed to not introduce regressions.
All the signals are being sent, it is just that the current sequence does not match Windows fully. I am happy to look into that when I have some extra time, but as previously stated, this will most likely be a similarly sized patch series to this one.
Thanks for your work. It's much better than the last revision.
Should I resubmit the patches you attached to the mailing list? Leaving your Signed off intact as long as I do not change the patches?
Best, Angelo
Best Regards, Zhiyi
/* 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 7243a1858cd..d8a27ea18f4 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3605,7 +3605,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE);
"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);@@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE);
"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);@@ -3676,7 +3676,7 @@ static void test_ownerdata_multiselect(void) expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_odstatechanged_seq,
"ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE);
"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);
On 7/1/22 01:57, Angelo Haller wrote:
On 30/06/2022 04.41, Zhiyi Zhang wrote:
On 6/29/22 05:16, Angelo Haller wrote:
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
v3: Merge nFirst & nLast definition into a single line. Add function call guard due to preceding patch changes.
dlls/comctl32/listview.c | 16 +++++++++++----- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 5ba1924cbd7..0243d9a84ce 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3604,6 +3604,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; @@ -3655,21 +3656,26 @@ 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); + }
Hi Angelo,
This looks like a separate change than what you describe in the subject. Please put it in a separate patch.
Hi Zhiyi,
thanks for the review.
I believe the patch does exactly what is described in the subject. Maybe I could add another line describing in detail what is being done, as it does not seem entirely clear? Maybe something like:
`Compute the range by determining the first and last item of the selection, then send one LVN_ODSTATECHANGED notification.` ?
Hi Angelo,
Sorry for the late response. I was busy with other things and kind of forgot about this patch set. I would recommend you to use GitLab in the future so that I know there is a task assigned to me.
For the code, add a line of 'Find the range for LVN_ODSTATECHANGED' before 'if (nFirst == -1)' and add an empty line before "LISTVIEW_SetItemState(infoPtr, i.nItem, &item); " should be enough.
The above code changes are for determining what the first and last items are. These are needed for making the call to `LISTVIEW_SetOwnerDataState()`, which is done in the code below. No other function calls are introduced in the patch.
Moving the computation of nFirst and nLast to a different patch would trigger a "set but not used" warning by the compiler.
Other than the above. There are some typos in 1/7 but I took care of it. I made some changes to the patch series overall and attached them in the mail attachments. Mostly some style fix. You can make your changes on top of them.
Thanks, I guess I must have missed those even with my spell checker on.
Then there are still some message sequences in test_ownerdata_multiselect() that don't pass. It looks like one issue for the rest of the todo_wines and they're probably existing failures. But could you fix them as well?
Yes, these are existing failures. I believe these should be addressed in a different patch series though.
Windows seems to send some very strange message sequences on ownerdata lists. The current behavior in wine is correct for non ownerdata lists. I believe more rigorous testing for non ownerdata listviews would be needed to not introduce regressions.
All the signals are being sent, it is just that the current sequence does not match Windows fully. I am happy to look into that when I have some extra time, but as previously stated, this will most likely be a similarly sized patch series to this one.
Thank you for looking into it. If the whole series is too large, you can send the whole patch set to email first. Then send the first 5~7 patches to GitLab if they look good.
Thanks for your work. It's much better than the last revision.
Should I resubmit the patches you attached to the mailing list? Leaving your Signed off intact as long as I do not change the patches?
Yes, feel free to use those patches. Sign-offs are not necessary after the transition to Gitlab. So you can remove my sign offs. And when you send them through Gitlab, I will approve them if the merge request looks good.
Thanks, Zhiyi
Best, Angelo
Best Regards, Zhiyi
/* 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 7243a1858cd..d8a27ea18f4 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3605,7 +3605,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3676,7 +3676,7 @@ static void test_ownerdata_multiselect(void) expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_odstatechanged_seq, - "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE); + "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);
On 17/07/2022 02.29, Zhiyi Zhang wrote:
On 7/1/22 01:57, Angelo Haller wrote:
On 30/06/2022 04.41, Zhiyi Zhang wrote:
On 6/29/22 05:16, Angelo Haller wrote:
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
v3: Merge nFirst & nLast definition into a single line. Add function call guard due to preceding patch changes.
dlls/comctl32/listview.c | 16 +++++++++++----- dlls/comctl32/tests/listview.c | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 5ba1924cbd7..0243d9a84ce 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -3604,6 +3604,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; @@ -3655,21 +3656,26 @@ 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); + }
Hi Angelo,
This looks like a separate change than what you describe in the subject. Please put it in a separate patch.
Hi Zhiyi,
thanks for the review.
I believe the patch does exactly what is described in the subject. Maybe I could add another line describing in detail what is being done, as it does not seem entirely clear? Maybe something like:
`Compute the range by determining the first and last item of the selection, then send one LVN_ODSTATECHANGED notification.` ?
Hi Angelo,
Sorry for the late response. I was busy with other things and kind of forgot about this patch set. I would recommend you to use GitLab in the future so that I know there is a task assigned to me.
For the code, add a line of 'Find the range for LVN_ODSTATECHANGED' before 'if (nFirst == -1)' and add an empty line before "LISTVIEW_SetItemState(infoPtr, i.nItem, &item); " should be enough.
Hi Zhiyi,
I migrated the patches to Gitlab and added the requested comment. Hope things look good now.
I don't believe I have the required rights to assign you, so here the link to the Gitlab MR: https://gitlab.winehq.org/wine/wine/-/merge_requests/550
Best, Angelo
The above code changes are for determining what the first and last items are. These are needed for making the call to `LISTVIEW_SetOwnerDataState()`, which is done in the code below. No other function calls are introduced in the patch.
Moving the computation of nFirst and nLast to a different patch would trigger a "set but not used" warning by the compiler.
Other than the above. There are some typos in 1/7 but I took care of it. I made some changes to the patch series overall and attached them in the mail attachments. Mostly some style fix. You can make your changes on top of them.
Thanks, I guess I must have missed those even with my spell checker on.
Then there are still some message sequences in test_ownerdata_multiselect() that don't pass. It looks like one issue for the rest of the todo_wines and they're probably existing failures. But could you fix them as well?
Yes, these are existing failures. I believe these should be addressed in a different patch series though.
Windows seems to send some very strange message sequences on ownerdata lists. The current behavior in wine is correct for non ownerdata lists. I believe more rigorous testing for non ownerdata listviews would be needed to not introduce regressions.
All the signals are being sent, it is just that the current sequence does not match Windows fully. I am happy to look into that when I have some extra time, but as previously stated, this will most likely be a similarly sized patch series to this one.
Thank you for looking into it. If the whole series is too large, you can send the whole patch set to email first. Then send the first 5~7 patches to GitLab if they look good.
Thanks for your work. It's much better than the last revision.
Should I resubmit the patches you attached to the mailing list? Leaving your Signed off intact as long as I do not change the patches?
Yes, feel free to use those patches. Sign-offs are not necessary after the transition to Gitlab. So you can remove my sign offs. And when you send them through Gitlab, I will approve them if the merge request looks good.
Thanks, Zhiyi
Best, Angelo
Best Regards, Zhiyi
/* 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 7243a1858cd..d8a27ea18f4 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3605,7 +3605,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3633,7 +3633,7 @@ static void test_ownerdata_multiselect(void) 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", TRUE); + "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); @@ -3676,7 +3676,7 @@ static void test_ownerdata_multiselect(void) expect(0, res); ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, ownerdata_multiselect_select_0_to_2_odstatechanged_seq, - "ownerdata multiselect: select multiple after skip via SHIFT+CONTROL+DOWN", TRUE); + "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);
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 0243d9a84ce..8118a3dd8de 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -8959,6 +8959,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
Add more test cases to ownderdata listviews:
- Complete test to include matching up/down key sequences using SHIFT/SHIFT+CONTROL
Signed-off-by: Angelo Haller angelo@szanni.org
--- v4: Move some up/down key sequences from v3 1/6 to account for 32KB Debian log buffer limit. --- dlls/comctl32/tests/listview.c | 139 +++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+)
diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index d8a27ea18f4..92415da981d 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -263,6 +263,13 @@ static const struct message ownerdata_multiselect_select_0_to_1_odstatechanged_s { 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 }, @@ -292,6 +299,36 @@ static const struct message ownerdata_multiselect_select_3_odstatechanged_seq[] { 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 } +}; + static const struct message change_all_parent_seq[] = { { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGING }, { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED }, @@ -3698,6 +3735,108 @@ static void test_ownerdata_multiselect(void) res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0); expect(1, res);
+ hold_key(VK_SHIFT); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select multiple items via SHIFT+UP */ + 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); + + hold_key(VK_CONTROL); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select multiple items 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_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); + + release_key(VK_SHIFT); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Keep selection but move cursor via CONTROL+UP */ + res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0); + expect(0, res); + ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX, + 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); + + hold_key(VK_SHIFT); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Select multiple via SHIFT+CONTROL+DOWN after moving cursor over an item without selecting */ + 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); + + release_key(VK_CONTROL); + release_key(VK_SHIFT); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + /* Deselect all items, select item 3 via UP */ + 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(1, res); + DestroyWindow(hwnd); }
-
Angelo Haller -
Zhiyi Zhang