[PATCH v3 0/2] MR4140: uxtheme/tests: Add some tests for OpenThemeData() with various app/class names.

[Dmitry Timoshkov (＠dmitry)]

Themed Delphi applications use "explorer::listview" and "explorer::treeview":
https://gitlab.com/freepascal.org/lazarus/lazarus/-/blob/main/lcl/interfaces...

--
  v3: 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

[PATCH v3 1/2] uxtheme/tests: Add some tests for OpenThemeData() with various app/class names.

[Dmitry Timoshkov]

From: Dmitry Timoshkov <dmitry(a)baikal.ru>

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
 dlls/uxtheme/tests/system.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/dlls/uxtheme/tests/system.c b/dlls/uxtheme/tests/system.c
index 197cd86f6b6..d305d0bcd24 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,22 @@ 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);
+    }
+
+    hr = SetWindowTheme(hwnd, L"explorer", NULL);
+    ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
+    SetLastError(0xdeadbeef);
+    htheme = OpenThemeData(hwnd, L"explorer::listview");
+    ok(!htheme, "OpenThemeData() should fail.\n");
+    ok(GetLastError() == E_PROP_ID_UNSUPPORTED, "Got unexpected %#lx.\n", GetLastError());
+    SetWindowTheme(hwnd, NULL, NULL);
+
     /* Test that scrollbar arrow parts are transparent */
     htheme = OpenThemeData(hwnd, L"ScrollBar");
     ok(!!htheme, "OpenThemeData failed.\n");
@@ -2589,7 +2614,7 @@ static void test_theme(void)
     /* > XP use opaque scrollbar arrow parts, but TMT_TRANSPARENT is TRUE */
     else
     {
-        ok(hr == S_OK, "Got unexpected hr %#lx,\n", hr);
+        ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
         ok(transparent, "Expected transparent.\n");
 
         transparent = IsThemeBackgroundPartiallyTransparent(htheme, SBP_ARROWBTN, 0);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/4140

[PATCH v3 2/2] uxtheme: Parse app/class name in OpenThemeData().

[Dmitry Timoshkov]

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       | 18 +++++++++++++++++-
 dlls/uxtheme/tests/system.c |  2 +-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/dlls/uxtheme/system.c b/dlls/uxtheme/system.c
index 156051406e3..f913316e12d 100644
--- a/dlls/uxtheme/system.c
+++ b/dlls/uxtheme/system.c
@@ -633,13 +633,29 @@ static HTHEME open_theme_data(HWND hwnd, LPCWSTR pszClassList, DWORD flags, UINT
 
     if(bThemeActive)
     {
+        const WCHAR *p;
+
         pszAppName = UXTHEME_GetWindowProperty(hwnd, atSubAppName, szAppBuff, ARRAY_SIZE(szAppBuff));
         /* If SetWindowTheme was used on the window, that overrides the class list passed to this function */
         pszUseClassList = UXTHEME_GetWindowProperty(hwnd, atSubIdList, szClassBuff, ARRAY_SIZE(szClassBuff));
         if(!pszUseClassList)
             pszUseClassList = pszClassList;
 
-        if (pszUseClassList)
+        p = wcschr(pszClassList, ':');
+        if (p)
+        {
+            /* If the application class is already set then fail */
+            if (!pszAppName)
+            {
+                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(szAppBuff, szClassBuff, dpi);
+            }
+        }
+        else
             hTheme = MSSTYLES_OpenThemeClass(pszAppName, pszUseClassList, dpi);
 
         /* Fall back to default class if the specified subclass is not found */
diff --git a/dlls/uxtheme/tests/system.c b/dlls/uxtheme/tests/system.c
index d305d0bcd24..eddf9fbf484 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)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/tests/system.c:
> 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"

Now that I come to think of it. You don't really need to test all these parts. All you need is to test "explorer::treeview" for the OpenThemeData() function and you can move the OpenThemeData() tests to test_OpenThemeData(). test_theme() is for testing whether a part exists or part properties in the theme files. If you still want to test these extra theme parts, you can keep them in test_theme() and make them in a separate commit.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49620

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/tests/system.c:
> 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);
> +    }
> +
> +    hr = SetWindowTheme(hwnd, L"explorer", NULL);
> +    ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
> +    SetLastError(0xdeadbeef);
> +    htheme = OpenThemeData(hwnd, L"explorer::listview");

