From a54dd7ba457fc6780ff460dc4eccccb5df4b3890 Mon Sep 17 00:00:00 2001 From: Semphris Date: Wed, 11 Mar 2026 17:57:08 -0400 Subject: [PATCH] Fix Windows file dialog calling the callback twice If the modern implementation of file dialogs on Windows fails, it will invoke the callback on error and report an error, then SDL would attempt invoking the dialog again using an older implementation, which would call the callback again. This is not desired behavior, so it has been changed as follows: - If the modern dialog fails before showing (missing library/symbol), don't call the callback and try the older dialogs in case we're on an old platform. - If the dialog fails while or after showing, assume the error is a normal invocation error (such as an invalid path), call the callback and don't show the older dialogs to prevent showing two dialogs in succession to the user. --- src/dialog/windows/SDL_windowsdialog.c | 27 ++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/dialog/windows/SDL_windowsdialog.c b/src/dialog/windows/SDL_windowsdialog.c index 631fff5c18..400fcf03ad 100644 --- a/src/dialog/windows/SDL_windowsdialog.c +++ b/src/dialog/windows/SDL_windowsdialog.c @@ -456,6 +456,9 @@ char *clear_filt_names(const char *filt) return cleared; } +// This function returns NOT success or error, but rather whether the callback +// was invoked or not (and if it was, no fallback should be attempted to prevent +// calling the callback twice). See https://github.com/libsdl-org/SDL/issues/15194 bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const char *default_file, SDL_Window *parent, bool allow_many, SDL_DialogFileCallback callback, void *userdata, const char *title, const char *accept, const char *cancel, wchar_t *filter_wchar, int nfilters) { bool is_save = dialog_type == SDL_FILEDIALOG_SAVEFILE; @@ -488,7 +491,8 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch wchar_t *default_file_w = NULL; wchar_t *default_folder_w = NULL; - bool success = false; + bool callback_called = false; + bool call_callback_on_error = false; bool co_init = false; // We can assume shell32 is already loaded here. @@ -604,6 +608,10 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch CHECK(pFileDialog->lpVtbl->SetFileName(pFileDialog, default_file_w)); } + // Right after this, a dialog is shown. No fallback should be attempted on + // error to prevent showing two dialogs to the user. + call_callback_on_error = true; + if (parent) { HWND window = (HWND) SDL_GetPointerProperty(SDL_GetWindowProperties(parent), SDL_PROP_WINDOW_WIN32_HWND_POINTER, NULL); @@ -616,7 +624,7 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch // This is a one-based index, not zero-based. Doc link in similar comment below CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter)); callback(userdata, results, selected_filter - 1); - success = true; + callback_called = true; goto quit; } else if (!SUCCEEDED(hr)) { goto quit; @@ -631,7 +639,7 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch // This is a one-based index, not zero-based. Doc link in similar comment below CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter)); callback(userdata, results, selected_filter - 1); - success = true; + callback_called = true; goto quit; } else if (!SUCCEEDED(hr)) { goto quit; @@ -665,7 +673,7 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch } callback(userdata, (const char * const *) files, selected_filter - 1); - success = true; + callback_called = true; } else { // This is a one-based index, not zero-based. // https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ifiledialog-getfiletypeindex#parameters @@ -681,18 +689,17 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch } const char * const results[] = { file, NULL }; callback(userdata, results, selected_filter - 1); - success = true; + callback_called = true; SDL_free(file); } - success = true; - #undef CHECK quit: - if (!success) { - WIN_SetError("dialogg"); + if (!callback_called && call_callback_on_error) { + WIN_SetError("dialog"); callback(userdata, NULL, -1); + callback_called = true; } if (co_init) { @@ -750,7 +757,7 @@ quit: SDL_free(files); } - return success; + return callback_called; } // TODO: The new version of file dialogs