collective
Browse / github.com/storytold/pdfcraft / patch
fix(ui): support drag and drop file URIs on Combine files page (#4985)
Drag and drop not working in "Combine files" page This page says "Drop PDFs, images, text files or folders here, or choose them." <img width="1311" height="739" alt="Image" src="https://github.com/user-attachments/assets/66d98954-768f-4863-8504-3fbd664fb74d" /> But nothing happens when I drop a PDF from file manager. OS: Kubuntu 26.04 Desktop Environment: KDE 6.6.6
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- YusufMntm (imported from pull request #5007, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +297 −28 in 6 files
- Review
- pending
- Shared
- 2026-10-28
- Id
b2980c8f035aba774cb4cb82
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/app.rs crates/ui-egui/src/app/input.rs crates/ui-egui/src/workflows/combine_ui.rs crates/ui-egui/src/workflows/combine_ui/parts.rs crates/ui-egui/src/workflows/files.rs crates/ui-egui/tests/it/combine_grid.rs
@@ -393,11 +393,11 @@ impl eframe::App for PdfCraftApp { let dropped = self.drop_on_signature_pad(dropped); #[cfg(not(target_arch = "wasm32"))] let dropped = self.drop_on_grid(dropped, ctx);- for f in dropped {- // Files dropped on the Combine files tab join its list instead of opening.- if self.combine_showing() {- self.drop_into_combine(f, ctx);- } else {+ // Files dropped on the Combine files tab join its list in batch instead of opening individually.+ if self.combine_showing() {+ self.drop_into_combine(&dropped, ctx);+ } else {+ for f in dropped { self.open_dropped(f, ctx); } }@@ -10,8 +10,13 @@ fn read_grid_file((name, source): GridFile) -> Result<(String, Vec<u8>), (String } impl PdfCraftApp {+ /// Open a file dropped onto the application window outside of special drop targets. #[cfg(not(target_arch = "wasm32"))] pub(super) fn open_dropped(&mut self, f: egui::DroppedFileHandle, _ctx: &egui::Context) {+ if let Some(clean_path) = crate::workflows::files::clean_dropped_path(f.path()) {+ self.open_path(&clean_path.to_string_lossy());+ return;+ } let p = f.path().to_string_lossy().into_owned(); if !p.is_empty() && f.path().is_absolute() { self.open_path(&p);@@ -28,11 +28,11 @@ pub(crate) use files::Incoming; pub use files::{Columns, CombineFile, CombineView, ConflictPreview, Lock, SortKey}; pub(crate) use files::{Preview, RowAction}; use files::{Snapshot, UnlockPrompt};-#[cfg(not(target_arch = "wasm32"))]-pub(crate) use parts::ago; pub(crate) use parts::range_of; #[cfg(test)] pub(crate) use parts::{Part, moved_order, moved_pages, removed_pages, step_to};+#[cfg(not(target_arch = "wasm32"))]+pub(crate) use parts::{ago, combinable_in}; #[cfg(test)] use parts::{title_of, utc_stamp}; pub(crate) use view::page;@@ -45,7 +45,7 @@ const HISTORY: usize = 100; /// Files found in a folder are added up to this many at a time. #[cfg(not(target_arch = "wasm32"))]-const MAX_FOLDER_FILES: usize = 1000;+pub(crate) const MAX_FOLDER_FILES: usize = 1000; /// How deep "Add folder and subfolders" looks. #[cfg(not(target_arch = "wasm32"))] const MAX_FOLDER_DEPTH: usize = 16;@@ -3,7 +3,7 @@ use super::*; /// The PDFs, images and text files in `dir` (and, if `recursive`, the folders inside it, not /// following links), in natural order, at most [`MAX_FOLDER_FILES`]; and whether there were more. #[cfg(not(target_arch = "wasm32"))]-pub(super) fn combinable_in(dir: &std::path::Path, recursive: bool) -> (Vec<std::path::PathBuf>, bool) {+pub(crate) fn combinable_in(dir: &std::path::Path, recursive: bool) -> (Vec<std::path::PathBuf>, bool) { let mut found = Vec::new(); let mut more = false; let mut folders = vec![(dir.to_path_buf(), 0usize)];@@ -394,13 +394,17 @@ impl PdfCraftApp { std::fs::read(p) }; match read {- Ok(b) => files.push((name, b)),+ Ok(b) => {+ files.push((name, b));+ modified.push(std::fs::metadata(p).and_then(|m| m.modified()).ok());+ } Err(e) => { self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]);- return;+ if purpose != FilePurpose::Combine {+ return;+ } } }- modified.push(std::fs::metadata(p).and_then(|m| m.modified()).ok()); } if files.is_empty() { return;@@ -418,37 +422,88 @@ impl PdfCraftApp { } } - /// A file dropped on the Combine files tab: added to its list.+ /// Files dropped on the Combine files tab: added to its list in batch, preserving drop order. #[cfg(not(target_arch = "wasm32"))]- pub(crate) fn drop_into_combine(&mut self, f: egui::DroppedFileHandle, _ctx: &egui::Context) {- // A folder: the PDFs in it and in the folders inside it.- if f.path().is_absolute() && f.path().is_dir() {- self.add_folder_to_combine(f.path(), true);+ pub(crate) fn drop_into_combine(&mut self, dropped: &[egui::DroppedFileHandle], _ctx: &egui::Context) {+ if dropped.is_empty() { return; }- if f.path().is_absolute() {- self.use_paths(FilePurpose::Combine, &[f.path().to_path_buf()]);- return;+ let mut incoming = Vec::new();+ for f in dropped {+ if let Some(clean_path) = clean_dropped_path(f.path()) {+ if clean_path.is_dir() {+ let (folder_paths, more) = crate::workflows::combine_ui::combinable_in(&clean_path, true);+ let folder =+ clean_path.file_name().map_or_else(|| clean_path.to_string_lossy().into_owned(), |n| n.to_string_lossy().into_owned());+ if folder_paths.is_empty() {+ self.notify_fmt("No PDFs, images or text files found in {folder}", &[("folder", &folder)]);+ } else {+ for p in folder_paths {+ let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "file.pdf".into());+ match std::fs::read(&p) {+ Ok(bytes) => {+ let modified = std::fs::metadata(&p).and_then(|m| m.modified()).ok();+ incoming.push(crate::combine_ui::Incoming { name, bytes: Arc::new(bytes), modified, note: None, open_doc: None });+ }+ Err(e) => {+ self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]);+ }+ }+ }+ if more {+ self.notify_fmt(+ "Added the first {n} files in {folder}",+ &[("n", &crate::workflows::combine_ui::MAX_FOLDER_FILES.to_string()), ("folder", &folder)],+ );+ }+ }+ } else {+ let name = clean_path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "file.pdf".into());+ match std::fs::read(&clean_path) {+ Ok(bytes) => {+ let modified = std::fs::metadata(&clean_path).and_then(|m| m.modified()).ok();+ incoming.push(crate::combine_ui::Incoming { name, bytes: Arc::new(bytes), modified, note: None, open_doc: None });+ }+ Err(e) => {+ self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]);+ }+ }+ }+ continue;+ }+ let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());+ match f.bytes() {+ Ok(bytes) => incoming.push(crate::combine_ui::Incoming { name, bytes: Arc::new(bytes), modified: None, note: None, open_doc: None }),+ Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]),+ } }- let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());- match f.bytes() {- Ok(bytes) => self.use_files(FilePurpose::Combine, vec![(name, bytes)]),- Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]),+ if !incoming.is_empty() {+ self.stage_combine_with(incoming); } } /// Browsers read dropped files asynchronously; they join the Combine files list next frame. #[cfg(target_arch = "wasm32")]- pub(crate) fn drop_into_combine(&mut self, f: egui::DroppedFileHandle, ctx: &egui::Context) {+ pub(crate) fn drop_into_combine(&mut self, dropped: &[egui::DroppedFileHandle], ctx: &egui::Context) {+ if dropped.is_empty() {+ return;+ }+ let dropped = dropped.to_vec(); let requests = self.requests.clone(); let request = self.file_request(FilePurpose::Combine, Vec::new()); let ctx = ctx.clone(); wasm_bindgen_futures::spawn_local(async move {- let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());- if let Ok(bytes) = f.bytes_async().await+ let mut files = Vec::with_capacity(dropped.len());+ for f in dropped {+ let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());+ if let Ok(bytes) = f.bytes_async().await {+ files.push((name, bytes));+ }+ }+ if !files.is_empty() && let Ok(mut q) = requests.lock() {- q.push(FileRequest { files: vec![(name, bytes)], ..request });+ q.push(FileRequest { files, ..request }); ctx.request_repaint(); } });@@ -1062,6 +1117,88 @@ impl PdfCraftApp { } } +/// Resolve a dropped file path, stripping `file://` URI schemes and percent-decoding+/// paths sent by Linux desktop environments (e.g. KDE Plasma / Dolphin) and other file managers.+pub(crate) fn clean_dropped_path(raw: &std::path::Path) -> Option<std::path::PathBuf> {+ if raw.is_absolute() {+ let s = raw.to_string_lossy();+ let lower = s.to_ascii_lowercase();+ if !lower.starts_with("file:") {+ return Some(raw.to_path_buf());+ }+ }+ let s = raw.to_string_lossy();+ let s_no_lines = s.trim_end_matches(|c| c == '\r' || c == '\n');+ let lower = s_no_lines.to_ascii_lowercase();+ let cleaned = if let Some(rest) = lower.strip_prefix("file://") {+ let after_host = if rest.starts_with("localhost/") {+ s_no_lines.get("file://localhost".len()..).unwrap_or("")+ } else {+ s_no_lines.get("file://".len()..).unwrap_or("")+ };+ decode_uri_path(after_host)+ } else if lower.starts_with("file:/") {+ let after_scheme = s_no_lines.get("file:".len()..).unwrap_or("");+ decode_uri_path(after_scheme)+ } else {+ s_no_lines.to_string()+ };++ if cleaned.is_empty() {+ return None;+ }+ let p = std::path::PathBuf::from(cleaned);+ if p.is_absolute() || raw.is_absolute() { Some(p) } else { None }+}++/// Decode percent-encoded byte sequences and strip any leading slash preceding Windows drive letters.+fn decode_uri_path(s: &str) -> String {+ let bytes = percent_decode(s);+ let decoded = String::from_utf8_lossy(&bytes).into_owned();+ if decoded.len() >= 3+ && (decoded.starts_with('/') || decoded.starts_with('\\'))+ && decoded.as_bytes().get(1).is_some_and(|b| b.is_ascii_alphabetic())+ && decoded.as_bytes().get(2) == Some(&b':')+ {+ decoded.get(1..).unwrap_or("").to_string()+ } else {+ decoded+ }+}++/// Decode percent-encoded byte sequences (e.g. `%20` -> ` `).+fn percent_decode(s: &str) -> Vec<u8> {+ let bytes = s.as_bytes();+ let mut out = Vec::with_capacity(bytes.len());+ let mut i = 0;+ while i < bytes.len() {+ if bytes.get(i) == Some(&b'%')+ && i + 2 < bytes.len()+ && let (Some(&b1), Some(&b2)) = (bytes.get(i + 1), bytes.get(i + 2))+ && let (Some(hi), Some(lo)) = (hex_val(b1), hex_val(b2))+ {+ out.push((hi << 4) | lo);+ i += 3;+ } else if let Some(&b) = bytes.get(i) {+ out.push(b);+ i += 1;+ } else {+ break;+ }+ }+ out+}++/// Convert an ASCII byte representing a hexadecimal digit (0-9, a-f, A-F) to its numerical value.+fn hex_val(b: u8) -> Option<u8> {+ match b {+ b'0'..=b'9' => Some(b - b'0'),+ b'a'..=b'f' => Some(b - b'a' + 10),+ b'A'..=b'F' => Some(b - b'A' + 10),+ _ => None,+ }+}+ #[cfg(test)] mod stem_tests { use super::{attachment_file_name, file_stem_from_user, is_plain_file_name};@@ -1151,6 +1288,35 @@ mod stem_tests { let _ = std::fs::remove_dir_all(&dir); } + #[test]+ fn clean_dropped_path_decodes_file_uris_and_percent_encoded_characters() {+ use super::{clean_dropped_path, percent_decode};+ assert_eq!(String::from_utf8(percent_decode("hello%20world")).unwrap(), "hello world");+ assert_eq!(String::from_utf8(percent_decode("%E5%B9%B4%E5%BA%A6")).unwrap(), "年度");+ assert_eq!(percent_decode("invalid%2G%"), b"invalid%2G%");++ #[cfg(not(windows))]+ {+ let unix_path = std::path::Path::new("file:///tmp/my%20file.pdf\r\n");+ assert_eq!(clean_dropped_path(unix_path), Some(std::path::PathBuf::from("/tmp/my file.pdf")));++ let localhost = std::path::Path::new("file://localhost/tmp/%E5%B9%B4%E5%BA%A6.pdf");+ assert_eq!(clean_dropped_path(localhost), Some(std::path::PathBuf::from("/tmp/年度.pdf")));+ }++ #[cfg(windows)]+ {+ let win_path = std::path::Path::new("file:///C:/Users/User/Desktop/my%20file.pdf");+ assert_eq!(clean_dropped_path(win_path), Some(std::path::PathBuf::from("C:/Users/User/Desktop/my file.pdf")));++ let localhost_win = std::path::Path::new("file://localhost/D:/docs/report.pdf");+ assert_eq!(clean_dropped_path(localhost_win), Some(std::path::PathBuf::from("D:/docs/report.pdf")));+ }++ assert_eq!(clean_dropped_path(std::path::Path::new("")), None);+ assert_eq!(clean_dropped_path(std::path::Path::new("relative.pdf")), None);+ }+ fn walk(dir: &std::path::Path) -> Vec<std::path::PathBuf> { let mut out = Vec::new(); for entry in std::fs::read_dir(dir).unwrap() {@@ -1909,3 +1909,101 @@ fn open_documents_already_in_the_list_are_not_added_again() { add_all(&mut h); assert_eq!(names(&h), ["a.pdf", "report.pdf", "report.pdf", "b.pdf"]); }++#[derive(Debug)]+struct DroppedUri {+ path: std::path::PathBuf,+}++impl egui::DroppedFile for DroppedUri {+ fn path(&self) -> &std::path::Path {+ &self.path+ }+ fn bytes(&self) -> Result<Vec<u8>, String> {+ Err("simulated desktop drop without preloaded bytes".into())+ }+}++/// #4985: Dragging and dropping file:// URIs and URL-encoded file paths into Combine files works+/// and adds the dropped files in batch.+#[test]+fn dropping_file_uris_and_url_encoded_paths_into_combine_grid_adds_them() {+ let dir = std::env::temp_dir().join(format!("pdfcraft-combine-uri-drop-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();++ let path1 = dir.join("first doc.pdf");+ let path2 = dir.join("second_doc.pdf");+ std::fs::write(&path1, fixture(1)).unwrap();+ std::fs::write(&path2, fixture(2)).unwrap();++ let norm1 = path1.to_string_lossy().replace('\\', "/");+ let norm1 = norm1.trim_start_matches('/');+ let uri1_str = format!("file:///{norm1}").replace(' ', "%20");++ let norm2 = path2.to_string_lossy().replace('\\', "/");+ let norm2 = norm2.trim_start_matches('/');+ let uri2_str = format!("file://localhost/{norm2}");++ let mut h = Harness::builder().with_size(vec2(1400.0, 900.0)).build_eframe(|_cc| {+ let mut app = PdfCraftApp::new();+ app.set_option("language", "en").unwrap();+ app+ });+ h.run_steps(3);+ h.state_mut().execute("page.combine");+ h.run_steps(2);++ let drop1: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: uri1_str.into() });+ let drop2: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: uri2_str.into() });++ h.input_mut().dropped_files.extend([drop1, drop2]);+ h.run_steps(4);++ let _ = std::fs::remove_dir_all(&dir);++ assert_eq!(names(&h), ["first doc.pdf", "second_doc.pdf"], "dropped file URIs are added to combine draft in batch");+}++/// #4985: Dropping mixed files and folders preserves drop order, and unreadable paths do not prevent readable files from being staged.+#[test]+fn dropping_mixed_files_and_folders_into_combine_preserves_order() {+ let dir = std::env::temp_dir().join(format!("pdfcraft-combine-mixed-drop-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ let sub = dir.join("subfolder");+ std::fs::create_dir_all(&sub).unwrap();++ let file_before = dir.join("1_before.pdf");+ let folder_file = sub.join("2_inside.pdf");+ let file_after = dir.join("3_after.pdf");+ let unreadable = dir.join("nonexistent.pdf");++ std::fs::write(&file_before, fixture(1)).unwrap();+ std::fs::write(&folder_file, fixture(1)).unwrap();+ std::fs::write(&file_after, fixture(1)).unwrap();++ let mut h = Harness::builder().with_size(vec2(1400.0, 900.0)).build_eframe(|_cc| {+ let mut app = PdfCraftApp::new();+ app.set_option("language", "en").unwrap();+ app+ });+ h.run_steps(3);+ h.state_mut().execute("page.combine");+ h.run_steps(2);++ let drop_before: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: file_before });+ let drop_folder: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: sub });+ let drop_bad: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: unreadable });+ let drop_after: egui::DroppedFileHandle = std::sync::Arc::new(DroppedUri { path: file_after });++ h.input_mut().dropped_files.extend([drop_before, drop_folder, drop_bad, drop_after]);+ h.run_steps(4);++ let _ = std::fs::remove_dir_all(&dir);++ assert_eq!(+ names(&h),+ ["1_before.pdf", "2_inside.pdf", "3_after.pdf"],+ "mixed drop preserves order and unreadable path does not block readable files"+ );+}