[PATCH 0/1] MR775: comctl32: Support passing bitmap and icon resource ID as a string when creating static control.
Follow Up of Merge request !693 by Jacek Caban. That pull request changed properly the static control of user32, but didn't address the comctl32 static control. Fix an issue of TES4:Oblivion Construction Set, reported by me in the Wine Bug: https://bugs.winehq.org/show_bug.cgi?id=53581#c5 after the merge request fixed the reported CS crash. Signed-off-by: Lorenzo Ferrillo [lorenzofersteam(a)live.it](mailto:lorenzofersteam(a)live.it) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/775
From: Lorenzo Ferrillo <lorenzofersteam(a)live.it> Follow Up of Merge request #693 by Jacek Caban. The pull request changed properly the static control of user32, but didn't address the comctl32 static control. Fix an issue of TES4:Oblivion Construction Set, reported by me in Wine Bug: https://bugs.winehq.org/show_bug.cgi?id=53581#c5 after the merge request fixed the reported CS crash. --- dlls/comctl32/static.c | 10 ++++++++-- dlls/comctl32/tests/static.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/static.c b/dlls/comctl32/static.c index c09655da903..2df542426dd 100644 --- a/dlls/comctl32/static.c +++ b/dlls/comctl32/static.c @@ -437,6 +437,7 @@ static BOOL hasTextStyle( DWORD style ) static LRESULT CALLBACK STATIC_WindowProc( HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam ) { + const WCHAR* window_name = NULL; LRESULT lResult = 0; LONG full_style = GetWindowLongW( hwnd, GWL_STYLE ); LONG style = full_style & SS_TYPEMASK; @@ -549,13 +550,18 @@ static LRESULT CALLBACK STATIC_WindowProc( HWND hwnd, UINT uMsg, WPARAM wParam, SetWindowPos(hwnd, NULL, 0, 0, rc.right - rc.left, rc.bottom - rc.top, SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOZORDER); } + if(cs->lpszName && cs->lpszName[0] == 0xffff) /*String is resource index */ + window_name = MAKEINTRESOURCEW(cs->lpszName[1]); + else + window_name = cs->lpszName; + switch (style) { case SS_ICON: { HICON hIcon; - hIcon = STATIC_LoadIconW(cs->hInstance, cs->lpszName, full_style); + hIcon = STATIC_LoadIconW(cs->hInstance, window_name, full_style); STATIC_SetIcon(hwnd, hIcon, full_style); } break; @@ -563,7 +569,7 @@ static LRESULT CALLBACK STATIC_WindowProc( HWND hwnd, UINT uMsg, WPARAM wParam, if ((ULONG_PTR)cs->hInstance >> 16) { HBITMAP hBitmap; - hBitmap = LoadBitmapW(cs->hInstance, cs->lpszName); + hBitmap = LoadBitmapW(cs->hInstance, window_name); STATIC_SetBitmap(hwnd, hBitmap, full_style); } break; diff --git a/dlls/comctl32/tests/static.c b/dlls/comctl32/tests/static.c index 756fed86fe0..b0debfee89d 100644 --- a/dlls/comctl32/tests/static.c +++ b/dlls/comctl32/tests/static.c @@ -216,6 +216,8 @@ static void test_image(HBITMAP image, BOOL is_dib, BOOL is_premult, BOOL is_alph static void test_set_image(void) { + char resource[4]; + WCHAR resource_unicode[3]; HWND hwnd = create_static(SS_BITMAP); HBITMAP bmp1, bmp2, image; @@ -270,6 +272,33 @@ static void test_set_image(void) test_image(image, TRUE, FALSE, FALSE); + resource[0] = '\xff'; + resource[1] = PtrToUlong(MAKEINTRESOURCEW(IDB_BITMAP_1x1_32BPP)); + resource[2] = PtrToUlong(MAKEINTRESOURCEW(IDB_BITMAP_1x1_32BPP)) >> 8; + resource[3] = 0; + + resource_unicode[0] = 0xffff; + resource_unicode[1] = PtrToUlong(MAKEINTRESOURCEW(IDB_BITMAP_1x1_32BPP)); + resource_unicode[2] = 0; + + hwnd = CreateWindowW(L"Static", resource_unicode, WS_VISIBLE|WS_CHILD|SS_BITMAP, 5, 5, 100, 100, + hMainWnd, (HMENU)CTRL_ID, GetModuleHandleW(NULL), 0); + + bmp1 = (HBITMAP)SendMessageW(hwnd, STM_GETIMAGE, IMAGE_BITMAP, 0); + ok(bmp1 != NULL, "got NULL\n"); + ok(bmp1 != image, "bmp == image\n"); + test_image(bmp1, TRUE, TRUE, TRUE); + DestroyWindow(hwnd); + + hwnd = CreateWindowA("Static", resource, WS_VISIBLE|WS_CHILD|SS_BITMAP, 5, 5, 100, 100, + hMainWnd, (HMENU)CTRL_ID, GetModuleHandleW(NULL), 0); + + bmp1 = (HBITMAP)SendMessageA(hwnd, STM_GETIMAGE, IMAGE_BITMAP, 0); + ok(bmp1 != NULL, "got NULL\n"); + ok(bmp1 != image, "bmp == image\n"); + test_image(bmp1, TRUE, TRUE, TRUE); + DestroyWindow(hwnd); + DeleteObject(image); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/775
A quick test shows that window text is not set to "\uffff..." in this case, but is instead empty. Not sure if this applies to user32 variant as well, but it's worth fixing. Also please remove whitespace-only lines in tests changes, and add a space between "if" and opening parentheses. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/775#note_7614
@nsivov do you mean that the name isn't assigned properly to the window when the name start with 0xffff (so it should be assigned but it isn't) or that it should not be assigned at all? I'm sorry, this may be just a language barrier, but it isn't clear that much to me. Where I can look at that? In !693 there is a test that create a window, assign a custom win procedure that copy the name to a buffer, and check the buffer for equality and they match. However the test use DialogBoxParam and I'm not sure where the window with the custom procedure is called (I'm new to this part of the code if we exclude a previous analysis for a regression in win32u). Will be fine in case if the second part is addressed in a separate commit? Regarding whitespace lines, what are the guidelines? I did see other commits that modify tests, and they do add sometimes whitespace only lines in the code they add. The code I added in the test isn't different from the style used in the !693 tests. As I based the changes to other commits, and there aren't clear giudelines about this in https://wiki.winehq.org/Wine_Developer%27s_Guide/Coding_Practice#Some_notes_... and https://wiki.winehq.org/Submitting_Patches#Code_guidelines I'd like to know if there are informal preferred rules in place. Thanks for the help. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/775#note_7616
@nsivov do you mean that the name isn't assigned properly to the window when the name start with 0xffff (so it should be assigned but it isn't) or that it should not be assigned at all?
I meant it should not be set at all. That's what happens on Windows, both for user32 and comctl32.
Will be fine in case if the second part is addressed in a separate commit?
Yes, it could be addressed separately.
Regarding whitespace lines, what are the guidelines?
"git am" will warn about those, at least it does for me. It's not about empty lines (with only a newline character), but about lines that end in whitespaces. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/775#note_7625
participants (3)
-
Lorenzo Ferrillo -
Lorenzo Ferrillo (＠llde)
-
Nikolay Sivov (＠nsivov)