[PATCH 0/1] MR6443: comdlg32: allow entering a filter in the itemdlg file name field

[Charlotte Pabst (＠CharlottePabst)]

This fixes a bug in EastMoney, which calls SetFileTypes() with a COMDLG_FILTERSPEC structure in which pszSpec = L"" followed by SetFileName(L"*.fml"). In windows, this matches all *.fml files. In wine, this matched no files. Before this patch, wine did not use filters from the file name field (marked as a FIXME) and a file type entry with an empty spec would match no files (as opposed to leaving the current filter unmodified).

The exact rules worked out by testing in a Windows 10 VM seem to be:

Current filter is initially empty and can be set by typing a filter
into the file name field (or SetFileName) or selecting an entry from the file type menu (or SetFileTypeIndex).

Selecting a file type entry with an empty spec leaves the current filter unchanged.

Selecting a file type entry with a non-empty spec clears the file name field if it currently contains a filter.

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

[PATCH 1/1] comdlg32: allow entering a filter in the itemdlg file name field

[Charlotte Pabst]

From: Charlotte Pabst <cpabst(a)codeweavers.com>

current filter is initially empty and can be set by typing a filter
into the file name field or selecting an entry from the file type menu.

selecting a file type entry with an empty spec leaves the current filter unchanged.

this behavior matches windows more closely and fixes applications
that rely on SetFileName() for filtering.

Signed-off-by: Charlotte Pabst <cpabst(a)codeweavers.com>
---
 dlls/comdlg32/itemdlg.c | 62 +++++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c
index 1cdebf8f738..4ad852d8b4d 100644
--- a/dlls/comdlg32/itemdlg.c
+++ b/dlls/comdlg32/itemdlg.c
@@ -129,6 +129,7 @@ typedef struct FileDialogImpl {
     LPWSTR custom_okbutton;
     LPWSTR custom_cancelbutton;
     LPWSTR custom_filenamelabel;
+    LPWSTR current_filter;
 
     UINT cctrl_width, cctrl_def_height, cctrls_cols;
     UINT cctrl_indent, dpi_x, dpi_y;
@@ -449,6 +450,25 @@ static UINT get_file_name(FileDialogImpl *This, LPWSTR *str)
     return len;
 }
 
+static void set_current_filter(FileDialogImpl *This, LPCWSTR str)
+{
+    IShellView *psv;
+
+    if(str && !str[0])
+        return;
+
+    if(This->current_filter)
+        LocalFree(This->current_filter);
+
+    This->current_filter = str ? StrDupW(str) : NULL;
+
+    if (This->peb && SUCCEEDED(IExplorerBrowser_GetCurrentView(This->peb, &IID_IShellView, (void**)&psv)))
+    {
+        IShellView_Refresh(psv);
+        IShellView_Release(psv);
+    }
+}
+
 static BOOL set_file_name(FileDialogImpl *This, LPCWSTR str)
 {
     if(This->set_filename)
@@ -456,6 +476,9 @@ static BOOL set_file_name(FileDialogImpl *This, LPCWSTR str)
 
     This->set_filename = str ? StrDupW(str) : NULL;
 
+    if (str && wcspbrk(str, L"*?"))
+        set_current_filter(This, str);
+
     return SetDlgItemTextW(This->dlg_hwnd, IDC_FILENAME, This->set_filename);
 }
 
@@ -574,7 +597,7 @@ static HRESULT on_default_action(FileDialogImpl *This)
     IShellFolder *psf_parent, *psf_desktop;
     LPITEMIDLIST *pidla;
     LPITEMIDLIST current_folder;
-    LPWSTR fn_iter, files = NULL, tmp_files;
+    LPWSTR fn_iter, files = NULL, tmp_files, filter = NULL;
     UINT file_count = 0, len, i;
     int open_action;
     HRESULT hr, ret = E_FAIL;
