Skip to content

dialog: Fix save file chooser with xdg portal #13007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ColinKinloch
Copy link
Contributor

@ColinKinloch ColinKinloch commented May 11, 2025

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 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)

Screencast.From.2025-05-11.18-47-44.mp4

KDE

Screencast.From.2025-05-11.18-44-39.mp4

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

Screencast.From.2025-05-11.18-50-16.mp4

Existing Issue(s)

@slouken slouken added this to the 3.4.0 milestone May 11, 2025
@Semphriss
Copy link
Contributor

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
Copy link
Contributor Author

Thanks for the input. What you're suggesting sums up my other attempt: 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
Copy link
Contributor

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 ColinKinloch force-pushed the save_dialog_portal_values branch from ddcc6f2 to 07d22c9 Compare May 25, 2025 15:02
@ColinKinloch ColinKinloch changed the title dialog: Add more xdg portal fields for saving dialog: Fix save file chooser with xdg portal May 25, 2025
@ColinKinloch ColinKinloch force-pushed the save_dialog_portal_values branch from 07d22c9 to 061e6eb Compare May 25, 2025 15:25
@ColinKinloch
Copy link
Contributor Author

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.

This correctly sets the xdg portal fields for targeting a specific
new filename or existing file.

"current_name" sets the dialogs placeholder name.
"current_file" targets an existing file.
"current_folder" for when the target is a folder.
@ColinKinloch ColinKinloch force-pushed the save_dialog_portal_values branch from 061e6eb to e2af056 Compare May 25, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants