-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
base: main
Are you sure you want to change the base?
dialog: Fix save file chooser with xdg portal #13007
Conversation
I'm not sure if I understand the issue well, but normally, For example, given a location string like
It's also possible to pass something like Re |
Thanks for the input. What you're suggesting sums up my other attempt: main...ColinKinloch:SDL:save_dialog_location_hack. On the windows side the portal field |
For Windows, anything on Windows Vista and above will use IFileDialog when #12979 will be in; for the |
ddcc6f2
to
07d22c9
Compare
07d22c9
to
061e6eb
Compare
I've the branch to fix |
061e6eb
to
e2af056
Compare
Any thoughts on this? |
I haven't tested it, but this seems reasonable to me. @icculus, thoughts? |
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 --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' |
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.
e2af056
to
5136446
Compare
I rebased and removed the debug logging. |
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
andcurrent_file
in the case the target is an existing file.current_name
andcurrent_folder
if the target is a file to be created in an existing folder.current_folder
as a fallback.I've also added calls to
SDL_IOFromFile
andSDL_CloseIO
totest/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 creationcurrent_name
: Placeholder for a save file operationcurrent_file
: Path to an existing file to save overI've only implement these
SDL_PROP
s 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 tocurrent_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, settingcurrent_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)