@@ -627,9 +650,9 @@ static HRESULT on_default_action(FileDialogImpl *This)
             {
                 WCHAR extbuf[MAX_PATH], *newext = NULL;
 
-                if(This->filterspec_count)
+                if(This->current_filter)
                 {
-                    newext = get_first_ext_from_spec(extbuf, This->filterspecs[This->filetypeindex].pszSpec);
+                    newext = get_first_ext_from_spec(extbuf, This->current_filter);
                 }
                 else if(This->default_ext)
                 {
@@ -668,6 +691,8 @@ static HRESULT on_default_action(FileDialogImpl *This)
                     }
                 }
             }
+        } else if (open_action == ONOPEN_SEARCH) {
+            filter = fn_iter;
         }
 
         pidla[i] = SHSimpleIDListFromPath(canon_filename);
@@ -678,8 +703,6 @@ static HRESULT on_default_action(FileDialogImpl *This)
         fn_iter += (WCHAR)lstrlenW(fn_iter) + 1;
     }
 
-    HeapFree(GetProcessHeap(), 0, files);
-    ILFree(current_folder);
 
     if((This->options & FOS_PICKFOLDERS) && open_action == ONOPEN_BROWSE)
         open_action = ONOPEN_OPEN; /* FIXME: Multiple folders? */
@@ -687,7 +710,7 @@ static HRESULT on_default_action(FileDialogImpl *This)
     switch(open_action)
     {
     case ONOPEN_SEARCH:
-        FIXME("Filtering not implemented.\n");
+        set_current_filter(This, filter);
         break;
 
     case ONOPEN_BROWSE:
@@ -764,6 +787,8 @@ static HRESULT on_default_action(FileDialogImpl *This)
     }
 
     /* Clean up */
+    HeapFree(GetProcessHeap(), 0, files);
+    ILFree(current_folder);
     for(i = 0; i < file_count; i++)
         ILFree(pidla[i]);
     HeapFree(GetProcessHeap(), 0, pidla);