Let's use explorer::treeview here instead because Wine's light theme currently doesn't have explorer::listview.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49621

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/system.c:
> if(bThemeActive)
>      {
> +        const WCHAR *p;
> +
>          pszAppName = UXTHEME_GetWindowProperty(hwnd, atSubAppName, szAppBuff, ARRAY_SIZE(szAppBuff));
>          /* If SetWindowTheme was used on the window, that overrides the class list passed to this function */
>          pszUseClassList = UXTHEME_GetWindowProperty(hwnd, atSubIdList, szClassBuff, ARRAY_SIZE(szClassBuff));
>          if(!pszUseClassList)
>              pszUseClassList = pszClassList;
>
> -        if (pszUseClassList)
> +        p = wcschr(pszClassList, ':');
> +        if (p)
> +        {
> +            /* If the application class is already set then fail */
> +            if (!pszAppName)

This is not failing. This is just going to fallback to the default class.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49622

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/system.c:
> if(bThemeActive)
>      {
> +        const WCHAR *p;
> +
>          pszAppName = UXTHEME_GetWindowProperty(hwnd, atSubAppName, szAppBuff, ARRAY_SIZE(szAppBuff));
>          /* If SetWindowTheme was used on the window, that overrides the class list passed to this function */
>          pszUseClassList = UXTHEME_GetWindowProperty(hwnd, atSubIdList, szClassBuff, ARRAY_SIZE(szClassBuff));
>          if(!pszUseClassList)
>              pszUseClassList = pszClassList;
>
> -        if (pszUseClassList)
> +        p = wcschr(pszClassList, ':');
> +        if (p)

Let's use StrStrW to check "::" instead. It's clearer that way.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49623

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/system.c:
> /* If SetWindowTheme was used on the window, that overrides the class list passed to this function */
>          pszUseClassList = UXTHEME_GetWindowProperty(hwnd, atSubIdList, szClassBuff, ARRAY_SIZE(szClassBuff));
>          if(!pszUseClassList)
>              pszUseClassList = pszClassList;
>
> -        if (pszUseClassList)
> +        p = wcschr(pszClassList, ':');
> +        if (p)
> +        {
> +            /* If the application class is already set then fail */
> +            if (!pszAppName)
> +            {
> +                p++;
> +                lstrcpynW(szAppBuff, pszUseClassList, min(p - pszUseClassList, 256));
> +                szAppBuff[p - pszUseClassList] = 0;
> +                if (*p == ':') p++;

I am afraid this is wrong. You're only parsing the first app/class here. You should do the parsing in MSSTYLES_OpenThemeClass() instead. For example, OpenThemeData(hwnd, L"abc:def;explorer::treeview") succeeds.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49624

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/uxtheme/tests/system.c:
> +    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);
> +    }
> +
> +    hr = SetWindowTheme(hwnd, L"explorer", NULL);
> +    ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr);
> +    SetLastError(0xdeadbeef);
> +    htheme = OpenThemeData(hwnd, L"explorer::listview");
> +    ok(!htheme, "OpenThemeData() should fail.\n");
> +    ok(GetLastError() == E_PROP_ID_UNSUPPORTED, "Got unexpected %#lx.\n", GetLastError());
> +    SetWindowTheme(hwnd, NULL, NULL);

Let's test that OpenThemeData(hwnd, L"abc:def;explorer::treeview") succeeds.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4140#note_49625