Procházet zdrojové kódy

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.

(cherry picked from commit a54dd7ba457fc6780ff460dc4eccccb5df4b3890)
Semphris před 2 týdny
rodič
revize
c546c5d335
1 změnil soubory, kde provedl 17 přidání a 10 odebrání
  1. 17 10
      src/dialog/windows/SDL_windowsdialog.c

+ 17 - 10
src/dialog/windows/SDL_windowsdialog.c

@@ -456,6 +456,9 @@ char *clear_filt_names(const char *filt)
     return cleared;
     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 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;
     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_file_w = NULL;
     wchar_t *default_folder_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;
     bool co_init = false;
 
 
     // We can assume shell32 is already loaded here.
     // 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));
         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) {
     if (parent) {
         HWND window = (HWND) SDL_GetPointerProperty(SDL_GetWindowProperties(parent), SDL_PROP_WINDOW_WIN32_HWND_POINTER, NULL);
         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
             // This is a one-based index, not zero-based. Doc link in similar comment below
             CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter));
             CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter));
             callback(userdata, results, selected_filter - 1);
             callback(userdata, results, selected_filter - 1);
-            success = true;
+            callback_called = true;
             goto quit;
             goto quit;
         } else if (!SUCCEEDED(hr)) {
         } else if (!SUCCEEDED(hr)) {
             goto quit;
             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
             // This is a one-based index, not zero-based. Doc link in similar comment below
             CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter));
             CHECK(pFileDialog->lpVtbl->GetFileTypeIndex(pFileDialog, &selected_filter));
             callback(userdata, results, selected_filter - 1);
             callback(userdata, results, selected_filter - 1);
-            success = true;
+            callback_called = true;
             goto quit;
             goto quit;
         } else if (!SUCCEEDED(hr)) {
         } else if (!SUCCEEDED(hr)) {
             goto quit;
             goto quit;
@@ -665,7 +673,7 @@ bool windows_ShowModernFileFolderDialog(SDL_FileDialogType dialog_type, const ch
         }
         }
 
 
         callback(userdata, (const char * const *) files, selected_filter - 1);
         callback(userdata, (const char * const *) files, selected_filter - 1);
-        success = true;
+        callback_called = true;
     } else {
     } else {
         // This is a one-based index, not zero-based.
         // 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
         // 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 };
         const char * const results[] = { file, NULL };
         callback(userdata, results, selected_filter - 1);
         callback(userdata, results, selected_filter - 1);
-        success = true;
+        callback_called = true;
         SDL_free(file);
         SDL_free(file);
     }
     }
 
 
-    success = true;
-
 #undef CHECK
 #undef CHECK
 
 
 quit:
 quit:
-    if (!success) {
-        WIN_SetError("dialogg");
+    if (!callback_called && call_callback_on_error) {
+        WIN_SetError("dialog");
         callback(userdata, NULL, -1);
         callback(userdata, NULL, -1);
+        callback_called = true;
     }
     }
 
 
     if (co_init) {
     if (co_init) {
@@ -750,7 +757,7 @@ quit:
         SDL_free(files);
         SDL_free(files);
     }
     }
 
 
-    return success;
+    return callback_called;
 }
 }
 
 
 // TODO: The new version of file dialogs
 // TODO: The new version of file dialogs