zenity: Fix non-UTF-8 paths, cancel results, filter names and the starting directory

CHANGELOG.md

@@ -8,6 +8,10 @@
 - Fix macOS file dialogs ignoring `set_directory` when `set_file_name` is also used.
 - Add `FileDialog::set_show_hidden_files` and `AsyncFileDialog::set_show_hidden_files` to control hidden file visibility. Supported on macOS, Windows, and Linux (GTK3).
 - Honor `FileDialog::set_directory` and `AsyncFileDialog::set_directory` when using the zenity fallback backend on Linux.
+- Fix the zenity fallback returning `None` for a file name that isn't valid UTF-8. zenity's output is now kept as bytes.
+- Return `None` instead of `Some(vec![])` when `pick_files` or `pick_folders` is cancelled in the zenity fallback, as the XDG portal backend does since #323.
+- Fix filter names that contain `|` in the zenity fallback, which zenity cut off at the `|`, reading the rest as file patterns. `|` in a filter name is now shown as `/`.
+- Open the directory set with `set_directory` in the zenity fallback's open and folder dialogs, which could start on Recent with GTK's default file chooser settings. The directory is now passed with `--filename` rather than as zenity's working directory, and a directory that doesn't exist no longer stops the dialog from opening.
 - Fix `liblary` typo in docs
 - Change wasm title to sanitize text contents via `title_el.set_text_content()` rather than `title_el.set_inner_html()`.
 - Set wasm dialog styles via `set_text_content()` rather than `set_inner_html()`, so rfd no longer uses `innerHTML`.

src/backend/linux/zenity.rs

@@ -1,4 +1,7 @@
-use std::{error::Error, fmt::Display, path::PathBuf, process::Command};
+use std::{
+    error::Error, ffi::OsStr, fmt::Display, os::unix::ffi::OsStrExt, path::PathBuf,
+    process::Command,
+};
 
 use crate::{
     message_dialog::{MessageButtons, MessageLevel},
@@ -8,7 +11,6 @@ use crate::{
 #[derive(Debug)]
 pub enum ZenityError {
     Io(std::io::Error),
-    FromUtf8Error(std::string::FromUtf8Error),
 }
 
 impl Error for ZenityError {}
@@ -17,7 +19,6 @@ impl Display for ZenityError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             ZenityError::Io(io) => write!(f, "{io}"),
-            ZenityError::FromUtf8Error(err) => err.fmt(f),
         }
     }
 }
@@ -28,12 +29,6 @@ impl From<std::io::Error> for ZenityError {
     }
 }
 
-impl From<std::string::FromUtf8Error> for ZenityError {
-    fn from(value: std::string::FromUtf8Error) -> Self {
-        Self::FromUtf8Error(value)
-    }
-}
-
 pub type ZenityResult<T> = Result<T, ZenityError>;
 
 fn command() -> Command {
@@ -58,37 +53,58 @@ fn file_dialog_command(dialog: &FileDialog) -> Command {
                 }
             })
             .collect();
-        command.arg(format!("{} | {}", filter.name, extensions.join(" ")));
+        // zenity ends the name at the first `|`, with no way to escape it, so show `/` instead.
+        // A non-ASCII lookalike would stop zenity from starting in the C locale.
+        let name = filter.name.replace('|', "/");
+        command.arg(format!("{} | {}", name, extensions.join(" ")));
     }
 
-    if let Some(name) = dialog.file_name.as_ref() {
-        command.arg("--filename");
-        command.arg(name);
-    }
+    // zenity starts in the folder of an absolute `--filename`, or in the path itself when it ends
+    // with `/`, whatever GTK's file chooser `startup-mode` is
+    let path = match (&dialog.starting_directory, &dialog.file_name) {
+        (Some(dir), name) => {
+            let dir = std::path::absolute(dir).unwrap_or_else(|_| dir.clone());
+            // Joining an empty name adds the trailing `/`
+            let path = dir.join(name.as_deref().unwrap_or_default());
+            // zenity 4 ignores `/`, as it's its own base name
+            Some(if path.as_os_str() == "/" {
+                "/./".into()
+            } else {
+                path
+            })
+        }
+        (None, name) => name.as_ref().map(PathBuf::from),
+    };
 
-    if let Some(dir) = dialog.starting_directory.as_ref() {
-        command.current_dir(dir);
+    if let Some(path) = path {
+        command.arg("--filename");
+        command.arg(path);
     }
 
     command
 }
 
