diff --git a/apps/photocraft/src/main.rs b/apps/photocraft/src/main.rs index 0b82237a39..837332962f 100644 --- a/apps/photocraft/src/main.rs +++ b/apps/photocraft/src/main.rs @@ -50,6 +50,9 @@ mod linux_libs; mod logging; mod monitor_profile; mod photoshop_settings; +// Save As through the desktop portal, which reports the file type picked (#6121). +#[cfg(target_os = "linux")] +mod portal_save; mod presentation; mod screen_color; mod services; diff --git a/apps/photocraft/src/portal_save.rs b/apps/photocraft/src/portal_save.rs new file mode 100644 index 0000000000..42588e2bc4 --- /dev/null +++ b/apps/photocraft/src/portal_save.rs @@ -0,0 +1,183 @@ +//! File › Save As through the XDG desktop portal on Linux (#6121, #4328, #4394). +//! +//! The GTK file chooser behind the portal keeps the typed name when the user picks another file +//! type, and rfd 0.17 reads only the file name from the portal's answer, so `image.psd` with PNG +//! picked came back as `image.psd` and was written as a PSD. The answer does say which type was +//! picked (`current_filter`), so Save As asks the portal itself and gives a name the dialog left +//! alone that type's extension ([`crate::services::with_chosen_type`]). rfd stays the fallback +//! (zenity, or no portal), and this can go once an rfd release reads the picked type +//! (PolyMeilex/rfd#341). +//! +//! The dialog is attached to the window on X11. On Wayland it has no parent: naming the window to +//! the portal needs an xdg-foreign handle, which takes `unsafe` code the workspace forbids. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use zbus::zvariant::{OwnedObjectPath, OwnedValue, Value}; + +/// One file type in the dialog: its label and extensions (`("PNG", ["png"])`). +pub type Filter = (String, Vec); + +/// A Save dialog to show. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SaveRequest { + /// The portal's parent window identifier (`x11:`), or empty. + pub parent: String, + /// The name the dialog shows: the stem of the suggested file (`test`), see + /// `services::dialog_file_name`. + pub name: Option, + /// The suggested file's name (`test.psd`): a name that comes back with its extension still + /// takes the extension of the type picked ([`crate::services::with_chosen_type`]). + pub suggested: Option, + pub folder: Option, + pub filters: Vec, + /// The filter the dialog starts on. + pub current: Option, +} + +/// What the user chose: the path, as typed, and the label of the file type picked. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Saved { + pub path: PathBuf, + pub filter: Option, +} + +/// The portal's parent window identifier for `window`: `x11:` on X11, empty elsewhere. +pub fn parent_id(window: Option) -> String { + use eframe::wgpu::rwh::RawWindowHandle; + match window { + Some(RawWindowHandle::Xlib(h)) => format!("x11:{:x}", h.window), + Some(RawWindowHandle::Xcb(h)) => format!("x11:{:x}", h.window.get()), + _ => String::new(), + } +} + +/// Shows the portal's Save dialog and waits for it. `Ok(None)` is Cancel; `Err` means the portal +/// couldn't show it (no portal, no FileChooser), so the caller falls back to rfd. +pub fn save_file(conn: &zbus::blocking::Connection, req: &SaveRequest) -> Result, String> { + use futures_lite::StreamExt; + use zbus::blocking::Proxy; + let unique = conn.unique_name().ok_or("Portal connection has no unique name")?.as_str().trim_start_matches(':').replace('.', "_"); + static NEXT: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let token = format!("photocraft_save_{}_{}", std::process::id(), NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed)); + let path = format!("/org/freedesktop/portal/desktop/request/{unique}/{token}"); + // Listen before asking, so a quick answer isn't missed. + let request = Proxy::new(conn, "org.freedesktop.portal.Desktop", path.as_str(), "org.freedesktop.portal.Request").map_err(|e| e.to_string())?; + let mut replies = futures_lite::future::block_on(request.inner().receive_signal("Response")).map_err(|e| e.to_string())?; + let chooser = Proxy::new(conn, "org.freedesktop.portal.Desktop", "/org/freedesktop/portal/desktop", "org.freedesktop.portal.FileChooser") + .map_err(|e| e.to_string())?; + let handle: OwnedObjectPath = chooser.call("SaveFile", &(req.parent.as_str(), "Save As", options(req, &token))).map_err(|e| e.to_string())?; + // A portal older than 0.9 ignores `handle_token` and answers on a request path of its own, + // which it returns: listen there instead, as the Request documentation asks. + let reply = if handle.as_str() == path { + futures_lite::future::block_on(replies.next()) + } else { + let request = Proxy::new(conn, "org.freedesktop.portal.Desktop", handle.as_str(), "org.freedesktop.portal.Request").map_err(|e| e.to_string())?; + let mut replies = futures_lite::future::block_on(request.inner().receive_signal("Response")).map_err(|e| e.to_string())?; + futures_lite::future::block_on(replies.next()) + } + .ok_or("The save dialog closed without an answer")?; + let (code, results): (u32, HashMap) = reply.body().deserialize().map_err(|e| e.to_string())?; + answer(code, &results) +} + +/// The `SaveFile` options for `req`. +fn options<'a>(req: &'a SaveRequest, token: &'a str) -> HashMap<&'static str, Value<'a>> { + let mut o: HashMap<&'static str, Value<'a>> = HashMap::from([("handle_token", Value::from(token)), ("modal", Value::from(true))]); + if let Some(name) = &req.name { + o.insert("current_name", Value::from(name.as_str())); + } + if let Some(folder) = &req.folder { + // A byte string with its terminating NUL, as the portal specifies. + use std::os::unix::ffi::OsStrExt; + let mut bytes = folder.as_os_str().as_bytes().to_vec(); + bytes.push(0); + o.insert("current_folder", Value::from(bytes)); + } + let wire = |(label, exts): &Filter| (label.clone(), exts.iter().map(|e| (0u32, format!("*.{e}"))).collect::>()); + if !req.filters.is_empty() { + o.insert("filters", Value::from(req.filters.iter().map(wire).collect::>())); + } + if let Some(f) = req.current.and_then(|i| req.filters.get(i)) { + o.insert("current_filter", Value::from(wire(f))); + } + o +} + +/// Reads the portal's `Response`: code 0 with the first `uris` entry. 1 is Cancel, and 2 a dialog +/// closed some other way, such as with Esc in GNOME's: no file either, and not a reason to show +/// rfd's dialog instead (it did, so Esc opened a second Save As). +fn answer(code: u32, results: &HashMap) -> Result, String> { + match code { + 0 => {} + 1 | 2 => return Ok(None), + _ => return Err(format!("the save dialog failed (portal response {code})")), + } + let uris: Vec = results + .get("uris") + .ok_or("no file in the portal's answer")? + .try_clone() + .map_err(|e| e.to_string())? + .try_into() + .map_err(|e: zbus::zvariant::Error| e.to_string())?; + let uri = uris.first().ok_or("no file in the portal's answer")?; + let path = file_uri_path(uri).ok_or_else(|| format!("not a local file: {uri}"))?; + let filter = results.get("current_filter").and_then(|v| { + let (label, _globs): (String, Vec<(u32, String)>) = v.try_clone().ok()?.try_into().ok()?; + Some(label) + }); + Ok(Some(Saved { path, filter })) +} + +/// `file:///home/me/A%20b.psd` → `/home/me/A b.psd`; `None` for other schemes. +fn file_uri_path(uri: &str) -> Option { + let rest = uri.strip_prefix("file://")?; + // Only local files: `file:///…` or `file://localhost/…`. + let rest = rest.strip_prefix("localhost").unwrap_or(rest); + if !rest.starts_with('/') { + return None; + } + let bytes = rest.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' + && let Some(b) = bytes.get(i + 1..i + 3).and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u8::from_str_radix(h, 16).ok()) + { + out.push(b); + i += 3; + } else { + out.push(bytes[i]); + i += 1; + } + } + use std::os::unix::ffi::OsStringExt; + Some(PathBuf::from(std::ffi::OsString::from_vec(out))) +} + +/// Save As on Linux: the portal's dialog, with the name given the extension of the file type +/// picked. When that changes the name to one that already exists, the dialog opens again under +/// the new name, so the portal asks before replacing it (it asked only about the typed name). +pub fn save_as(conn: &zbus::blocking::Connection, mut req: SaveRequest) -> Result, String> { + loop { + let Some(saved) = save_file(conn, &req)? else { return Ok(None) }; + let picked = saved.filter.as_deref().and_then(|label| req.filters.iter().position(|(l, _)| l == label)); + // With one type offered there is nothing to pick, and a typed extension is the user's. + let exts: &[String] = match picked { + Some(i) if req.filters.len() > 1 => &req.filters[i].1, + _ => &[], + }; + let path = crate::services::with_chosen_type(&saved.path, req.suggested.as_deref(), exts); + if path == saved.path || !path.exists() { + return Ok(Some(path)); + } + req.name = path.file_name().map(|n| n.to_string_lossy().into_owned()); + req.suggested = req.name.clone(); + req.folder = path.parent().map(Path::to_path_buf); + req.current = picked; + } +} + +#[cfg(test)] +mod tests; diff --git a/apps/photocraft/src/portal_save/tests.rs b/apps/photocraft/src/portal_save/tests.rs new file mode 100644 index 0000000000..ed44a10d9c --- /dev/null +++ b/apps/photocraft/src/portal_save/tests.rs @@ -0,0 +1,232 @@ +use super::*; +use std::sync::{Arc, Mutex}; + +#[test] +fn file_uris_become_local_paths() { + assert_eq!(file_uri_path("file:///home/me/Untitled-1.psd"), Some(PathBuf::from("/home/me/Untitled-1.psd"))); + assert_eq!(file_uri_path("file:///home/me/A%20b%C3%A9.png"), Some(PathBuf::from("/home/me/A bé.png"))); + assert_eq!(file_uri_path("file://localhost/tmp/x.png"), Some(PathBuf::from("/tmp/x.png"))); + assert_eq!(file_uri_path("sftp://host/x.png"), None); + assert_eq!(file_uri_path("file://host/x.png"), None); +} + +#[test] +fn x11_windows_are_named_to_the_portal() { + use eframe::wgpu::rwh::{RawWindowHandle, XlibWindowHandle}; + assert_eq!(parent_id(Some(RawWindowHandle::Xlib(XlibWindowHandle::new(0x3a0000f)))), "x11:3a0000f"); + assert_eq!(parent_id(None), ""); +} + +/// What the mock portal answers to one `SaveFile` call: the response code, and the file and +/// filter label the user "picked". +type Script = Vec<(u32, Option<(&'static str, &'static str)>)>; + +/// The options of each `SaveFile` call the mock portal received: current_name and the label of +/// current_filter. +type Seen = Arc, Option)>>>; + +/// A private session bus (`dbus-daemon`) with a mock `org.freedesktop.portal.FileChooser` that +/// answers `SaveFile` from `script`. `None` when there is no `dbus-daemon` to run. +fn mock_portal(script: Script) -> Option<(zbus::blocking::Connection, Seen, std::process::Child)> { + mock_portal_with(script, false) +} + +/// [`mock_portal`]; with `own_paths` it is a portal older than 0.9, which ignores `handle_token`, +/// answers on a request path of its own and returns that from `SaveFile`, as a real dialog does +/// once the user is done with it (here after 300 ms). +fn mock_portal_with(script: Script, own_paths: bool) -> Option<(zbus::blocking::Connection, Seen, std::process::Child)> { + use std::io::BufRead; + let mut daemon = + std::process::Command::new("dbus-daemon").args(["--session", "--nofork", "--print-address=1"]).stdout(std::process::Stdio::piped()).spawn().ok()?; + let mut address = String::new(); + std::io::BufReader::new(daemon.stdout.take()?).read_line(&mut address).ok()?; + let address = address.trim().to_string(); + let server = zbus::blocking::connection::Builder::address(address.as_str()).unwrap().name("org.freedesktop.portal.Desktop").unwrap().build().unwrap(); + let seen: Seen = Arc::default(); + let log = seen.clone(); + std::thread::spawn(move || { + let mut script = script.into_iter(); + for msg in zbus::blocking::MessageIterator::from(&server) { + let Ok(msg) = msg else { break }; + let header = msg.header(); + if header.message_type() != zbus::message::Type::MethodCall || header.member().map(|m| m.as_str()) != Some("SaveFile") { + continue; + } + let (_parent, _title, options): (String, String, HashMap) = msg.body().deserialize().unwrap(); + let text = |key: &str| options.get(key).and_then(|v| String::try_from(v.try_clone().ok()?).ok()); + let filter = options.get("current_filter").and_then(|v| { + let (label, _): (String, Vec<(u32, String)>) = v.try_clone().ok()?.try_into().ok()?; + Some(label) + }); + log.lock().unwrap().push((text("current_name"), filter)); + let sender = header.sender().unwrap().as_str().trim_start_matches(':').replace('.', "_"); + let token = if own_paths { format!("old{}", log.lock().unwrap().len()) } else { text("handle_token").unwrap() }; + let path = format!("/org/freedesktop/portal/desktop/request/{sender}/{token}"); + server.reply(&header, &OwnedObjectPath::try_from(path.as_str()).unwrap()).unwrap(); + if own_paths { + std::thread::sleep(std::time::Duration::from_millis(300)); + } + let (code, pick) = script.next().unwrap_or((2, None)); + let mut results: HashMap<&str, Value<'_>> = HashMap::new(); + if let Some((uri, label)) = pick { + results.insert("uris", Value::from(vec![uri.to_string()])); + results.insert("current_filter", Value::from((label.to_string(), vec![(0u32, "*.x".to_string())]))); + } + server + .emit_signal( + header.sender().cloned().map(zbus::names::BusName::from), + path.as_str(), + "org.freedesktop.portal.Request", + "Response", + &(code, results), + ) + .unwrap(); + } + }); + let client = zbus::blocking::connection::Builder::address(address.as_str()).unwrap().build().unwrap(); + Some((client, seen, daemon)) +} + +fn request(name: &str) -> SaveRequest { + let filters = [("PSD Document", &["psd", "psb"][..]), ("PNG", &["png"]), ("JPEG", &["jpg"])]; + SaveRequest { + parent: String::new(), + name: Some(name.into()), + suggested: Some(name.into()), + folder: Some(PathBuf::from("/tmp")), + filters: filters.iter().map(|(l, e)| (l.to_string(), e.iter().map(|s| s.to_string()).collect())).collect(), + current: Some(0), + } +} + +/// #6121 end to end through a real D-Bus: the name comes back as typed with the type picked, +/// and Save As writes it with that type's extension. +#[test] +fn save_as_follows_the_file_type_picked_in_the_portal_dialog() { + let dir = std::env::temp_dir().join(format!("photocraft-portal-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let typed = format!("file://{}/Untitled-1.psd", dir.display()); + let typed: &'static str = Box::leak(typed.into_boxed_str()); + let Some((conn, seen, mut daemon)) = mock_portal(vec![(0, Some((typed, "PNG"))), (1, None), (0, Some((typed, "PSD Document")))]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + // PNG picked with `Untitled-1.psd` still in the name: written as Untitled-1.png. + assert_eq!(save_as(&conn, request("Untitled-1.psd")).unwrap(), Some(dir.join("Untitled-1.png"))); + assert_eq!(seen.lock().unwrap()[0], (Some("Untitled-1.psd".into()), Some("PSD Document".into())), "the request carried the name and type"); + // Cancel. + assert_eq!(save_as(&conn, request("Untitled-1.psd")).unwrap(), None); + // The type the name already has: unchanged. + assert_eq!(save_as(&conn, request("Untitled-1.psd")).unwrap(), Some(dir.join("Untitled-1.psd"))); + daemon.kill().ok(); + std::fs::remove_dir_all(&dir).ok(); +} + +/// The dialog opens on the bare name (`test`, see `services::dialog_file_name`), so GTK doesn't +/// ask to replace test.psd when PNG is picked: the type picked gives the extension, and a name +/// that then exists is asked about under that name. +#[test] +fn a_bare_name_takes_the_picked_type_and_an_existing_target_is_asked_about() { + let dir = std::env::temp_dir().join(format!("photocraft-portal-bare-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("test.psd"), b"open document").unwrap(); + let uri = |name: &str| -> &'static str { Box::leak(format!("file://{}/{name}", dir.display()).into_boxed_str()) }; + let Some((conn, seen, mut daemon)) = mock_portal(vec![ + (0, Some((uri("test"), "PNG"))), + (0, Some((uri("test.psd"), "PNG"))), + (0, Some((uri("test"), "PSD Document"))), + (0, Some((uri("test.psd"), "PSD Document"))), + ]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + // As File › Save As opens it: `test` shown, `test.psd` suggested. + let opened = || SaveRequest { suggested: Some("test.psd".into()), ..request("test") }; + // PNG picked: test.png, and nothing was asked about test.psd. + assert_eq!(save_as(&conn, opened()).unwrap(), Some(dir.join("test.png"))); + // The suggested extension back in the name (typed again, or GTK's completion) with PNG + // picked: test.png too, not a PSD written over test.psd. + assert_eq!(save_as(&conn, opened()).unwrap(), Some(dir.join("test.png"))); + // The type untouched: test.psd, which exists, so the dialog asks again as test.psd (the + // portal's replace prompt is then about the file that is written). + assert_eq!(save_as(&conn, opened()).unwrap(), Some(dir.join("test.psd"))); + let seen = seen.lock().unwrap().clone(); + let names: Vec<_> = seen.iter().map(|(name, _)| name.as_deref()).collect(); + assert_eq!(names, [Some("test"), Some("test"), Some("test"), Some("test.psd")], "{seen:?}"); + assert_eq!(seen[3].1.as_deref(), Some("PSD Document")); + daemon.kill().ok(); + std::fs::remove_dir_all(&dir).ok(); +} + +/// GNOME's dialog closed with Esc answers 2 ("ended some other way"): no file, like Cancel, and +/// no fallback to rfd, which opened a second Save As. +#[test] +fn a_dialog_closed_some_other_way_is_a_cancel() { + let Some((conn, _seen, mut daemon)) = mock_portal(vec![(2, None)]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + assert_eq!(save_as(&conn, request("test")), Ok(None)); + daemon.kill().ok(); +} + +/// An extension the user typed is theirs: `art.png` with the PSD type left as the dialog opened +/// saves a PNG, as it did before the type was read. +#[test] +fn a_typed_extension_wins_over_an_untouched_file_type() { + let typed: &'static str = "file:///tmp/photocraft-none/art.png"; + let Some((conn, _seen, mut daemon)) = mock_portal(vec![(0, Some((typed, "PSD Document")))]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + assert_eq!(save_as(&conn, request("Untitled-1.psd")).unwrap(), Some(PathBuf::from("/tmp/photocraft-none/art.png"))); + daemon.kill().ok(); +} + +/// A portal older than 0.9 answers on a request path of its own: the wait follows the handle +/// `SaveFile` returns, as the Request documentation asks, instead of the predicted path. +#[test] +fn the_answer_is_awaited_on_the_handle_the_portal_returns() { + let typed: &'static str = "file:///tmp/photocraft-none/Untitled-1.psd"; + let Some((conn, _seen, mut daemon)) = mock_portal_with(vec![(0, Some((typed, "PNG")))], true) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + assert_eq!(save_as(&conn, request("Untitled-1.psd")).unwrap(), Some(PathBuf::from("/tmp/photocraft-none/Untitled-1.png"))); + daemon.kill().ok(); +} + +/// When the extension the type gives names a file that exists, the dialog opens again under that +/// name (so the portal asks before replacing it) instead of overwriting it unasked. +#[test] +fn a_renamed_file_that_exists_is_confirmed_in_the_dialog() { + let dir = std::env::temp_dir().join(format!("photocraft-portal-exists-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("art.png"), b"old").unwrap(); + let typed: &'static str = Box::leak(format!("file://{}/art.psd", dir.display()).into_boxed_str()); + let confirmed: &'static str = Box::leak(format!("file://{}/art.png", dir.display()).into_boxed_str()); + let Some((conn, seen, mut daemon)) = mock_portal(vec![(0, Some((typed, "PNG"))), (0, Some((confirmed, "PNG")))]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + assert_eq!(save_as(&conn, request("art.psd")).unwrap(), Some(dir.join("art.png"))); + let seen = seen.lock().unwrap().clone(); + assert_eq!(seen.len(), 2, "asked again: {seen:?}"); + assert_eq!(seen[1], (Some("art.png".into()), Some("PNG".into())), "under the new name, on the picked type"); + assert_eq!(std::fs::read(dir.join("art.png")).unwrap(), b"old", "nothing written by the dialog"); + daemon.kill().ok(); + std::fs::remove_dir_all(&dir).ok(); +} + +/// With only one type offered (the Swatches panel's .aco) a typed extension is kept. +#[test] +fn a_single_file_type_keeps_the_typed_extension() { + let typed: &'static str = "file:///tmp/photocraft-none/Swatches.ase"; + let Some((conn, _seen, mut daemon)) = mock_portal(vec![(0, Some((typed, "Color Swatches")))]) else { + eprintln!("no dbus-daemon: skipped"); + return; + }; + let req = SaveRequest { filters: vec![("Color Swatches".into(), vec!["aco".into()])], ..request("Swatches.aco") }; + assert_eq!(save_as(&conn, req).unwrap(), Some(PathBuf::from("/tmp/photocraft-none/Swatches.ase"))); + daemon.kill().ok(); +} diff --git a/apps/photocraft/src/services.rs b/apps/photocraft/src/services.rs index 9de1c475ea..6c34fda33a 100644 --- a/apps/photocraft/src/services.rs +++ b/apps/photocraft/src/services.rs @@ -95,18 +95,51 @@ fn save_filters(suggested: &str) -> Vec<(String, Vec)> { v } -/// The name the save dialog starts with. On Windows a visible extension stays put when the user -/// picks another type (with File name extensions shown, `image.psd` saved as PhotoCraft stayed -/// `image.psd`, #4394), while a bare name gets the chosen type's extension, so there the dialog -/// gets the stem and the suggested type (listed first by [`save_filters`]) supplies `.psd`. A -/// stem that still has a dot (`scan.jpg.psd`: Windows could keep `scan.jpg`) or an upper-case +/// `path` with the extension of the file type picked in the save dialog (`chosen`), where the +/// dialog left the name alone (#6121): the format follows the extension, and the GTK file chooser +/// keeps the typed name when another type is picked. Only a name that still has the extension the +/// dialog opened with (`opened_with`, `Untitled-1.psd` with PNG picked) or none gets the type's +/// extension, as PolyMeilex/rfd#341 does. An extension the user typed that PhotoCraft saves +/// (`art.png` with the PSD type untouched, the way to save a PNG before this) is theirs and stays, +/// and so does one of the type's own (`.psb` for PSD). One PhotoCraft doesn't save (`scan.2024`) +/// is part of the name, and the type's extension is added after it. +#[cfg(any(target_os = "linux", test))] +pub(crate) fn with_chosen_type(path: &Path, opened_with: Option<&str>, chosen: &[String]) -> PathBuf { + let Some(first) = chosen.first() else { return path.to_path_buf() }; + let ext = path.extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase); + let opened_ext = opened_with.and_then(|n| Path::new(n).extension()).and_then(|e| e.to_str()).map(str::to_ascii_lowercase); + match ext { + Some(ext) if chosen.contains(&ext) => path.to_path_buf(), + Some(ext) if opened_ext.as_ref() == Some(&ext) => path.with_extension(first), + Some(ext) if SAVE_FILTERS.iter().chain(OTHER_SAVE_FILTERS).any(|(_, exts)| exts.contains(&ext.as_str())) => path.to_path_buf(), + _ => { + let mut name = path.as_os_str().to_owned(); + name.push("."); + name.push(first); + PathBuf::from(name) + } + } +} + +/// The name the save dialog starts with. Where the type picked gives a bare name its extension +/// (`adds_extension`), the dialog gets the stem and the suggested type (listed first by +/// [`save_filters`]) supplies `.psd`: +/// - On Windows a visible extension stays put when the user picks another type (with File name +/// extensions shown, `image.psd` saved as PhotoCraft stayed `image.psd`, #4394). +/// - On Linux the portal's dialog keeps the name as typed, and before returning asks whether to +/// replace a file of that name, so `test.psd` with PNG picked asked to replace test.psd, which +/// the save leaves alone (#6121). From the stem [`with_chosen_type`] gives the picked type's +/// extension, and a name that then exists is asked about under that name +/// (`portal_save::save_as`). +/// +/// A stem that still has a dot (`scan.jpg.psd`: Windows could keep `scan.jpg`) or an upper-case /// extension (the user's spelling) keeps the whole name. Elsewhere the whole name is kept: the -/// macOS panel has no type list, and on Linux rfd doesn't report which type was picked. -fn dialog_file_name(suggested: &str, windows: bool) -> Option { +/// macOS panel has no type list, and rfd's Linux dialogs don't say which type was picked. +fn dialog_file_name(suggested: &str, adds_extension: bool) -> Option { let path = Path::new(suggested); let name = path.file_name()?.to_string_lossy().into_owned(); let (Some(stem), Some(ext)) = (path.file_stem(), path.extension().and_then(|e| e.to_str())) else { return Some(name) }; - let bare = windows && Path::new(stem).extension().is_none() && !ext.is_empty() && ext == ext.to_ascii_lowercase(); + let bare = adds_extension && Path::new(stem).extension().is_none() && !ext.is_empty() && ext == ext.to_ascii_lowercase(); Some(if bare { stem.to_string_lossy().into_owned() } else { name }) } @@ -148,17 +181,52 @@ fn show_file_dialog(request: FileDialogRequest, parent: Option<&eframe::Frame>, Box::pin(async move { picked.await.map(|folder| FileDialogAnswer::FolderTo(path_of(&folder))) }) } FileDialogRequest::Save { suggested } => { - for (name, exts) in save_filters(&suggested) { - dialog = dialog.add_filter(name, &exts); + let filters = save_filters(&suggested); + let name = dialog_file_name(&suggested, cfg!(windows)); + let dir = Path::new(&suggested).parent().filter(|p| !p.as_os_str().is_empty()).map(Path::to_path_buf); + for (name, exts) in &filters { + dialog = dialog.add_filter(name, exts); } - if let Some(name) = dialog_file_name(&suggested, cfg!(windows)) { + if let Some(name) = &name { dialog = dialog.set_file_name(name); } - if let Some(dir) = Path::new(&suggested).parent().filter(|p| !p.as_os_str().is_empty()) { + if let Some(dir) = &dir { dialog = dialog.set_directory(dir); } - let picked = dialog.save_file(); - Box::pin(async move { picked.await.map(|file| FileDialogAnswer::SaveTo(path_of(&file))) }) + // On Linux the portal's own dialog, which reports the file type picked (#6121); rfd if + // the portal can't show it. + #[cfg(target_os = "linux")] + { + use eframe::wgpu::rwh::HasWindowHandle; + let parent = crate::portal_save::parent_id(parent.and_then(|p| p.window_handle().ok()).map(|h| h.as_raw())); + let req = crate::portal_save::SaveRequest { + parent, + name: dialog_file_name(&suggested, true), + suggested: Path::new(&suggested).file_name().map(|n| n.to_string_lossy().into_owned()), + folder: dir, + filters, + current: Some(0), + }; + Box::pin(async move { + let portal = zbus::blocking::connection::Builder::session() + .and_then(|b| b.build()) + .map_err(|e| e.to_string()) + .and_then(|conn| crate::portal_save::save_as(&conn, req)); + match portal { + Ok(path) => path.map(|p| FileDialogAnswer::SaveTo(p.to_string_lossy().into_owned())), + Err(e) => { + log::info!("portal save dialog unavailable ({e}), using rfd"); + dialog.save_file().await.map(|file| FileDialogAnswer::SaveTo(path_of(&file))) + } + } + }) + } + #[cfg(not(target_os = "linux"))] + { + let _ = filters; + let picked = dialog.save_file(); + Box::pin(async move { picked.await.map(|file| FileDialogAnswer::SaveTo(path_of(&file))) }) + } } }; // Without the thread the reply is dropped unanswered, which the app takes as Cancel. @@ -585,6 +653,41 @@ mod tests { use photocraft_ui_egui::{PhotocraftApp, prefs_ui}; use serde_json::{Value, json}; + /// #6121: the file type picked in the save dialog gives the name its extension where the + /// dialog left the one it opened with, and only there. + #[test] + fn the_picked_file_type_gives_the_name_its_extension() { + let exts = |e: &[&str]| e.iter().map(|s| s.to_string()).collect::>(); + let (png, psd) = (exts(&["png"]), exts(&["psd", "psb"])); + let opened = Some("Untitled-1.psd"); + let cases = [ + // PNG picked, the name as the dialog opened (or renamed, its extension kept). + ("/pics/Untitled-1.psd", opened, &png, "/pics/Untitled-1.png"), + ("/pics/art.psd", opened, &png, "/pics/art.png"), + ("/pics/ART.PSD", opened, &png, "/pics/ART.png"), + // No extension: the type's. One PhotoCraft doesn't save is part of the name. + ("/pics/art", opened, &png, "/pics/art.png"), + ("/pics/scan.2024", opened, &png, "/pics/scan.2024.png"), + // Typed, an extension PhotoCraft saves is the user's: kept, whatever the type says + // (the PSD type untouched, or PNG picked and .jpg typed). + ("/pics/art.png", opened, &psd, "/pics/art.png"), + ("/pics/art.jpg", opened, &png, "/pics/art.jpg"), + ("/pics/art.pcraft", opened, &png, "/pics/art.pcraft"), + // Already the type's: kept, including its other extensions. + ("/pics/art.png", opened, &png, "/pics/art.png"), + ("/pics/art.PNG", opened, &png, "/pics/art.PNG"), + ("/pics/big.psb", opened, &psd, "/pics/big.psb"), + // The dialog opened on a .pcraft: that is the extension the type replaces. + ("/pics/art.pcraft", Some("art.pcraft"), &png, "/pics/art.png"), + ("/pics/art.psd", Some("art.pcraft"), &png, "/pics/art.psd"), + ]; + for (typed, opened, chosen, want) in cases { + assert_eq!(with_chosen_type(Path::new(typed), opened, chosen), PathBuf::from(want), "{typed} (opened {opened:?})"); + } + // No type picked: the name as typed. + assert_eq!(with_chosen_type(Path::new("/pics/art.psd"), opened, &[]), PathBuf::from("/pics/art.psd")); + } + #[cfg(all(unix, not(target_os = "macos")))] #[test] fn open_filters_cover_uppercase_and_mixed_case_extensions() {