From b4dc90b2ced9163bb08818bee2a04c9f536f1cc3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 30 Oct 2026 22:19:37 -0700 Subject: [PATCH] Redact: Undo past saved redactions while the Save dialog is open asks again The Save dialog proposes its name when it opens, and the document's bytes are taken once the user chooses. An Undo past a saved Apply redactions in between wrote the restored content to the redacted copy the dialog had proposed. Save now asks again, proposing the name for the restored content. --- crates/ui-egui/src/editing/save.rs | 18 ++++++++--- crates/ui-egui/tests/it/redact.rs | 50 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) 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() {