[PATCH v2 resend 1/6] comctl32/tests: Expand ownerdata listview tests.

[Angelo Haller]

On 10/06/2022 03.12, Zhiyi Zhang wrote:
>
> On 5/26/22 04:00, Angelo Haller wrote:
>> From: Angelo Haller <angelo at szanni.org>
>>
>> Add more test cases to ownderdata listviews.
>> Check LVN_ITEMCHANGED IDs.
>>
>> Signed-off-by: Angelo Haller <angelo at szanni.org>
>> ---
>>   dlls/comctl32/tests/listview.c | 59 +++++++++++++++++++++++++++++-----
>>   1 file changed, 51 insertions(+), 8 deletions(-)
>>
>> diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c
>> index 6ac7f53137d..78b3e3ae069 100644
>> --- a/dlls/comctl32/tests/listview.c
>> +++ b/dlls/comctl32/tests/listview.c
>> @@ -255,11 +255,33 @@ static const struct message ownerdata_deselect_all_parent_seq[] = {
>>       { 0 }
>>   };
>>   
>> -static const struct message ownerdata_multiselect_odstatechanged_seq[] = {
>> -    { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
>> +static const struct message ownerdata_multiselect_select_0_to_1_seq[] = {
>> +    { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
>>       { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED },
>> -    { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
>> -    { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED },
>> +    { 0 }
>> +};
>> +
>> +static const struct message ownerdata_multiselect_select_0_to_2_seq[] = {
>> +    { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
>> +    { 0 }
>> +};
>> +
>> +static const struct message ownerdata_multiselect_deselect_all_select_3_seq[] = {
>> +    { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED },
>> +    { 0 }
>> +};
>> +
>> +static const struct message ownerdata_multiselect_deselect_3_select_2_seq[] = {
>> +    { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED },
>> +    { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
>>       { 0 }
>>   };
>>   
>> @@ -3575,8 +3597,8 @@ static void test_ownerdata_multiselect(void)
>>       expect(0, res);
>>   
>>       ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
>> -                ownerdata_multiselect_odstatechanged_seq,
>> -                "ownerdata select multiple notification", TRUE);
>> +                ownerdata_multiselect_select_0_to_1_seq,
>> +                "ownerdata multiselect: select multiple via SHIFT", TRUE);
>>   
>>       res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0);
>>       expect(0, res);
>> @@ -3592,8 +3614,8 @@ static void test_ownerdata_multiselect(void)
>>       expect(0, res);
>>   
>>       ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
>> -                ownerdata_multiselect_odstatechanged_seq,
>> -                "ownerdata select multiple notification", TRUE);
>> +                ownerdata_multiselect_select_0_to_2_seq,
>> +                "ownerdata multiselect: select multiple via SHIFT+CONTROL", TRUE);
>>   
>>       res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0);
>>       expect(0, res);
>> @@ -3604,6 +3626,27 @@ static void test_ownerdata_multiselect(void)
>>       res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
>>       expect(3, res);
>>   
>> +    flush_sequences(sequences, NUM_MSG_SEQUENCES);
>> +
>> +    res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0);
>> +    expect(0, res);
>> +
>> +    ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
>> +                ownerdata_multiselect_deselect_all_select_3_seq,
>> +                "ownerdata multiselect: deselect all, select item 3", TRUE);
>> +
>> +    res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
>> +    expect(1, res);
> Hi Angelo,
>
> Thanks for looking into this. There are a few things can be improved for this series.
>
> Please complete the keyboard sequence by sending WM_KEYUP for VK_DOWN and
> WM_KEYUP for VK_UP.
I'll add the WM_KEYUP events for sure. Not sure how that slipped.
> Also please add tests for pressing VK_UP while holding Shift after setting pressing VK_DOWN while holding Shift.
> Same for holding both Shift and Control And add checks for NMLVODSTATECHANGE member values. For example,
> I don't think uOldState should always be 0.

Happy to add more tests for VK_UP.

With regards to the uOldState: I have not found an instance where it is 
not 0. Which kind of makes sense, at least according to the underlying 
logic.

Windows seems to only use LVN_ODSTATECHANGED to set a new state 
(selection of multiple items). For un-setting (uOldState != 0) Windows 
will instead send an all items changed (LVN_ITEMCHANGED = -1) signal 
instead. That is at least what I have observed in my testing.

>> +
>> +    flush_sequences(sequences, NUM_MSG_SEQUENCES);
>> +
>> +    res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0);
>> +    expect(0, res);
>> +
>> +    ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
>> +                ownerdata_multiselect_deselect_3_select_2_seq,
>> +                "ownerdata multiselect: deselect item 3, select item 2", TRUE);
>> +
>>       DestroyWindow(hwnd);
>>   }
>>