[PATCH 2/2] user32/listbox: Update the size in SetColumnWidth unconditionally

[Gabriel Ivăncescu]

On Tue, Aug 21, 2018 at 1:25 PM, Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
>
> On 08/20/2018 06:49 PM, Gabriel Ivăncescu wrote:
>>
>> When the listbox receives a WM_WINDOWPOSCHANGED message and calls
>> DefWindowProc on it, which then ends up sending a WM_SIZE to the window,
>> some applications swallow that up and won't pass the WM_SIZE to the listbox.
>> However, they do pass a LB_SETCOLUMNWIDTH, even when the column width did
>> not change at all. So we have to unconditionally update the size itself (not
>> just the page) when we receive that message.
>>
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22440
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>   dlls/user32/listbox.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
>> index 991ab87..fbed95c 100644
>> --- a/dlls/user32/listbox.c
>> +++ b/dlls/user32/listbox.c
>> @@ -1265,10 +1265,12 @@ static LRESULT LISTBOX_SetHorizontalExtent(
>> LB_DESCR *descr, INT extent )
>>    */
>>   static LRESULT LISTBOX_SetColumnWidth( LB_DESCR *descr, INT width)
>>   {
>> -    if (width == descr->column_width) return LB_OKAY;
>> -    TRACE("[%p]: new column width = %d\n", descr->self, width );
>> +    /* update the SIZE unconditionally, because some applications swallow
>> +       WM_SIZE from DefWindowProc and only send LB_SETCOLUMNWIDTH, even
>> +       when the column width doesn't even change (but its size does) */
>> +    TRACE("[%p]: new column width = %d\n", descr->self, width);
>>       descr->column_width = width;
>> -    LISTBOX_UpdatePage( descr );
>> +    LISTBOX_UpdateSize(descr);
>>       return LB_OKAY;
>>   }
>>
>
>
> Hi, Gabriel.
>
> Thanks for the patches. I think there's more problems with resizing, that
> could affect how this one should be fixed. For example if
> LBS_NOINTEGRALHEIGHT is not set it's possible to get last item partially
> visible. On Windows initial window size is also adjusted to be a multiple of
> item height, so setting column width does not have to resize anything.
>
> When control does not have WM_SIZE is integral height still enforced?
>
> Could you look into that?

Hello Nikolay,

In LISTBOX_UpdateSize, the check for lack of LBS_NOINTEGRALHEIGHT is
skipped (since Total Commander does have it set in this case). Though,
shouldn't that handle it properly if LBS_NOINTEGRALHEIGHT is not set?
It seems UpdateSize has code to deal with that. The second part of
that check for LBS_OWNERDRAWVARIABLE is always going to be true in
such situation, since SetColumnWidth only applies to multi-column
listboxes and they are incompatible with that style.

I don't think that calling UpdateSize can do any actual "harm", except
just recalculating some things (based on width & height).
Unfortunately I don't know of an application without
LBS_NOINTEGRALHEIGHT to test, so I'd have to artificially inject it in
Total Commander but I don't know if that's a proper way to test it...

Maybe UpdateSize itself needs fixing when LBS_NOINTEGRALHEIGHT is not
set? (but I feel it should be a different bug)


This patch just handles a quirk of some applications and apparently
Windows. I don't know what Windows does exactly, but I think it
updates the width & height when it receives a LB_SETCOLUMNWIDTH as
well, otherwise it wouldn't work. The important part of UpdateSize
that has to get called unconditionally is the GetClientRect(
descr->self, &rect ); and updating the width & height that (possibly)
changed even if the column width did not. After that it calls
UpdatePage anyway. I mean I could've called GetClientRect myself and
do that, but I thought reusing UpdateSize was a better idea :) Since
it also handles LBS_NOINTEGRALHEIGHT unset.


In summary: the check of   if(descr->column_width == width)   is
logically valid but it results in the bug, because SetColumnWidth
apparently gets called instead of WM_SIZE *even* if the column's width
does *not* change at all, which is obviously a quirk that Windows
seems to handle and probably why applications make use of it. So the
width & height themselves have to be updated to match what WM_SIZE
would have done (if they did, there's no way to know without calling
UpdateSize or replicating that code...).

So what happens is something like this:

WM_WINDOWPOSCHANGED in listbox calls DefWindowProc -> eventually sends
WM_MOVE (passed correctly to listbox) followed by WM_SIZE -> app's
window proc hijacks WM_SIZE and does not pass WM_SIZE to listbox,
instead it sends LB_SETCOLUMNWIDTH, *even* if the width did not change
at all (so descr->column_width == width in many such cases). However,
the width & height did change, since it completely replaces any such
WM_SIZE with it, that's why it has to be updated without regard for
column_width.

But, if you had something else in mind, please elaborate, I feel like
I'm missing something obvious... or I was too verbose. This quirk
surprised me at least.


Another way to "fix" this would be to call UpdateSize from
WM_WINDOWPOSCHANGED itself, but then we'd call it again when WM_SIZE
is passed later (in rare cases for multi-column listboxes), and I
don't think that's a good idea as it will also mess with
non-multi-column listboxes... and feels more hackish to me. But maybe
it's better?