diff --git a/crates/ui-egui/src/editing/save.rs b/crates/ui-egui/src/editing/save.rs index 41f647ff92..855c2faea9 100644 --- a/crates/ui-egui/src/editing/save.rs +++ b/crates/ui-egui/src/editing/save.rs @@ -58,10 +58,7 @@ impl PdfCraftApp { // redacted: ask where instead of writing it over that file. In the web build a delivered // step stays counted in `redactions_applied`, so fewer steps than were delivered means one // was undone. - let unredacted = self - .views - .get(index) - .is_some_and(|v| v.redactions_undone_after_save > 0 || v.saved_redactions_cut_off || v.redactions_applied < v.redactions_downloaded); + let unredacted = self.views.get(index).is_some_and(restores_saved_redactions); // The restored content is not a redacted copy: propose the name without `_Redacted`, so // the dialog's default is not the redacted file. let name = if unredacted && !redacted { unredacted_file_name(&name) } else { name }; @@ -80,6 +77,12 @@ impl PdfCraftApp { let dialog = in_document_folder(dialog, doc_path.as_deref()); // The bytes are taken once the user has chosen, so edits made meanwhile are saved too. self.ask_one(crate::pickers::Ask::Save(dialog), None, move |app, dest| { + // An Undo past saved redactions while the dialog was open makes the name + // it proposed wrong: ask again with the name for the restored content. + if !unredacted && let Some(index) = app.views.iter().position(|v| v.id == id && restores_saved_redactions(v)) { + app.save_then(index, SaveTarget::As, after); + return; + } app.save_doc_to_then(id, &dest.to_string_lossy(), target, Box::new(after)); }); return false; @@ -260,6 +263,13 @@ pub(crate) fn is_mail_temp_copy(path: &str) -> bool { || p.contains("/library/containers/com.apple.mail/data/library/mail downloads/") } +/// Whether `view` holds content that a saved or delivered Apply redactions removed: a saved step +/// is undone, cannot be redone, or (in the web build) fewer steps are in effect than were +/// delivered. +fn restores_saved_redactions(view: &crate::DocView) -> bool { + view.redactions_undone_after_save > 0 || view.saved_redactions_cut_off || view.redactions_applied < view.redactions_downloaded +} + /// `name` without the `_Redacted` that [`pdfcraft_engine::redacted_file_name`] adds before the /// extension ("contract_Redacted.pdf" gives "contract.pdf"). Any other name is returned as it is. pub(crate) fn unredacted_file_name(name: &str) -> String { diff --git a/crates/ui-egui/tests/it/redact.rs b/crates/ui-egui/tests/it/redact.rs index 9a7f637722..62611f9001 100644 --- a/crates/ui-egui/tests/it/redact.rs +++ b/crates/ui-egui/tests/it/redact.rs @@ -311,6 +311,56 @@ fn delivered_redactions_save_in_place() { let _ = std::fs::remove_dir_all(&dir); } +/// Undo past a saved Apply redactions while the Save dialog is open makes Save ask again, so the +/// restored content is not written to the redacted copy the dialog proposed. +#[test] +fn undoing_saved_redactions_while_the_save_dialog_is_open_asks_again() { + let dir = std::env::temp_dir().join(format!("pdfcraft-redacted-dialog-undo-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let original = dir.join("contract.pdf"); + std::fs::write(&original, include_bytes!("../data/form.pdf")).unwrap(); + let path = original.to_string_lossy().into_owned(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_path(&path); + app + }); + crate::support::settle(&mut h); + let apply = |h: &mut Harness<'static, PdfCraftApp>| { + h.state_mut().execute("redact.pages"); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + assert!(h.state_mut().execute("redact.apply")); + h.run_steps(2); + h.get_by_label("Apply").click(); + h.run_steps(4); + }; + apply(&mut h); + let copy = dir.join("contract_Redacted.pdf"); + h.state_mut().pick_override = Some(vec![copy.to_string_lossy().into_owned()]); + assert!(!h.state_mut().save_active(SaveTarget::InPlace)); + h.run_steps(3); + let saved = std::fs::read(©).unwrap(); + // A second Apply redactions makes Save ask where. Before the answer, Undo takes back the + // second Apply, its marks and the saved Apply. + apply(&mut h); + assert!(!h.state_mut().save_active(SaveTarget::InPlace)); + for _ in 0..3 { + h.state_mut().execute("edit.undo"); + } + assert_eq!(h.state().views[0].redactions_undone_after_save, 1); + let elsewhere = dir.join("contract_restored.pdf"); + h.state_mut().pick_override = Some(vec![elsewhere.to_string_lossy().into_owned()]); + h.run_steps(4); + h.state_mut().pick_override = None; + assert_eq!(std::fs::read(©).unwrap(), saved, "the redacted copy is left as it was"); + assert!(elsewhere.exists(), "Save asked again and wrote where the user chose"); + let _ = std::fs::remove_dir_all(&dir); +} + /// Undo of Apply redactions brings back Save in place; Redo makes Save ask where again. #[test] fn undoing_apply_redactions_saves_in_place_again() {