[PATCH 1/2] comctl32/listbox: Don't update scrollbar during window creation
This is needed for the linked bug, although it doesn't fix it yet. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=43465 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/comctl32/listbox.c | 7 +++++++ dlls/comctl32/tests/listbox.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef86e0..a9d1db458b 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -91,6 +91,7 @@ typedef struct HFONT font; /* Current font */ LCID locale; /* Current locale for string comparisons */ HEADCOMBO *lphc; /* ComboLBox */ + BOOL got_wm_size; /* Did we get already a WM_SIZE? */ } LB_DESCR; @@ -200,6 +201,10 @@ static void LISTBOX_UpdateScroll( LB_DESCR *descr ) the programmer use it for his/her own purposes. */ if (descr->style & LBS_NOREDRAW) return; + + /* When height is 0 don't update scrollbar during CreateWindowEx, that is until we receive our first WM_SIZE */ + if (!descr->got_wm_size && descr->height == 0) return; + info.cbSize = sizeof(info); if (descr->style & LBS_MULTICOLUMN) @@ -2502,6 +2507,7 @@ static BOOL LISTBOX_Create( HWND hwnd, LPHEADCOMBO lphc ) descr->font = 0; descr->locale = GetUserDefaultLCID(); descr->lphc = lphc; + descr->got_wm_size = FALSE; if( lphc ) { @@ -2887,6 +2893,7 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam, case WM_SIZE: LISTBOX_UpdateSize( descr ); + descr->got_wm_size = TRUE; return 0; case WM_GETFONT: return (LRESULT)descr->font; diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index c9d13a8d67..e3a3b42c2f 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -2176,6 +2176,40 @@ static void test_WM_MEASUREITEM(void) DestroyWindow(parent); } +static void test_WS_VSCROLL(void) +{ + HWND parent, listbox; + UINT style; + + parent = create_parent(); + + /* Listbox must always have WS_VSCROLL after creating when height 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); + SendMessageA(listbox, WM_SIZE, SIZE_RESTORED, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Listbox must not have WS_VSCROLL after creating when height > 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 100, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Adding elements must change WS_VSCROLL flag */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); + + DestroyWindow(parent); +} + START_TEST(listbox) { ULONG_PTR ctx_cookie; @@ -2202,6 +2236,7 @@ START_TEST(listbox) test_extents(); test_WM_MEASUREITEM(); test_LB_SETSEL(); + test_WS_VSCROLL(); unload_v6_module(ctx_cookie, hCtx); } -- 2.19.0
Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/user32/listbox.c | 7 +++++++ dlls/user32/tests/listbox.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index c8bd148d63..7bbdcd2d39 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -86,6 +86,7 @@ typedef struct HFONT font; /* Current font */ LCID locale; /* Current locale for string comparisons */ LPHEADCOMBO lphc; /* ComboLBox */ + BOOL got_wm_size; /* Did we get already a WM_SIZE? */ } LB_DESCR; @@ -225,6 +226,10 @@ static void LISTBOX_UpdateScroll( LB_DESCR *descr ) the programmer use it for his/her own purposes. */ if (descr->style & LBS_NOREDRAW) return; + + /* When height is 0 don't update scrollbar during CreateWindowEx, that is until we receive our first WM_SIZE */ + if (!descr->got_wm_size && descr->height == 0) return; + info.cbSize = sizeof(info); if (descr->style & LBS_MULTICOLUMN) @@ -2507,6 +2512,7 @@ static BOOL LISTBOX_Create( HWND hwnd, LPHEADCOMBO lphc ) descr->font = 0; descr->locale = GetUserDefaultLCID(); descr->lphc = lphc; + descr->got_wm_size = FALSE; if( lphc ) { @@ -2993,6 +2999,7 @@ LRESULT ListBoxWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam return ret; case WM_SIZE: LISTBOX_UpdateSize( descr ); + descr->got_wm_size = TRUE; return 0; case WM_GETFONT: return (LRESULT)descr->font; diff --git a/dlls/user32/tests/listbox.c b/dlls/user32/tests/listbox.c index dfa8de7b88..785b6d7df9 100644 --- a/dlls/user32/tests/listbox.c +++ b/dlls/user32/tests/listbox.c @@ -1980,6 +1980,40 @@ static void test_WM_MEASUREITEM(void) DestroyWindow(parent); } +static void test_WS_VSCROLL(void) +{ + HWND parent, listbox; + UINT style; + + parent = create_parent(); + + /* Listbox must always have WS_VSCROLL after creating when height 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); + SendMessageA(listbox, WM_SIZE, SIZE_RESTORED, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Listbox must not have WS_VSCROLL after creating when height > 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 100, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Adding elements must change WS_VSCROLL flag */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); + + DestroyWindow(parent); +} + START_TEST(listbox) { const struct listbox_test SS = @@ -2065,4 +2099,5 @@ START_TEST(listbox) test_extents(); test_WM_MEASUREITEM(); test_LB_SETSEL(); + test_WS_VSCROLL(); } -- 2.19.0
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=41993 Your paranoid android. === wvistau64 (32 bit Windows report) === user32: listbox.c:844: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:1079: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_zh_CN (32 bit Windows report) === user32: listbox.c:844: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:1079: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_fr (32 bit Windows report) === user32: listbox.c:844: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:1079: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_he (32 bit Windows report) === user32: listbox.c:844: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:1079: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit Windows report) === user32: listbox.c:844: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:1079: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=41992 Your paranoid android. === wvistau64 (32 bit Windows report) === comctl32: listbox.c:1129: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_zh_CN (32 bit Windows report) === comctl32: listbox.c:1129: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_fr (32 bit Windows report) === comctl32: listbox.c:1129: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64_he (32 bit Windows report) === comctl32: listbox.c:1129: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit Windows report) === comctl32: listbox.c:1129: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18
On 09/13/2018 06:42 PM, Fabian Maurer wrote:
This is needed for the linked bug, although it doesn't fix it yet.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=43465 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/comctl32/listbox.c | 7 +++++++ dlls/comctl32/tests/listbox.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef86e0..a9d1db458b 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -91,6 +91,7 @@ typedef struct HFONT font; /* Current font */ LCID locale; /* Current locale for string comparisons */ HEADCOMBO *lphc; /* ComboLBox */ + BOOL got_wm_size; /* Did we get already a WM_SIZE? */ } LB_DESCR;
Please find a better way.
@@ -200,6 +201,10 @@ static void LISTBOX_UpdateScroll( LB_DESCR *descr ) the programmer use it for his/her own purposes. */
if (descr->style & LBS_NOREDRAW) return; + + /* When height is 0 don't update scrollbar during CreateWindowEx, that is until we receive our first WM_SIZE */ + if (!descr->got_wm_size && descr->height == 0) return; + info.cbSize = sizeof(info);
if (descr->style & LBS_MULTICOLUMN) @@ -2502,6 +2507,7 @@ static BOOL LISTBOX_Create( HWND hwnd, LPHEADCOMBO lphc ) descr->font = 0; descr->locale = GetUserDefaultLCID(); descr->lphc = lphc; + descr->got_wm_size = FALSE;
if( lphc ) { @@ -2887,6 +2893,7 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam,
case WM_SIZE: LISTBOX_UpdateSize( descr ); + descr->got_wm_size = TRUE; return 0; case WM_GETFONT: return (LRESULT)descr->font; diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index c9d13a8d67..e3a3b42c2f 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -2176,6 +2176,40 @@ static void test_WM_MEASUREITEM(void) DestroyWindow(parent); }
+static void test_WS_VSCROLL(void) +{ + HWND parent, listbox; + UINT style; + + parent = create_parent(); + + /* Listbox must always have WS_VSCROLL after creating when height 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); + SendMessageA(listbox, WM_SIZE, SIZE_RESTORED, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Listbox must not have WS_VSCROLL after creating when height > 0 was specified */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 100, parent, NULL, NULL, 0); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + DestroyWindow(listbox); + + /* Adding elements must change WS_VSCROLL flag */ + listbox = CreateWindowA("LISTBOX", "TestList", (LBS_STANDARD & ~LBS_SORT) | WS_CHILD | WS_VSCROLL, 0, 0, 100, 0, parent, NULL, NULL, 0); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) == 0, "Listbox must not have WS_VSCROLL\n"); + SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"test1"); + style = GetWindowLongW(listbox, GWL_STYLE); + ok((style & WS_VSCROLL) != 0, "Listbox must have WS_VSCROLL\n"); +
Tests are a bit artificial, because WM_SIZE is normally sent by the system.
+ DestroyWindow(parent); +} + START_TEST(listbox) { ULONG_PTR ctx_cookie; @@ -2202,6 +2236,7 @@ START_TEST(listbox) test_extents(); test_WM_MEASUREITEM(); test_LB_SETSEL(); + test_WS_VSCROLL();
unload_v6_module(ctx_cookie, hCtx); }
On Donnerstag, 13. September 2018 17:58:10 CEST Nikolay Sivov wrote:
Tests are a bit artificial, because WM_SIZE is normally sent by the system.
Yes, but I figured this is the easiest way to show that only the first WM_SIZE doesn't change WS_VSCROLL when height is 0. The second does actually update WS_VSCROLL, shouldn't matter if sent by the system or the test. Sending it directly would take out all the other variables, I think. Regards, Fabian Maurer
On Donnerstag, 13. September 2018 17:58:10 CEST Nikolay Sivov wrote:
Please find a better way.
Sorry, overlooked this. Do you know of an easy way to check if we're handling a message that was triggered by CreateWindowEx? Because that's what this is about, the WM_SIZE from CreateWindowEx causes the issue here. Maybe you know a solution I overlooked, if not, I'll have to keep looking. Regards, Fabian Maurer
On 09/14/2018 02:58 PM, Fabian Maurer wrote:
On Donnerstag, 13. September 2018 17:58:10 CEST Nikolay Sivov wrote:
Please find a better way.
Sorry, overlooked this.
Do you know of an easy way to check if we're handling a message that was triggered by CreateWindowEx? Because that's what this is about, the WM_SIZE from CreateWindowEx causes the issue here. Maybe you know a solution I overlooked, if not, I'll have to keep looking.
What is the issue exactly?
Regards,
Fabian Maurer
On Freitag, 14. September 2018 14:14:07 CEST Nikolay Sivov wrote:
What is the issue exactly?
As the tests show, a listbox with WS_VSCROLL and height 0 has the WS_VSCROLL style set after window creation. However, when LISTBOX_UpdateScroll is called, this hides the scrollbar (removing the WS_VSCROLL style) - since it's not needed when the listbox is empty. Now, CreateWindowEx sends WM_SIZE with SIZE_RESTORED, this triggers LISTBOX_UpdateSize which then calls LISTBOX_UpdateScroll. CreateWindowEx also sends WM_SETFONT which will trigger a resize - leading to the same issue. But as the tests also show, this is only relevant during CreateWindowEx. When with listbox empty and height 0, causing a WM_SIZE due to SetWindowPos causes the scrollbar to be hidden. I didn't find a better solution yet, since it indeed seems to only be problematic during initial window creation. FWIW, the issue is themed delphi programs, their comboboxes are *very* sensitive about listbox height/WS_VSCROLL style when WM_NCCALCSIZE is sent. Regards, Fabian Maurer
participants (3)
-
Fabian Maurer -
Marvin -
Nikolay Sivov