[PATCH v2 0/2] MR4140: uxtheme/tests: Add some tests for OpenThemeData() with various app/class names.
Themed Delphi applications use "explorer::listview" and "explorer::treeview": https://gitlab.com/freepascal.org/lazarus/lazarus/-/blob/main/lcl/interfaces... -- v2: uxtheme: Parse app/class name in OpenThemeData(). uxtheme/tests: Add some tests for OpenThemeData() with various app/class names. https://gitlab.winehq.org/wine/wine/-/merge_requests/4140
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/uxtheme/tests/system.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dlls/uxtheme/tests/system.c b/dlls/uxtheme/tests/system.c index 197cd86f6b6..7dcf3283172 100644 --- a/dlls/uxtheme/tests/system.c +++ b/dlls/uxtheme/tests/system.c @@ -2558,10 +2558,19 @@ static void test_GetThemeBackgroundRegion(void) static void test_theme(void) { + static const WCHAR * const app_class[] = + { + L"button", L"combobox", L"edit", L"explorerbar", L"header", + L"listview", L"explorer::listview", L"menu", L"progress", + L"rebar", L"scrollbar", L"spin", L"startpanel", L"status", + L"tab", L"taskband", L"taskbar", L"toolbar", L"tooltip", + L"trackbar", L"treeview", L"explorer::treeview", L"window" + }; BOOL transparent; HTHEME htheme; HRESULT hr; HWND hwnd; + int i; if (!IsThemeActive()) { @@ -2572,6 +2581,14 @@ static void test_theme(void) hwnd = CreateWindowA(WC_STATICA, "", WS_POPUP, 0, 0, 1, 1, 0, 0, 0, NULL); ok(!!hwnd, "CreateWindowA failed, error %#lx.\n", GetLastError()); + for (i = 0; i < ARRAY_SIZE(app_class); i++) + { + htheme = OpenThemeData(hwnd, app_class[i]); + todo_wine_if(i == 3 || i == 6 || i == 12 || i == 15 || i == 16 || i == 21) + ok(htheme != NULL, "OpenThemeData(%s) error %#lx.\n", wine_dbgstr_w(app_class[i]), GetLastError()); + CloseThemeData(htheme); + } + /* Test that scrollbar arrow parts are transparent */ htheme = OpenThemeData(hwnd, L"ScrollBar"); ok(!!htheme, "OpenThemeData failed.\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4140
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Themed Delphi applications use "explorer::listview" and "explorer::treeview": https://gitlab.com/freepascal.org/lazarus/lazarus/-/blob/main/lcl/interfaces... Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/uxtheme/system.c | 15 ++++++++++++++- dlls/uxtheme/tests/system.c | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/dlls/uxtheme/system.c b/dlls/uxtheme/system.c index 156051406e3..441ea8146df 100644 --- a/dlls/uxtheme/system.c +++ b/dlls/uxtheme/system.c @@ -640,7 +640,20 @@ static HTHEME open_theme_data(HWND hwnd, LPCWSTR pszClassList, DWORD flags, UINT pszUseClassList = pszClassList; if (pszUseClassList) - hTheme = MSSTYLES_OpenThemeClass(pszAppName, pszUseClassList, dpi); + { + const WCHAR *p = wcschr(pszUseClassList, ':'); + if (p) + { + p++; + lstrcpynW(szAppBuff, pszUseClassList, min(p - pszUseClassList, 256)); + szAppBuff[p - pszUseClassList] = 0; + if (*p == ':') p++; + lstrcpynW(szClassBuff, p, min(wcslen(p) + 1, 256)); + hTheme = MSSTYLES_OpenThemeClass(pszAppName ? pszAppName : szAppBuff, szClassBuff, dpi); + } + else + hTheme = MSSTYLES_OpenThemeClass(pszAppName, pszUseClassList, dpi); + } /* Fall back to default class if the specified subclass is not found */ if (!hTheme) diff --git a/dlls/uxtheme/tests/system.c b/dlls/uxtheme/tests/system.c index 7dcf3283172..d8e1268c2d6 100644 --- a/dlls/uxtheme/tests/system.c +++ b/dlls/uxtheme/tests/system.c @@ -2584,7 +2584,7 @@ static void test_theme(void) for (i = 0; i < ARRAY_SIZE(app_class); i++) { htheme = OpenThemeData(hwnd, app_class[i]); - todo_wine_if(i == 3 || i == 6 || i == 12 || i == 15 || i == 16 || i == 21) + todo_wine_if(i == 3 || i == 6 || i == 12 || i == 15 || i == 16) ok(htheme != NULL, "OpenThemeData(%s) error %#lx.\n", wine_dbgstr_w(app_class[i]), GetLastError()); CloseThemeData(htheme); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4140
Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/system.c:
pszUseClassList = pszClassList;
if (pszUseClassList) - hTheme = MSSTYLES_OpenThemeClass(pszAppName, pszUseClassList, dpi); + { + const WCHAR *p = wcschr(pszUseClassList, ':'); + if (p) + { + p++; + lstrcpynW(szAppBuff, pszUseClassList, min(p - pszUseClassList, 256)); + szAppBuff[p - pszUseClassList] = 0; + if (*p == ':') p++; + lstrcpynW(szClassBuff, p, min(wcslen(p) + 1, 256)); + hTheme = MSSTYLES_OpenThemeClass(pszAppName ? pszAppName : szAppBuff, szClassBuff, dpi);
I just tested on Windows 10. It turns out you can't set the application class both with SetWindowTheme() and OpenThemeData(). If you do, OpenThemeData() fails. So please add some tests for that and change the OpenThemeData() implementation accordingly. ``` SetWindowTheme(hwnd, L"explorer", NULL); theme = OpenThemeData(hwnd, L"explorer::listview"); if (!theme) printf("OpenThemeData failed.\n"); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49586
I just tested on Windows 10. It turns out you can't set the application class both with SetWindowTheme() and OpenThemeData(). If you do, OpenThemeData() fails. So please add some tests for that and change the OpenThemeData() implementation accordingly.
Probably I'm missing something, could you please elaborate how this patch is related to the described above behaviour? The only thing that the patch aims to fix is to make OpenThemeData() accept combined application/class names like "explorer::listview". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49588
On Mon Oct 23 17:07:19 2023 +0000, Dmitry Timoshkov wrote:
I just tested on Windows 10. It turns out you can't set the application class both with SetWindowTheme() and OpenThemeData(). If you do, OpenThemeData() fails. So please add some tests for that and change the OpenThemeData() implementation accordingly. Probably I'm missing something, could you please elaborate how this patch is related to the described above behaviour? The only thing that the patch aims to fix is to make OpenThemeData() accept combined application/class names like "explorer::listview". So if you call `SetWindowTheme(hwnd, L"explorer", NULL);` then call `OpenThemeData(hwnd, L"explorer::listview");`, the OpenThemeData() call should fail. So in your patch, there is no need to do checks like `pszAppName ? pszAppName : szAppBuff` because it shouldn't go that far. Instead, you should fail OpenThemeData() early if that happens, which is when an application class is already set and OpenThemeData() tries to use another application class.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49594
participants (3)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Zhiyi Zhang (＠zhiyi)