@@ -2248,9 +2273,7 @@ static LRESULT on_command_filetype(FileDialogImpl *This, WPARAM wparam, LPARAM l
 {
     if(HIWORD(wparam) == CBN_SELCHANGE)
     {
-        IShellView *psv;
-        HRESULT hr;
-        LPWSTR filename;
+        LPWSTR filename = NULL;
         UINT prev_index = This->filetypeindex;
 
         This->filetypeindex = SendMessageW((HWND)lparam, CB_GETCURSEL, 0, 0);
@@ -2259,14 +2282,15 @@ static LRESULT on_command_filetype(FileDialogImpl *This, WPARAM wparam, LPARAM l
         if(prev_index == This->filetypeindex)
             return FALSE;
 
-        hr = IExplorerBrowser_GetCurrentView(This->peb, &IID_IShellView, (void**)&psv);
-        if(SUCCEEDED(hr))
+        set_current_filter(This, This->filterspecs[This->filetypeindex].pszSpec);
+
+        get_file_name(This, &filename);
+
+        if(filename && wcspbrk(filename, L"*?") != NULL && This->filterspecs[This->filetypeindex].pszSpec[0])
         {
-            IShellView_Refresh(psv);
-            IShellView_Release(psv);
+            set_file_name(This, L"");
         }
-
-        if(This->dlg_type == ITEMDLG_TYPE_SAVE && get_file_name(This, &filename))
+        else if(filename && This->dlg_type == ITEMDLG_TYPE_SAVE)
         {
             WCHAR buf[MAX_PATH], extbuf[MAX_PATH], *ext;
 
@@ -2281,9 +2305,10 @@ static LRESULT on_command_filetype(FileDialogImpl *This, WPARAM wparam, LPARAM l
                 lstrcatW(buf, ext);
                 set_file_name(This, buf);
             }
-            CoTaskMemFree(filename);
         }
 
+        CoTaskMemFree(filename);
+
         /* The documentation claims that OnTypeChange is called only
          * when the dialog is opened, but this is obviously not the
          * case. */
@@ -2475,6 +2500,7 @@ static ULONG WINAPI IFileDialog2_fnRelease(IFileDialog2 *iface)
         LocalFree(This->custom_okbutton);
         LocalFree(This->custom_cancelbutton);
         LocalFree(This->custom_filenamelabel);
+        LocalFree(This->current_filter);
 
         DestroyMenu(This->hmenu_opendropdown);
         DeleteObject(This->hfont_opendropdown);
@@ -3651,7 +3677,7 @@ static HRESULT WINAPI ICommDlgBrowser3_fnIncludeObject(ICommDlgBrowser3 *iface,
     ULONG attr;
     TRACE("%p (%p, %p)\n", This, shv, pidl);
 
-    if(!This->filterspec_count && !(This->options & FOS_PICKFOLDERS))
+    if(!This->current_filter && !(This->options & FOS_PICKFOLDERS))
         return S_OK;
 
     hr = SHGetIDListFromObject((IUnknown*)shv, &parent_pidl);
@@ -3684,7 +3710,7 @@ static HRESULT WINAPI ICommDlgBrowser3_fnIncludeObject(ICommDlgBrowser3 *iface,
     hr = S_OK;
     if(SUCCEEDED(IShellItem_GetDisplayName(psi, SIGDN_PARENTRELATIVEPARSING, &filename)))
     {
-        if(!PathMatchSpecW(filename, This->filterspecs[This->filetypeindex].pszSpec))
+        if(!PathMatchSpecW(filename, This->current_filter))
             hr = S_FALSE;
         CoTaskMemFree(filename);
     }
-- 
GitLab

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

[PATCH 1/1] comdlg32: allow entering a filter in the itemdlg file name field

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=148217

Your paranoid android.


=== debian11 (32 bit report) ===

comdlg32:
itemdlg.c:1529: Test failed: (GetDisplayName) Got L"winetest.wte"
itemdlg.c:1531: Test failed: (GetDisplayName) Got L"winetest.wte"
itemdlg.c:1533: Test failed: (GetDisplayName) Got L"winetest.wte"

=== debian11b (64 bit WoW report) ===

comdlg32:
itemdlg.c:1529: Test failed: (GetDisplayName) Got L"winetest.wte"
itemdlg.c:1531: Test failed: (GetDisplayName) Got L"winetest.wte"
itemdlg.c:1533: Test failed: (GetDisplayName) Got L"winetest.wte"

user32:
input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000020D00D6, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/comdlg32/itemdlg.c:
> {
>                  WCHAR extbuf[MAX_PATH], *newext = NULL;
>
> -                if(This->filterspec_count)
> +                if(This->current_filter)
>                  {
> -                    newext = get_first_ext_from_spec(extbuf, This->filterspecs[This->filetypeindex].pszSpec);
> +                    newext = get_first_ext_from_spec(extbuf, This->current_filter);

Will it really add an extension from a manually-typed filter?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81299

[Esme Povirk (＠madewokherd)]

Seems like this broke some tests in the CI: `comdlg32:itemdlg:032c done (3) in 3s 1935B`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81300

[Esme Povirk (＠madewokherd)]

Some tests for this would be nice as well, so that it doesn't have to be manually verified.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81301

[Charlotte Pabst (＠CharlottePabst)]

On Thu Sep  5 22:12:36 2024 +0000, Esme Povirk wrote:
> Will it really add an extension from a manually-typed filter?
> yes, it does, i tested that

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81346

[Charlotte Pabst (＠CharlottePabst)]

On Fri Sep  6 12:41:49 2024 +0000, Charlotte Pabst wrote:
> yes, it does, i tested that
> i think what's relevant to know is that windows hides file extensions from you by default. so generally, that any file you can see you are be able to address via its name without the extension (because you usually can't even see the extension)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81348