diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 2c9d6b4579..91bad575c4 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -520,11 +520,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, all at once, instead of opening. + if self.combine_showing() && !dropped.is_empty() { + self.drop_into_combine(dropped, ctx); + } else { + for f in dropped { self.open_dropped(f, ctx); } } diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index cda282e9ed..4d6742cf70 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -19,7 +19,7 @@ impl PdfCraftApp { self.open_path(&p); return; } - let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into()); + let name = crate::files::dropped_name(&f); match f.bytes() { Ok(bytes) => { if self.ctx.is_some() && !self.run_inline { @@ -76,7 +76,7 @@ impl PdfCraftApp { }; let mut files = Vec::new(); for f in &dropped { - let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into()); + let name = crate::files::dropped_name(f); if f.path().is_absolute() { files.push((name, Err(f.path().to_path_buf()))); continue; @@ -130,7 +130,7 @@ impl PdfCraftApp { let inbox = self.inbox.clone(); 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()); + let name = crate::files::dropped_name(&f); if let Ok(bytes) = f.bytes_async().await && let Ok(mut q) = inbox.lock() { diff --git a/crates/ui-egui/src/workflows/combine_ui.rs b/crates/ui-egui/src/workflows/combine_ui.rs index c31ede9ca1..59d0c234e6 100644 --- a/crates/ui-egui/src/workflows/combine_ui.rs +++ b/crates/ui-egui/src/workflows/combine_ui.rs @@ -23,7 +23,6 @@ mod session; mod table; mod view; -#[cfg(not(target_arch = "wasm32"))] pub(crate) use files::Incoming; pub use files::{Columns, CombineFile, CombineView, ConflictPreview, Lock, SortKey}; pub(crate) use files::{Preview, RowAction}; diff --git a/crates/ui-egui/src/workflows/combine_ui/files.rs b/crates/ui-egui/src/workflows/combine_ui/files.rs index 4947e4f8b1..85634c2aff 100644 --- a/crates/ui-egui/src/workflows/combine_ui/files.rs +++ b/crates/ui-egui/src/workflows/combine_ui/files.rs @@ -274,6 +274,13 @@ pub(crate) struct Incoming { pub open_doc: Option, } +impl Incoming { + /// A file known only by its name and bytes. + pub(crate) fn new(name: String, bytes: Vec) -> Self { + Incoming { name, bytes: Arc::new(bytes), modified: None, note: None, open_doc: None } + } +} + /// A new value for [`CombineFile::auth`]: counted for the whole run, outside the undo history. fn next_auth() -> u64 { static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); diff --git a/crates/ui-egui/src/workflows/combine_ui/parts.rs b/crates/ui-egui/src/workflows/combine_ui/parts.rs index 5cba711a03..a64d0eeb34 100644 --- a/crates/ui-egui/src/workflows/combine_ui/parts.rs +++ b/crates/ui-egui/src/workflows/combine_ui/parts.rs @@ -38,6 +38,12 @@ fn combinable(ext: &str) -> bool { std::iter::once("pdf").chain(pdfcraft_engine::CONVERTIBLE).any(|c| ext.eq_ignore_ascii_case(c)) } +/// Whether Combine takes a file by what it is: a PDF (by its header or its name) or an image or +/// text file it converts, as `Session::as_pdf` tells them apart. +pub(super) fn takes(name: &str, bytes: &[u8]) -> bool { + pdfcraft_engine::source_kind(name, bytes).is_some() || name.to_ascii_lowercase().ends_with(".pdf") +} + /// A file's bookmark in the combined PDF: its name without ".pdf", or without the extension of /// the image or text file it was made from. pub(super) fn title_of(name: &str) -> &str { diff --git a/crates/ui-egui/src/workflows/combine_ui/session.rs b/crates/ui-egui/src/workflows/combine_ui/session.rs index 594b5f745a..7ef50d5ffc 100644 --- a/crates/ui-egui/src/workflows/combine_ui/session.rs +++ b/crates/ui-egui/src/workflows/combine_ui/session.rs @@ -3,7 +3,7 @@ use super::MAX_FOLDER_FILES; use super::files::{AssessError, Assessed, CombineFile, Incoming, Secret, Snapshot, SortKey, UnlockPrompt, assess}; #[cfg(not(target_arch = "wasm32"))] use super::parts::combinable_in; -use super::parts::{Part, items_of, moved_order, natural, parts_of, range_of, title_of}; +use super::parts::{Part, items_of, moved_order, natural, parts_of, range_of, takes, title_of}; use super::{Arc, BTreeSet, CombineTab, HISTORY, Modifiers, PdfCraftApp}; impl PdfCraftApp { @@ -246,9 +246,21 @@ impl PdfCraftApp { /// Add picked files to the Combine files list (and show it). pub(crate) fn stage_combine(&mut self, files: Vec<(String, Vec)>) { - let incoming = - files.into_iter().map(|(name, bytes)| Incoming { name, bytes: Arc::new(bytes), modified: None, note: None, open_doc: None }).collect(); - self.stage_combine_with(incoming); + self.stage_combine_with(files.into_iter().map(|(name, bytes)| Incoming::new(name, bytes)).collect()); + } + + /// Files dropped on the Combine files tab join the end of its list in the order dropped, as + /// one undo step. A drop isn't filtered like a pick, so files of a kind Combine doesn't take + /// are left out, and named. + pub(crate) fn add_dropped_to_combine(&mut self, files: Vec) { + let (taken, refused): (Vec<_>, Vec<_>) = files.into_iter().partition(|f| takes(&f.name, &f.bytes)); + if !refused.is_empty() { + let names: Vec<&str> = refused.iter().map(|f| f.name.as_str()).collect(); + self.notify_fmt("Not added: {names}. Combine takes PDFs, images and text files.", &[("names", &names.join(", "))]); + } + if !taken.is_empty() { + self.stage_combine_with(taken); + } } /// Add files to the Combine files list, with what can be known about them up front. diff --git a/crates/ui-egui/src/workflows/files.rs b/crates/ui-egui/src/workflows/files.rs index b8bda7183a..6a7a0434b3 100644 --- a/crates/ui-egui/src/workflows/files.rs +++ b/crates/ui-egui/src/workflows/files.rs @@ -149,6 +149,8 @@ pub struct FileRequest { purpose: FilePurpose, target: Option, files: Vec<(String, Vec)>, + /// Dropped on the Combine files tab rather than picked: what Combine can't take is left out. + dropped: bool, } /// Files picked asynchronously (web), waiting to be used. @@ -310,6 +312,11 @@ fn read_named(path: &std::path::Path) -> Result<(String, Vec), (String, Stri } } +/// The name of a file dropped on the window ("dropped.pdf" when it has none). +pub(crate) fn dropped_name(f: &egui::DroppedFileHandle) -> String { + f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into()) +} + impl PdfCraftApp { /// Ask for files to add to the Combine files list (its Add files… button). pub fn combine_dialog(&mut self) { @@ -451,37 +458,53 @@ impl PdfCraftApp { } } - /// A file dropped on the Combine files tab: added to its list. + /// Files dropped on the Combine files tab join its list in the order dropped (see + /// [`Self::add_dropped_to_combine`]); a folder adds the files in it and its subfolders. #[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); - return; - } - if f.path().is_absolute() { - self.use_paths(FilePurpose::Combine, &[f.path().to_path_buf()]); - return; - } - 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())]), + pub(crate) fn drop_into_combine(&mut self, dropped: Vec, _ctx: &egui::Context) { + let mut files = Vec::new(); + for f in dropped { + let path = f.path(); + if path.is_absolute() && path.is_dir() { + // After the files dropped before it. + self.add_dropped_to_combine(std::mem::take(&mut files)); + self.add_folder_to_combine(path, true); + continue; + } + let name = dropped_name(&f); + // A file on disk is read from there, with when it was last modified; a drop that + // carries only bytes (no path) as it is. + let (read, modified) = if path.is_absolute() { + (std::fs::read(path).map_err(|e| e.to_string()), std::fs::metadata(path).and_then(|m| m.modified()).ok()) + } else { + (f.bytes(), None) + }; + match read { + Ok(bytes) => files.push(crate::combine_ui::Incoming { modified, ..crate::combine_ui::Incoming::new(name, bytes) }), + Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e)]), + } } + self.add_dropped_to_combine(files); } /// 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: Vec, ctx: &egui::Context) { let requests = self.requests.clone(); - let request = self.file_request(FilePurpose::Combine, Vec::new()); + let request = FileRequest { dropped: true, ..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 + // One after another, so they join the list in the order dropped. + let mut files = Vec::new(); + for f in dropped { + if let Ok(bytes) = f.bytes_async().await { + files.push((dropped_name(&f), 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(); } }); @@ -492,7 +515,7 @@ impl PdfCraftApp { /// `requests` once the files are read; it is used on a later frame. pub fn file_request(&self, purpose: FilePurpose, files: Vec<(String, Vec)>) -> FileRequest { let target = if purpose.edits_active_document() { self.pick_target(self.active_ids().map(|(_, id)| id)) } else { None }; - FileRequest { purpose, target, files } + FileRequest { purpose, target, files, dropped: false } } /// `doc` as it is now, for a pick that will edit it. @@ -516,12 +539,19 @@ impl PdfCraftApp { /// Handle files picked asynchronously. pub(crate) fn process_file_requests(&mut self) { let pending: Vec<_> = self.requests.lock().map(|mut q| std::mem::take(&mut *q)).unwrap_or_default(); - for FileRequest { purpose, target, files } in pending { + for FileRequest { purpose, target, files, dropped } in pending { if !self.still_pick_target(target) { continue; } // Last-resort guard (AGENTS.md §4), as for desktop picks and commands. - if let Err(m) = pdfcraft_engine::guard(|| self.use_files(purpose, files)) { + let used = pdfcraft_engine::guard(|| { + if dropped { + self.add_dropped_to_combine(files.into_iter().map(|(name, bytes)| crate::combine_ui::Incoming::new(name, bytes)).collect()); + } else { + self.use_files(purpose, files); + } + }); + if let Err(m) = used { self.notify_fmt("That didn't work: an internal error stopped it ({m}).", &[("m", m.as_str())]); } } diff --git a/crates/ui-egui/tests/it/ui.rs b/crates/ui-egui/tests/it/ui.rs index a7f5b4cf7f..a464595615 100644 --- a/crates/ui-egui/tests/it/ui.rs +++ b/crates/ui-egui/tests/it/ui.rs @@ -1166,6 +1166,86 @@ fn a_folder_dropped_on_the_combine_tab_adds_its_pdfs() { assert_eq!(names, ["two.pdf", "one.pdf"], "subfolders included, in path order"); } +/// The Combine files tab, showing, with one file in its list already. +fn combine_tab_with_one_file() -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app + }); + h.run_steps(3); + h.state_mut().set_option("combine-view", "list").unwrap(); + h.state_mut().execute("page.combine"); + h.state_mut().use_files(pdfcraft_ui_egui::FilePurpose::Combine, vec![("first.pdf".into(), FIXTURE.to_vec())]); + h.run_steps(2); + h +} + +fn combine_names(h: &Harness<'_, PdfCraftApp>) -> Vec { + h.state().combine_draft.iter().map(|f| f.name.clone()).collect() +} + +#[test] +fn files_dropped_on_the_combine_tab_join_the_end_of_its_list_in_the_order_dropped() { + let mut h = combine_tab_with_one_file(); + let dir = std::env::temp_dir().join(format!("pdfcraft-combine-drop-order-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + // Files on disk come with their path (and no bytes); browsers hand over only a name and bytes. + let on_disk = |name: &str| -> egui::DroppedFileHandle { + std::fs::write(dir.join(name), FIXTURE).unwrap(); + std::sync::Arc::new(Dropped { path: dir.join(name), bytes: Vec::new() }) + }; + let in_memory = + |name: &str, bytes: &[u8]| -> egui::DroppedFileHandle { std::sync::Arc::new(Dropped { path: name.into(), bytes: bytes.to_vec() }) }; + let dropped = [on_disk("c.pdf"), in_memory("a.pdf", FIXTURE), in_memory("notes.txt", b"Some notes"), on_disk("b.pdf")]; + h.input_mut().dropped_files.extend(dropped); + h.run_steps(3); + let _ = std::fs::remove_dir_all(&dir); + assert!(h.state().views.is_empty(), "nothing opens"); + assert_eq!(combine_names(&h), ["first.pdf", "c.pdf", "a.pdf", "notes.txt", "b.pdf"]); + let draft = &h.state().combine_draft; + assert!(draft.iter().all(|f| f.problem.is_none()), "every file can be combined"); + assert!(draft[3].pages == 1, "the text file became a page"); + assert!(draft[1].modified.is_some() && draft[2].modified.is_none()); + // One drop, one step to undo. + h.state_mut().execute("edit.undo"); + h.run_steps(2); + assert_eq!(combine_names(&h), ["first.pdf"]); +} + +#[test] +fn a_file_combine_does_not_take_is_left_out_of_a_drop_and_named() { + let mut h = combine_tab_with_one_file(); + let path = std::env::temp_dir().join(format!("pdfcraft-combine-drop-{}.zip", std::process::id())); + std::fs::write(&path, b"PK\x03\x04").unwrap(); + let dropped: [egui::DroppedFileHandle; 4] = [ + std::sync::Arc::new(Dropped { path: "report.docx".into(), bytes: b"PK\x03\x04".to_vec() }), + std::sync::Arc::new(Dropped { path: "one.pdf".into(), bytes: FIXTURE.to_vec() }), + std::sync::Arc::new(Dropped { path: path.clone(), bytes: Vec::new() }), + // A PDF without the name of one is still a PDF. + std::sync::Arc::new(Dropped { path: "scan".into(), bytes: FIXTURE.to_vec() }), + ]; + h.input_mut().dropped_files.extend(dropped); + h.run_steps(3); + std::fs::remove_file(&path).ok(); + assert_eq!(combine_names(&h), ["first.pdf", "one.pdf", "scan"]); + let zip = path.file_name().unwrap().to_string_lossy().into_owned(); + h.get_by_label(&format!("Not added: report.docx, {zip}. Combine takes PDFs, images and text files.")); +} + +#[test] +fn a_pdf_dropped_on_a_document_opens_while_the_combine_tab_waits_behind_it() { + let mut h = combine_tab_with_one_file(); + h.state_mut().open_bytes("open.pdf", None, FIXTURE.to_vec()).unwrap(); + h.run_steps(2); + assert!(!h.state().combine_showing() && h.state().combine_tab.open); + let file: egui::DroppedFileHandle = std::sync::Arc::new(Dropped { path: "dropped.pdf".into(), bytes: FIXTURE.to_vec() }); + h.input_mut().dropped_files.push(file); + support::wait_until(&mut h, "dropped document", |app| app.views.len() == 2); + assert_eq!(combine_names(&h), ["first.pdf"], "the Combine files list is left as it was"); +} + #[test] fn files_and_quit_from_the_operating_system() { // #73: macOS hands Finder double-clicks, Open With and Dock drops over as Apple events. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index bf46e16b41..67ee899f85 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3034,8 +3034,8 @@ milestone = "M4" status = "shipped" commands = ["page.combine"] tools = ["doc_combine"] -evidence = ["crates/organize/src/tests.rs::combine_concatenates_with_a_bookmark_per_file", "crates/ui-egui/tests/it/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/it/editing.rs::combine_files_opens_in_its_own_tab", "crates/ui-egui/tests/it/editing.rs::combine_lists_size_and_warns_before_combining", "crates/ui-egui/tests/it/editing.rs::a_password_protected_file_is_unlocked_with_its_password_and_combined", "crates/ui-egui/tests/it/editing.rs::a_folder_adds_its_pdfs_images_and_text_in_order_and_optionally_its_subfolders", "crates/ui-egui/tests/it/editing.rs::images_and_text_files_join_the_combine_list_as_pdfs", "crates/ui-egui/tests/it/editing.rs::files_that_cannot_be_read_are_listed_and_left_out_once_the_user_agrees", "crates/ui-egui/tests/it/ui.rs::pdfs_dropped_on_the_combine_tab_join_its_list", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/automation/tests/it/automation.rs::combine_opens_protected_files_with_their_passwords", "crates/automation/tests/it/automation.rs::combine_takes_images_and_text_files", "crates/engine/src/tests.rs::files_for_combine_are_made_into_pdfs_and_pdfs_pass_through", "crates/organize/src/tests.rs::combine_keeps_pdfx_when_every_source_agrees", "crates/organize/src/tests.rs::combine_claims_no_pdfx_when_the_sources_disagree"] -notes = "Combine files is its own tab: a table of the files (name, pages, size, modified, warnings) with column sorting, resizing and moving; files, folders (with subfolders) and open documents can be added, or dropped on the tab. Images (PNG, JPEG, TIFF, GIF, BMP, JPEG 2000) and .txt files become PDFs as they are added (#569; also in doc_combine). A file that can't be read stays in the list with why, and Combine asks before leaving it out; Office documents, HTML and other formats are not converted yet. Problems show before Combine (password, security settings that withhold page assembly, bad page ranges, duplicates); protected files are unlocked with their open or permissions password (also doc_combine passwords). The result is never encrypted, and is not yet offered for protection in the same step. A PDF/X result (output intent, GTS_PDFXVersion, Trapped, XMP pdfxid) only when every source declares the same version and output intent (#263). When they differ the result claims none, and nothing reports that yet; Acrobat behaviour not observed." +evidence = ["crates/organize/src/tests.rs::combine_concatenates_with_a_bookmark_per_file", "crates/ui-egui/tests/it/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/it/editing.rs::combine_files_opens_in_its_own_tab", "crates/ui-egui/tests/it/editing.rs::combine_lists_size_and_warns_before_combining", "crates/ui-egui/tests/it/editing.rs::a_password_protected_file_is_unlocked_with_its_password_and_combined", "crates/ui-egui/tests/it/editing.rs::a_folder_adds_its_pdfs_images_and_text_in_order_and_optionally_its_subfolders", "crates/ui-egui/tests/it/editing.rs::images_and_text_files_join_the_combine_list_as_pdfs", "crates/ui-egui/tests/it/editing.rs::files_that_cannot_be_read_are_listed_and_left_out_once_the_user_agrees", "crates/ui-egui/tests/it/ui.rs::pdfs_dropped_on_the_combine_tab_join_its_list", "crates/ui-egui/tests/it/ui.rs::files_dropped_on_the_combine_tab_join_the_end_of_its_list_in_the_order_dropped", "crates/ui-egui/tests/it/ui.rs::a_file_combine_does_not_take_is_left_out_of_a_drop_and_named", "crates/ui-egui/tests/it/ui.rs::a_pdf_dropped_on_a_document_opens_while_the_combine_tab_waits_behind_it", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/automation/tests/it/automation.rs::combine_opens_protected_files_with_their_passwords", "crates/automation/tests/it/automation.rs::combine_takes_images_and_text_files", "crates/engine/src/tests.rs::files_for_combine_are_made_into_pdfs_and_pdfs_pass_through", "crates/organize/src/tests.rs::combine_keeps_pdfx_when_every_source_agrees", "crates/organize/src/tests.rs::combine_claims_no_pdfx_when_the_sources_disagree"] +notes = "Combine files is its own tab: a table of the files (name, pages, size, modified, warnings) with column sorting, resizing and moving; files, folders (with subfolders) and open documents can be added, or dropped on the tab (desktop and web): a drop joins the end of the list in the order dropped, as one undo step, not at the place under the pointer; files of a kind Combine does not take are left out and named. Dropped elsewhere, files open as before. Dropping adds no engine behaviour, so it has no tool of its own (doc_combine takes the files). Images (PNG, JPEG, TIFF, GIF, BMP, JPEG 2000) and .txt files become PDFs as they are added (#569; also in doc_combine). A file that can't be read stays in the list with why, and Combine asks before leaving it out; Office documents, HTML and other formats are not converted yet. Problems show before Combine (password, security settings that withhold page assembly, bad page ranges, duplicates); protected files are unlocked with their open or permissions password (also doc_combine passwords). The result is never encrypted, and is not yet offered for protection in the same step. A PDF/X result (output intent, GTS_PDFXVersion, Trapped, XMP pdfxid) only when every source declares the same version and output intent (#263). When they differ the result claims none, and nothing reports that yet; Acrobat behaviour not observed." [[feature]] id = "organize.combine-bookmarks"