[PATCH v5 0/2] MR8608: comdlg32: Display filter specs in itemdlg File Type combo box

[Maotong Zhang (＠xiaotong)]

Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=57113

--
  v5: comdlg32: Display filter specs in itemdlg File Type combo box

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

[PATCH v5 1/2] comdlg32/tests: Fix file type combo box selection in file dialogs

[Maotong Zhang]

From: Maotong Zhang

Verified on Windows 10: the string (*.txt) is required to find the combo box
---
 dlls/comdlg32/tests/itemdlg.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/dlls/comdlg32/tests/itemdlg.c b/dlls/comdlg32/tests/itemdlg.c
index 7ab8fa8f148..f1d0a62525e 100644
--- a/dlls/comdlg32/tests/itemdlg.c
+++ b/dlls/comdlg32/tests/itemdlg.c
@@ -1165,15 +1165,11 @@ static void filedialog_change_filetype(IFileDialog *pfd, HWND dlg_hwnd)
     const WCHAR filetype1_broken[] = {'f','n','a','m','e','1',' ', '(','*','.','t','x','t',')',0};

cb_filetype = find_window(dlg_hwnd, NULL, filetype1);
-    ok(cb_filetype != NULL || broken(cb_filetype == NULL), "Could not find combobox on first attempt\n");
-
     if(!cb_filetype)
     {
-        /* Not sure when this happens. Some specific version?
-         * Seen on 32-bit English Vista */
-        trace("Didn't find combobox on first attempt, trying broken string..\n");
+        /* Verified on Windows 10: the string "(*.txt)" is required to find the combobox. */
         cb_filetype = find_window(dlg_hwnd, NULL, filetype1_broken);
-        ok(broken(cb_filetype != NULL), "Failed to find combobox on second attempt\n");
+        ok(cb_filetype != NULL, "Failed to find combobox.\n");
         if(!cb_filetype)
             return;
     }

[PATCH v5 2/2] comdlg32: Display filter specs in itemdlg File Type combo box

[Maotong Zhang]

From: Maotong Zhang

Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=57113
---
 dlls/comdlg32/itemdlg.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c
index 99af524a06f..521886e5372 100644
--- a/dlls/comdlg32/itemdlg.c
+++ b/dlls/comdlg32/itemdlg.c
@@ -2549,6 +2549,26 @@ static HRESULT WINAPI IFileDialog2_fnSetFileTypes(IFileDialog2 *iface, UINT cFil
     {
         This->filterspecs[i].pszName = StrDupW(rgFilterSpec[i].pszName);
         This->filterspecs[i].pszSpec = StrDupW(rgFilterSpec[i].pszSpec);
+        
+        if (This->filterspecs[i].pszName != NULL && This->filterspecs[i].pszSpec != NULL)
+        {
+            DWORD name_len = lstrlenW(This->filterspecs[i].pszName);
+    
+            if (name_len == 0 || This->filterspecs[i].pszName[name_len - 1] != L')')
+            {
+                DWORD spec_len = lstrlenW(This->filterspecs[i].pszSpec);
+
+                DWORD total_len = name_len + spec_len + 4;
+    
+                WCHAR* pszName = LocalAlloc(LMEM_FIXED, total_len * sizeof(WCHAR));
+                if (pszName != NULL)
+                {
+                    swprintf(pszName, total_len, L"%s (%s)", This->filterspecs[i].pszName, This->filterspecs[i].pszSpec);
+                    LocalFree(This->filterspecs[i].pszName);
+                    This->filterspecs[i].pszName = pszName;
+                }
+            }
+        } 
     }
     This->filterspec_count = cFileTypes;

[Maotong Zhang (＠xiaotong)]

On Mon Jul 28 14:02:00 2025 +0000, Esme Povirk wrote:
> Ah, we may have had some language difficulties. I only meant that the
> `(void*)` was unnecessary. I also found, since making that comment, that
> a different set of allocation/free functions are used with this variable.
> I think the value should be allocated like this:
> `WCHAR* pszName = LocalAlloc(LMEM_FIXED, total_len * sizeof(WCHAR));`
> And the older value should be freed like this:
> `LocalFree(This->filterspecs[i].pszName);`
> I found this by looking for the definition of `StrDupW`, which is in `dlls/kernelbase/string.c`.

Sorry, I misunderstood.

[Maotong Zhang (＠xiaotong)]

On Mon Jul 28 15:12:33 2025 +0000, Esme Povirk wrote:
> This leaks the value previously assigned to
> `This->filterspecs[i].pszName`. Also, elsewhere in the file, this
> variable is freed with `LocalFree` so I think we have to do the
> allocation with `LocalAlloc`.

This->filterspecs uses HeapAlloc(), and StrDupW() uses LocalAlloc().
This is really confusing, so it's better to choose LocalAlloc() and LocalFree()