-async fn run(mut command: Command) -> ZenityResult<Option<String>> {
+async fn run(mut command: Command) -> ZenityResult<Option<Vec<u8>>> {
     let res = {
         let (tx, rx) = crate::oneshot::channel();
         std::thread::spawn(move || {
             tx.send(command.output()).ok();
         });
         rx.await.map_err(std::io::Error::other)??
     };
-    let buffer = String::from_utf8(res.stdout)?;
+    let buffer = res.stdout;
 
     Ok((res.status.success() || !buffer.is_empty()).then_some(buffer))
 }
 
 /// Strip the single newline zenity prints after its output
-fn strip_newline(buffer: &str) -> &str {
-    buffer.strip_suffix('\n').unwrap_or(buffer)
+fn strip_newline(buffer: &[u8]) -> &[u8] {
+    buffer.strip_suffix(b"\n").unwrap_or(buffer)
+}
+
+/// Keep the bytes zenity prints as they are, as a file name doesn't have to be UTF-8
+fn to_path(bytes: &[u8]) -> PathBuf {
+    PathBuf::from(OsStr::from_bytes(bytes))
 }
 
 pub async fn pick_file(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
@@ -99,19 +115,18 @@ pub async fn pick_file(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
         .await?
         .as_deref()
         .map(strip_newline)
-        .map(PathBuf::from))
+        .map(to_path))
 }
 
-pub async fn pick_files(dialog: &FileDialog) -> ZenityResult<Vec<PathBuf>> {
+pub async fn pick_files(dialog: &FileDialog) -> ZenityResult<Option<Vec<PathBuf>>> {
     let mut command = file_dialog_command(dialog);
     command.args(["--file-selection", "--multiple", "--separator=\n"]);
 
     Ok(run(command)
         .await?
         .as_deref()
         .map(strip_newline)
-        .map(|s| s.split('\n').map(PathBuf::from).collect())
-        .unwrap_or_default())
+        .map(|s| s.split(|&byte| byte == b'\n').map(to_path).collect()))
 }
 
 pub async fn pick_folder(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
@@ -122,10 +137,10 @@ pub async fn pick_folder(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
         .await?
         .as_deref()
         .map(strip_newline)
-        .map(PathBuf::from))
+        .map(to_path))
 }
 
-pub async fn pick_folders(dialog: &FileDialog) -> ZenityResult<Vec<PathBuf>> {
+pub async fn pick_folders(dialog: &FileDialog) -> ZenityResult<Option<Vec<PathBuf>>> {
     let mut command = file_dialog_command(dialog);
     command.args([
         "--file-selection",
@@ -138,8 +153,7 @@ pub async fn pick_folders(dialog: &FileDialog) -> ZenityResult<Vec<PathBuf>> {
         .await?
         .as_deref()
         .map(strip_newline)
-        .map(|s| s.split('\n').map(PathBuf::from).collect())
-        .unwrap_or_default())
+        .map(|s| s.split(|&byte| byte == b'\n').map(to_path).collect()))
 }
 
 pub async fn save_file(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
@@ -150,7 +164,7 @@ pub async fn save_file(dialog: &FileDialog) -> ZenityResult<Option<PathBuf>> {
         .await?
         .as_deref()
         .map(strip_newline)
-        .map(PathBuf::from))
+        .map(to_path))
 }
 
 pub async fn message(
@@ -245,6 +259,8 @@ pub async fn question(
 
 #[cfg(test)]
 mod tests {
+    use std::{ffi::OsStr, os::unix::ffi::OsStrExt, path::PathBuf};
+
     use crate::FileDialog;
 
     #[test]
@@ -317,4 +333,75 @@ mod tests {
         let path = pollster::block_on(super::save_file(&FileDialog::default())).unwrap();
         dbg!(path);
     }
+
+    #[test]
+    fn output_path() {
+        assert_eq!(
+            super::to_path(super::strip_newline(b"/home/user/caf\xe9.txt\n")),
+            PathBuf::from(OsStr::from_bytes(b"/home/user/caf\xe9.txt"))
+        );
+        assert_eq!(super::strip_newline(b"x\n\n"), b"x\n");
+    }
+
+    #[test]
+    fn file_filter_name_with_pipe() {
+        let dialog = FileDialog::new().add_filter("Text | Markdown", &["txt", "md"]);
+        let command = super::file_dialog_command(&dialog);
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            [
+                "--no-markup",
+                "--file-filter",
+                "Text / Markdown | *.txt *.md"
+            ]
+        );
+    }
+
+    #[test]
+    fn file_dialog_starting_directory() {
+        let dialog = FileDialog::new().set_directory("/home/user/Documents");
+        let command = super::file_dialog_command(&dialog);
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            ["--no-markup", "--filename", "/home/user/Documents/"]
+        );
+        assert_eq!(command.get_current_dir(), None);
+
+        let dialog = dialog.set_file_name("report.txt");
+        let command = super::file_dialog_command(&dialog);
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            [
+                "--no-markup",
+                "--filename",
+                "/home/user/Documents/report.txt"
+            ]
+        );
+
+        let dialog = FileDialog::new().set_directory("Documents");
+        let command = super::file_dialog_command(&dialog);
+        let documents = std::env::current_dir().unwrap().join("Documents/");
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            [
+                "--no-markup".as_ref(),
+                "--filename".as_ref(),
+                documents.as_os_str()
+            ]
+        );
+
+        let dialog = FileDialog::new().set_directory("/");
+        let command = super::file_dialog_command(&dialog);
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            ["--no-markup", "--filename", "/./"]
+        );
+
+        let dialog = FileDialog::new().set_file_name("report.txt");
+        let command = super::file_dialog_command(&dialog);
+        assert_eq!(
+            command.get_args().collect::<Vec<_>>(),
+            ["--no-markup", "--filename", "report.txt"]
+        );
+    }
 }

src/backend/xdg_desktop_portal.rs

@@ -134,7 +134,7 @@ impl FilePickerDialogImpl for FileDialog {
         } else {
             warn!("Using zenity fallback");
             match block_on(zenity::pick_files(&self)) {
-                Ok(res) => Some(res),
+                Ok(res) => res,
                 Err(err) => {
                     error!("Failed to pick files with zenity: {err}");
                     None
@@ -219,7 +219,7 @@ impl FolderPickerDialogImpl for FileDialog {
         } else {
             warn!("Using zenity fallback");
             match block_on(zenity::pick_folders(&self)) {
-                Ok(res) => Some(res),
+                Ok(res) => res,
                 Err(err) => {
                     error!("Failed to pick folders with zenity: {err}");
                     None