dialog: Fix save file chooser with xdg portal

[ColinKinloch]

This reduces the likelyhood of the user saving a file without an extension.

Updated Description

This MR corrects the behaviour of the xdg-portal implementation of SDL_ShowSaveFileDialog. Currently SDL assumes the target path is a folder by setting the field "current_folder".

Instead this branch sets:

• current_name and current_file in the case the target is an existing file.
• current_name and current_folder if the target is a file to be created in an existing folder.
• Just current_folder as a fallback.

I've also added calls to SDL_IOFromFile and SDL_CloseIO to test/testdialog.c which makes it easier to test that the dialog is working.

Old Description

One goal of flatpak is to restrict access to the filesystem, as such a portal SaveFile dialog returns a virtual path for the file chosen by the user. The name of this file cannot be changed (e.g. to fix the extension), therefore it is important to pass enough hints to the portal so the dialog that is created guides the user to name the file something sensible.

In this branch I add the following from the xdg-desktop-portal File Chooser spec:

• current_filter: Index of the filter to show on dialog creation
• current_name: Placeholder for a save file operation
• current_file: Path to an existing file to save over

I've only implement these SDL_PROPs for the portal backend, however they presumably can apply to other platforms.

In https://github.com/libsdl-org/SDL/compare/main...ColinKinloch:SDL:save_dialog_location_hack I attempted to populate the values from SDL_PROP_FILE_DIALOG_LOCATION_STRING, this may be a preferable approach. The wiki describes it as "the default folder or file to start the dialog at" however currently it is mapped directly to current_folder which is described as "Suggested folder in which the file should be saved".

Unfortunately not all portal implementations act exactly the same.

Bellow are some examples of the behaviours of the different portals tested in gnome by forcing the portal:

GNOME (nautilus)

https://github.com/user-attachments/assets/a9a7cce6-35ab-4dff-8323-8238d36cb19b

KDE

https://github.com/user-attachments/assets/f8525e2e-1b25-4fc5-a712-a1e3919665f4

Using current_file with the kde portal to save over an existing file removes the extension, setting current_filter hides this issue as the kde file chooser with automatically add the extension. I've reported this bug to kde.

GTK

https://github.com/user-attachments/assets/b04bd53f-115b-4b10-9b55-994e5f52e753

Existing Issue(s)

[Semphriss]

I'm not sure if I understand the issue well, but normally, SDL_PROP_FILE_DIALOG_LOCATION_STRING should cover all of those values. It expects an absolute path to the file which the dialog should be pre-filled with, with the filename being the default value in the filename text box, and the path before the file name being the default folder opened in the dialog. The filename can be omitted to just open the file dialog in a certain location without default filename.

For example, given a location string like /path/to/file.ext, the values should be filled with:

• current_name: file.ext
• current_file: /path/to/file.ext, if it exists (not sure how that argument is handled if the file doesn't exist yet?)
• current_folder: /path/to/

It's also possible to pass something like /path/to/, in which case current_name and current_file should be left unset.

---

Re current_filter: from what I can remember, Windows also supports that option. I think it would be worth adding it as an option if more than one platform supports it.

[ColinKinloch]

Thanks for the input. What you're suggesting sums up my other attempt: https://github.com/libsdl-org/SDL/compare/main...ColinKinloch:SDL:save_dialog_location_hack. I'll clean it up and submit it.

---

On the windows side the portal field current_name could be mapped to the OPENFILENAMEW field lpstrFileTitle, but I've not used it and I'm not find the docs particularly helpful.

[Semphriss]

On the windows side the portal field current_name could be mapped to the OPENFILENAMEW field lpstrFileTitle, but I've not used it and I'm not find the docs particularly helpful.

For Windows, anything on Windows Vista and above will use IFileDialog when #12979 will be in; for the OPENFILENAMEW implementation, it currently uses lpstrFile and lpstrInitialDir.

[ColinKinloch]

I've the branch to fix SDL_ShowSaveFileDialog rather than providing a way to work around it. It also makes the testdialog test create the target file when saving, I think this is useful but I understand if not including SDL3/SDL_iostream.h in the dialog test is preferred.

[ColinKinloch]

Any thoughts on this?

[slouken]

I haven't tested it, but this seems reasonable to me. @icculus, thoughts?

[ColinKinloch]

Out of interest it can be tested with:

flatpak install org.freedesktop.Sdk//24.08
flatpak run --filesystem="$(pwd)" org.freedesktop.Sdk//24.08 -c 'cmake -B _build_fp -G Ninja -DSDL_TESTS=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON'
flatpak run --filesystem="$(pwd)" org.freedesktop.Sdk//24.08 -c 'cmake --build _build_fp'
SDL_LOGGING=system=debug flatpak run --devel --filesystem="$(pwd)" --device=dri --socket=wayland org.freedesktop.Sdk//24.08 -c './_build_fp/test/testdialog'

[ColinKinloch]

I rebased and removed the debug logging.

[ColinKinloch]

Is there something I can do to make reviewing easier?

[slouken]

Is there something I can do to make reviewing easier?

Yes, can you fix the conflicts?

Also, there are a number of issues with your PR.

First, you're not checking the return values of SDL_malloc() and SDL_strdup(). Any of them can return NULL and should be checked, both in SDL and in testdialog.c.

Second, you're not freeing location_folder and location_name in the early return cases.

Third, initial_path could be NULL in testdialog.c and will crash in SDL_strlen() if that's the case.

[ColinKinloch]

Ah, sorry. I've fixed the issues you mentioned.

[ColinKinloch]

I updated testdialog to free last_saved_path on exit and make the SDL_ShowSaveFileDialog a little more readable. It also removes the unnecessary S_ISLNK.

[slouken]

Merged, thanks!