[PATCH v3 0/2] MR6443: comdlg32: allow entering a filter in the itemdlg file name field
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. This was tested using https://github.com/microsoft/Windows-classic-samples/blob/main/Samples/Win7S... (MIT Licensed). There is a c_rgSaveTypes variable at the top which can be modified to various configurations (especially ones that include a COMDLG_FILTERSPEC with an empty pszSpec) -- v3: comdlg32: add tests for itemdlg file name field filters comdlg32: allow entering a filter in the itemdlg file name field https://gitlab.winehq.org/wine/wine/-/merge_requests/6443
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 | 90 +++++++++++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c index 1cdebf8f738..784a14b47e4 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; @@ -623,21 +646,22 @@ static HRESULT on_default_action(FileDialogImpl *This) /* Add the proper extension */ if(open_action == ONOPEN_OPEN) { - if(This->dlg_type == ITEMDLG_TYPE_SAVE) + WCHAR extbuf[MAX_PATH], *newext = NULL; + + if(This->current_filter) { - WCHAR extbuf[MAX_PATH], *newext = NULL; + newext = get_first_ext_from_spec(extbuf, This->current_filter); + } - if(This->filterspec_count) - { - newext = get_first_ext_from_spec(extbuf, This->filterspecs[This->filetypeindex].pszSpec); - } - else if(This->default_ext) - { - lstrcpyW(extbuf, L"."); - lstrcatW(extbuf, This->default_ext); - newext = extbuf; - } + if(!newext && This->default_ext) + { + lstrcpyW(extbuf, L"."); + lstrcatW(extbuf, This->default_ext); + newext = extbuf; + } + if(This->dlg_type == ITEMDLG_TYPE_SAVE) + { if(newext) { WCHAR *ext = PathFindExtensionW(canon_filename); @@ -650,10 +674,9 @@ static HRESULT on_default_action(FileDialogImpl *This) if( !(This->options & FOS_NOVALIDATE) && (This->options & FOS_FILEMUSTEXIST) && !PathFileExistsW(canon_filename)) { - if(This->default_ext) + if(newext) { - lstrcatW(canon_filename, L"."); - lstrcatW(canon_filename, This->default_ext); + lstrcatW(canon_filename, newext); if(!PathFileExistsW(canon_filename)) { @@ -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); @@ -2519,6 +2545,8 @@ static HRESULT WINAPI IFileDialog2_fnSetFileTypes(IFileDialog2 *iface, UINT cFil } This->filterspec_count = cFileTypes; + set_current_filter(This, This->filterspecs[This->filetypeindex].pszSpec); + return S_OK; } @@ -3651,7 +3679,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 +3712,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
From: Charlotte Pabst <cpabst(a)codeweavers.com> Signed-off-by: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/comdlg32/tests/itemdlg.c | 57 +++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/dlls/comdlg32/tests/itemdlg.c b/dlls/comdlg32/tests/itemdlg.c index 573a267a37e..b59ce62db2c 100644 --- a/dlls/comdlg32/tests/itemdlg.c +++ b/dlls/comdlg32/tests/itemdlg.c @@ -1328,7 +1328,7 @@ static void touch_file(LPCWSTR filename) CloseHandle(file); } -static void test_filename_savedlg_(LPCWSTR set_filename, LPCWSTR defext, +static void test_filename_savedlg_(LPCWSTR set_filename, LPCWSTR set_filename2, LPCWSTR defext, const COMDLG_FILTERSPEC *filterspec, UINT fs_count, LPCWSTR exp_filename, const char *file, int line) { @@ -1357,6 +1357,12 @@ static void test_filename_savedlg_(LPCWSTR set_filename, LPCWSTR defext, ok_(file,line)(hr == S_OK, "SetDefaultExtensions failed: Got 0x%08lx\n", hr); } + if(set_filename2) + { + hr = IFileSaveDialog_SetFileName(pfsd, set_filename2); + ok_(file,line)(hr == S_OK, "SetFileName failed: Got 0x%08lx\n", hr); + } + pfde = IFileDialogEvents_Constructor(); pfdeimpl = impl_from_IFileDialogEvents(pfde); pfdeimpl->set_filename = set_filename; @@ -1388,10 +1394,11 @@ static void test_filename_savedlg_(LPCWSTR set_filename, LPCWSTR defext, IFileDialogEvents_Release(pfde); } -#define test_filename_savedlg(set_filename, defext, filterspec, fs_count, exp_filename) \ - test_filename_savedlg_(set_filename, defext, filterspec, fs_count, exp_filename, __FILE__, __LINE__) +#define test_filename_savedlg(set_filename, set_filename2, defext, filterspec, fs_count, exp_filename) \ + test_filename_savedlg_(set_filename, set_filename2, defext, filterspec, fs_count, exp_filename, __FILE__, __LINE__) -static void test_filename_opendlg_(LPCWSTR set_filename, IShellItem *psi_current, LPCWSTR defext, +static void test_filename_opendlg_(LPCWSTR set_filename, LPCWSTR set_filename2, + IShellItem *psi_current, LPCWSTR defext, const COMDLG_FILTERSPEC *filterspec, UINT fs_count, LPCWSTR exp_filename, const char *file, int line) { @@ -1421,6 +1428,12 @@ static void test_filename_opendlg_(LPCWSTR set_filename, IShellItem *psi_current ok_(file,line)(hr == S_OK, "SetFileTypes failed: Got 0x%08lx\n", hr); } + if(set_filename2) + { + hr = IFileOpenDialog_SetFileName(pfod, set_filename2); + ok_(file,line)(hr == S_OK, "SetFileName failed: Got 0x%08lx\n", hr); + } + hr = IFileOpenDialog_SetFolder(pfod, psi_current); ok_(file,line)(hr == S_OK, "SetFolder failed: Got 0x%08lx\n", hr); @@ -1481,8 +1494,8 @@ static void test_filename_opendlg_(LPCWSTR set_filename, IShellItem *psi_current IFileDialogEvents_Release(pfde); } -#define test_filename_opendlg(set_filename, psi, defext, filterspec, fs_count, exp_filename) \ - test_filename_opendlg_(set_filename, psi, defext, filterspec, fs_count, exp_filename, __FILE__, __LINE__) +#define test_filename_opendlg(set_filename, set_filename2, psi, defext, filterspec, fs_count, exp_filename) \ + test_filename_opendlg_(set_filename, set_filename2, psi, defext, filterspec, fs_count, exp_filename, __FILE__, __LINE__) static void test_filename(void) { @@ -1516,23 +1529,25 @@ static void test_filename(void) }; /* No extension */ - test_filename_savedlg(filename_noextW, NULL, NULL, 0, filename_noextW); + test_filename_savedlg(filename_noextW, NULL, NULL, NULL, 0, filename_noextW); /* Default extension */ - test_filename_savedlg(filename_noextW, defextW, NULL, 0, filename_defextW); + test_filename_savedlg(filename_noextW, NULL, defextW, NULL, 0, filename_defextW); /* Default extension on filename ending with a . */ - test_filename_savedlg(filename_dotextW, defextW, NULL, 0, filename_dotanddefW); + test_filename_savedlg(filename_dotextW, NULL, defextW, NULL, 0, filename_dotanddefW); /* Default extension on filename with default extension */ - test_filename_savedlg(filename_defextW, defextW, NULL, 0, filename_defextW); + test_filename_savedlg(filename_defextW, NULL, defextW, NULL, 0, filename_defextW); /* Default extension on filename with another extension */ - test_filename_savedlg(filename_ext1W, defextW, NULL, 0, filename_ext1anddefW); + test_filename_savedlg(filename_ext1W, NULL, defextW, NULL, 0, filename_ext1anddefW); /* Default extension, filterspec without default extension */ - test_filename_savedlg(filename_noextW, defextW, filterspec, 2, filename_ext1W); + test_filename_savedlg(filename_noextW, NULL, defextW, filterspec, 2, filename_ext1W); /* Default extension, filterspec with default extension */ - test_filename_savedlg(filename_noextW, defextW, filterspec, 3, filename_ext1W); + test_filename_savedlg(filename_noextW, NULL, defextW, filterspec, 3, filename_ext1W); /* Default extension, filterspec with "complex" extension */ - test_filename_savedlg(filename_noextW, defextW, filterspec2, 1, filename_ext2W); + test_filename_savedlg(filename_noextW, NULL, defextW, filterspec2, 1, filename_ext2W); /* Default extension, filterspec with extension that differs in case from the specified filename */ - test_filename_savedlg(filename_mixedcaseW, defextW, filterspec, 0, filename_mixedcaseW); + test_filename_savedlg(filename_mixedcaseW, NULL, defextW, filterspec, 0, filename_mixedcaseW); + /* Default extension, filterspec with default extension, filename filter */ + test_filename_savedlg(filename_noextW, ext2, defextW, filterspec, 3, filename_ext2W); GetCurrentDirectoryW(MAX_PATH, buf); ok(!!pSHCreateItemFromParsingName, "SHCreateItemFromParsingName is missing.\n"); @@ -1544,17 +1559,21 @@ static void test_filename(void) touch_file(filename_ext2W); /* IFileOpenDialog, default extension */ - test_filename_opendlg(filename_noextW, psi_current, defextW, NULL, 0, filename_noextW); + test_filename_opendlg(filename_noextW, NULL, psi_current, defextW, NULL, 0, filename_noextW); /* IFileOpenDialog, default extension and filterspec */ - test_filename_opendlg(filename_noextW, psi_current, defextW, filterspec, 2, filename_noextW); + test_filename_opendlg(filename_noextW, NULL, psi_current, defextW, filterspec, 2, filename_noextW); + /* IFileOpenDialog, default extension, filterspec and filename filter */ + test_filename_opendlg(filename_noextW, ext2, psi_current, defextW, filterspec, 2, filename_noextW); DeleteFileW(filename_noextW); /* IFileOpenDialog, default extension, noextW deleted */ - test_filename_opendlg(filename_noextW, psi_current, defextW, NULL, 0, filename_defextW); + test_filename_opendlg(filename_noextW, NULL, psi_current, defextW, NULL, 0, filename_defextW); + /* IFileOpenDialog, default extension and filename filter, noextW deleted */ + test_filename_opendlg(filename_noextW, ext2, psi_current, defextW, NULL, 0, filename_ext2W); if(0) /* Interactive */ { /* IFileOpenDialog, filterspec, no default extension, noextW deleted */ - test_filename_opendlg(filename_noextW, psi_current, NULL, filterspec, 2, NULL); + test_filename_opendlg(filename_noextW, NULL, psi_current, NULL, filterspec, 2, NULL); } IShellItem_Release(psi_current); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6443
Added tests. The tests revealed that the current_filter file extension adding logic wasn't being done for open dialogs yet, so I fixed that as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6443#note_81660
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6443
participants (3)
-
Charlotte Pabst -
Charlotte Pabst (＠CharlottePabst)
-
Esme Povirk (＠madewokherd)