From e09fcba9180e842c3eba0da0c079fb165d0a3a58 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 28 Oct 2026 06:24:26 -0700 Subject: [PATCH] Redact: Undo past a saved Apply redactions makes Save ask where Undo of an Apply redactions step that was already saved brought back the redacted content, and Save then wrote it in place over the _Redacted file. The view now counts the saved steps that Undo takes back, and Save asks where while any are undone. A new Apply redactions made while saved steps are undone clears Redo, so Save keeps asking until the next save or Revert. --- crates/ui-egui/src/canvas.rs | 9 +++ crates/ui-egui/src/dialogs.rs | 5 ++ crates/ui-egui/src/editing.rs | 11 ++- crates/ui-egui/src/editing/close.rs | 2 + crates/ui-egui/src/editing/save.rs | 7 +- crates/ui-egui/tests/it/redact.rs | 111 ++++++++++++++++++++++++++++ 6 files changed, 142 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index f8eeccb18e..695ca7621b 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -455,6 +455,13 @@ pub struct DocView { /// desktop save set it to 0; after a desktop save the document has the name it was saved /// under. A web download keeps it, so later downloads carry the `_Redacted` name. pub redactions_applied: usize, + /// How many Apply redactions steps saved into the document's file are undone in this view. + /// While it is above 0, Save asks where, so the restored content is not written over the + /// redacted file. Redo takes one off. Revert, Reload and a desktop save set it to 0. + pub redactions_undone_after_save: usize, + /// Set when Apply redactions runs while saved Apply redactions steps are undone: Redo can no + /// longer bring those steps back, so Save asks where until a desktop save or Revert clears it. + pub saved_redactions_cut_off: bool, /// A crop rectangle being dragged (Crop tool). pub crop_drag: crate::crop::CropDrag, /// Enhance ▸ Camera image: the quadrilateral being placed. @@ -718,6 +725,8 @@ impl DocView { redact_drag: None, pending_redaction: None, redactions_applied: 0, + redactions_undone_after_save: 0, + saved_redactions_cut_off: false, crop_drag: None, camera: None, camera_done: None, diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index f927e00fc6..7a5bdda2d7 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1685,6 +1685,11 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { if app.apply_edit(Edit::ApplyRedactions { pages: None }) { if let Some(view) = app.active.and_then(|i| app.views.get_mut(i)) { view.redactions_applied = view.redactions_applied.saturating_add(1); + // A new step clears the redo history, so the undone saved steps can't come back. + if view.redactions_undone_after_save > 0 { + view.redactions_undone_after_save = 0; + view.saved_redactions_cut_off = true; + } } if marks == 1 { app.notify_tr("Applied 1 redaction mark. Save to remove the content from the file."); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 4e1f123969..17bb319c2c 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -221,8 +221,15 @@ impl PdfCraftApp { } self.views[i].comments.selected = None; if label == (Edit::ApplyRedactions { pages: None }).label() { - let applied = &mut self.views[i].redactions_applied; - *applied = if undo { applied.saturating_sub(1) } else { applied.saturating_add(1) }; + let view = &mut self.views[i]; + let (applied, undone) = (view.redactions_applied, view.redactions_undone_after_save); + // Undo past the last save counts as undone; Redo first takes those back. + match (undo, applied, undone) { + (true, 0, _) => view.redactions_undone_after_save = undone.saturating_add(1), + (true, _, _) => view.redactions_applied = applied.saturating_sub(1), + (false, _, 0) => view.redactions_applied = applied.saturating_add(1), + (false, _, _) => view.redactions_undone_after_save = undone.saturating_sub(1), + } } let label = crate::i18n::edit_label(&label); if undo { diff --git a/crates/ui-egui/src/editing/close.rs b/crates/ui-egui/src/editing/close.rs index e93d457a0f..af8040b07e 100644 --- a/crates/ui-egui/src/editing/close.rs +++ b/crates/ui-egui/src/editing/close.rs @@ -56,6 +56,8 @@ impl PdfCraftApp { view.pending_edit = None; // The redactions applied since the last save are discarded too. view.redactions_applied = 0; + view.redactions_undone_after_save = 0; + view.saved_redactions_cut_off = false; } self.notify_tr("Reverted to the last saved version"); } diff --git a/crates/ui-egui/src/editing/save.rs b/crates/ui-egui/src/editing/save.rs index 02093188e8..690619c689 100644 --- a/crates/ui-egui/src/editing/save.rs +++ b/crates/ui-egui/src/editing/save.rs @@ -51,7 +51,10 @@ impl PdfCraftApp { // After Apply redactions, Save asks where and proposes the name with `_Redacted`. let redacted = self.views.get(index).is_some_and(|v| v.redactions_applied > 0); let name = if redacted { pdfcraft_engine::redacted_file_name(&name) } else { name }; - let target = if redacted { SaveTarget::As } else { target }; + // Undo past a saved Apply redactions brings back content that the file on disk has + // redacted: ask where instead of writing it over that file. + let unredacted = self.views.get(index).is_some_and(|v| v.redactions_undone_after_save > 0 || v.saved_redactions_cut_off); + let target = if redacted || unredacted { SaveTarget::As } else { target }; #[cfg(not(target_arch = "wasm32"))] { let doc_path = path.clone(); @@ -205,6 +208,8 @@ impl PdfCraftApp { // Saving over a version that changed on disk keeps this one: nothing to ask. view.disk_changed = false; view.redactions_applied = 0; + view.redactions_undone_after_save = 0; + view.saved_redactions_cut_off = false; } self.notify_fmt("Saved {name}", &[("name", &short_name(dest))]); #[cfg(target_os = "macos")] diff --git a/crates/ui-egui/tests/it/redact.rs b/crates/ui-egui/tests/it/redact.rs index 514259b918..747a97cb9a 100644 --- a/crates/ui-egui/tests/it/redact.rs +++ b/crates/ui-egui/tests/it/redact.rs @@ -118,6 +118,117 @@ fn saving_after_applying_redactions_asks_where() { let _ = std::fs::remove_dir_all(&dir); } +/// Undo past an Apply redactions that is already saved in the `_Redacted` copy makes Save ask +/// where, so the restored content is not written over the copy. Redo makes Save write in place. +#[test] +fn undoing_saved_redactions_does_not_save_over_the_redacted_copy() { + let dir = std::env::temp_dir().join(format!("pdfcraft-redacted-saved-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); + 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); + 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); + h.state_mut().pick_override = None; + assert!(copy.exists()); + // Undo and Redo leave the saved redactions in effect: Save writes the copy in place. + h.state_mut().execute("edit.undo"); + h.run_steps(3); + h.state_mut().execute("edit.redo"); + h.run_steps(3); + assert_eq!((h.state().views[0].redactions_applied, h.state().views[0].redactions_undone_after_save), (0, 0)); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "with the redactions back, Save writes the copy in place"); + let saved = std::fs::read(©).unwrap(); + h.state_mut().execute("edit.undo"); + h.run_steps(3); + 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()]); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "Save asks where instead of writing over the copy"); + h.run_steps(3); + h.state_mut().pick_override = None; + assert_eq!(std::fs::read(©).unwrap(), saved, "the redacted copy is left as it was"); + assert!(elsewhere.exists(), "the restored version is written where the user chose"); + assert_eq!(h.state().views[0].redactions_undone_after_save, 0); + // The restored version is now the saved one: Redo applies the redactions to it again. + h.state_mut().execute("edit.redo"); + h.run_steps(3); + assert_eq!(h.state().views[0].redactions_applied, 1); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A new Apply redactions after Undo past a saved one clears Redo, so Undo and Redo of the new +/// step do not bring Save in place back over the `_Redacted` copy. +#[test] +fn redoing_a_new_apply_after_undoing_saved_redactions_still_asks_where() { + let dir = std::env::temp_dir().join(format!("pdfcraft-redacted-branch-{}", 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); + 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); + 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); + h.state_mut().pick_override = None; + let saved = std::fs::read(©).unwrap(); + h.state_mut().execute("edit.undo"); + h.run_steps(3); + // Apply the marks again as a new step, then Undo and Redo it. + assert!(h.state_mut().execute("redact.apply")); + h.run_steps(2); + h.get_by_label("Apply").click(); + h.run_steps(4); + h.state_mut().execute("edit.undo"); + h.run_steps(3); + let view = &h.state().views[0]; + assert_eq!((view.redactions_applied, view.redactions_undone_after_save, view.saved_redactions_cut_off), (0, 0, true)); + h.state_mut().execute("edit.redo"); + h.run_steps(3); + let elsewhere = dir.join("contract_again.pdf"); + h.state_mut().pick_override = Some(vec![elsewhere.to_string_lossy().into_owned()]); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "Save asks where instead of writing over the copy"); + h.run_steps(3); + h.state_mut().pick_override = None; + assert_eq!(std::fs::read(©).unwrap(), saved, "the redacted copy is left as it was"); + assert!(!h.state().views[0].saved_redactions_cut_off); + 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() {