From 3e025b91436c45d5b515a2de058d44fb2dcb3733 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:44:54 +0800 Subject: [PATCH] M8: the places applied redactions leave holding the removed text are an EditNotice (#2448) Main's `EditNotice` is "something the last edit did that the user should hear about", set by `Session::apply` and cleared by the next edit, undo and redo, which is what the PR's separate `redaction_elsewhere` field did by hand. It is now `EditNotice::RedactionElsewhere(Elsewhere)`, pushed when a place is counted or the text couldn't be searched for: tools list it in `warnings` with the other notices, the interface says it through the same path, and `Document::redaction_elsewhere()` gives `redact_apply` and the Apply dialog the counts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/automation/src/redact.rs | 2 +- crates/automation/tests/it/automation.rs | 8 ++++ crates/engine/src/document.rs | 54 +++++++++++++++++++++--- crates/engine/src/open.rs | 1 - crates/engine/src/session/editing.rs | 8 ++-- crates/engine/src/tests.rs | 9 ++-- crates/ui-egui/src/dialogs.rs | 4 +- crates/ui-egui/src/editing.rs | 6 ++- 8 files changed, 71 insertions(+), 21 deletions(-) diff --git a/crates/automation/src/redact.rs b/crates/automation/src/redact.rs index baea3d75c5..4b95736613 100644 --- a/crates/automation/src/redact.rs +++ b/crates/automation/src/redact.rs @@ -166,7 +166,7 @@ impl Automation { out["applied"] = json!(before - after); out["marks_pending"] = json!(after); // Where the removed text is still held that applying leaves as it is. - let left = self.doc(a)?.redaction_elsewhere.unwrap_or_default(); + let left = self.doc(a)?.redaction_elsewhere().unwrap_or_default(); out["still_holding"] = json!({ "bookmarks": left.bookmarks, "attachments": left.attachments, diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index bf2d1b3ceb..653c18c4bf 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -4365,6 +4365,14 @@ fn redact_apply_reports_places_that_still_hold_the_removed_text() { ok(&mut a, "redact_mark", json!({ "doc": doc, "find": "Bluefin" })); let r = ok(&mut a, "redact_apply", json!({ "doc": doc })); assert_eq!(r["still_holding"], json!({ "bookmarks": 1, "attachments": 0, "comments": 1, "xmp": 0, "total": 2, "unsearched": false }), "{r}"); + // Said in the warnings too, with the other edit notices. + assert_eq!( + r["warnings"], + json!([ + "the redacted text also appears in 1 bookmark title and 1 comment, which redaction doesn't change (Remove hidden information removes them)" + ]), + "{r}" + ); assert!(!page_text(&mut a, doc)[0].contains("Bluefin")); let titles: Vec = ok(&mut a, "bookmark_list", json!({ "doc": doc }))["bookmarks"] .as_array() diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index fa0bedafad..863f370a72 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -60,14 +60,10 @@ pub struct Document { /// (also in `info.warnings`, kept there when the document is re-read). pub xfa_warnings: Vec, /// What the last successful [`Session::apply`](crate::Session::apply) wants said (see - /// [`EditNotice`]): text its fonts can't draw, pages whose own backdrop stays off. The - /// interface shows it; tools return it. + /// [`EditNotice`]): text its fonts can't draw, pages whose own backdrop stays off, places + /// applied redactions left that still hold the removed text. The interface shows it; tools + /// return it. pub edit_notices: Vec, - /// What the last successful [`Session::apply`](crate::Session::apply), when it applied redactions, - /// found still holding the removed text in places applying leaves as they are (bookmark titles, - /// attachments, remaining comments, other XMP; see [`RedactionElsewhere`](crate::RedactionElsewhere)). - /// `None` after any other edit, an undo or a redo. The interface says so; tools return it. - pub redaction_elsewhere: Option, } /// Something the last successful edit did that the user should hear about, never left silent @@ -81,6 +77,11 @@ pub enum EditNotice { /// turning it on could paint over something (`pdfcraft_edit::backdrops_left_off`). They look /// the same on white paper. BackdropsLeftOff { pages: Vec }, + /// Applying redactions left these places as they were, and they still hold the removed text + /// (bookmark titles, attachments, the comments that remain, XMP on other objects; see + /// [`RedactionElsewhere`](crate::RedactionElsewhere)). Only when one is counted, or the text + /// couldn't be searched for. + RedactionElsewhere(pdfcraft_redact::Elsewhere), } impl std::fmt::Display for EditNotice { @@ -95,10 +96,39 @@ impl std::fmt::Display for EditNotice { page_runs(pages) ) } + EditNotice::RedactionElsewhere(e) => elsewhere_text(e, f), } } } +/// [`EditNotice::RedactionElsewhere`] in words (English; the interface has its own wording). +fn elsewhere_text(e: &pdfcraft_redact::Elsewhere, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let count = |n: usize, one: &str, many: &str| match n { + 0 => None, + 1 => Some(format!("1 {one}")), + n => Some(format!("{n} {many}")), + }; + let places: Vec = [ + count(e.bookmarks, "bookmark title", "bookmark titles"), + count(e.attachments, "attachment", "attachments"), + count(e.comments, "comment", "comments"), + count(e.xmp, "other metadata stream", "other metadata streams"), + ] + .into_iter() + .flatten() + .collect(); + let list = match places.split_last() { + None => "no place counted".to_string(), + Some((last, [])) => last.clone(), + Some((last, rest)) => format!("{} and {last}", rest.join(", ")), + }; + if e.unsearched { + write!(f, "the redacted text couldn't be searched for, so {list} may still hold it (Remove hidden information removes them)") + } else { + write!(f, "the redacted text also appears in {list}, which redaction doesn't change (Remove hidden information removes them)") + } +} + /// 0-based pages as 1-based runs: `[0, 1, 2, 5]` is "1-3, 6". pub(crate) fn page_runs(pages: &[usize]) -> String { let mut runs: Vec<(usize, usize)> = Vec::new(); @@ -185,6 +215,16 @@ pub(crate) fn undrawn_notices(prev: &pdfcraft_cos::Document, next: &pdfcraft_cos } impl Document { + /// The places the last edit's applied redactions left that still hold the removed text + /// ([`EditNotice::RedactionElsewhere`]), or `None` when there are none or the last edit + /// applied none. + pub fn redaction_elsewhere(&self) -> Option { + self.edit_notices.iter().find_map(|n| match n { + EditNotice::RedactionElsewhere(e) => Some(*e), + _ => None, + }) + } + /// How the document opens (Document Properties ▸ Initial View). /// Accessibility ▸ Check for accessibility: the full check (`None` if the document can't be /// read for editing). diff --git a/crates/engine/src/open.rs b/crates/engine/src/open.rs index c43ac354d1..32db07abee 100644 --- a/crates/engine/src/open.rs +++ b/crates/engine/src/open.rs @@ -352,7 +352,6 @@ impl Session { fonts_stale: false, xfa_warnings: Vec::new(), edit_notices: Vec::new(), - redaction_elsewhere: None, } } } diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index d7e84f09f6..023ce138ab 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -26,7 +26,6 @@ impl Session { let xfa_time_scale = self.xfa_time_scale.unwrap_or(1); let doc = self.doc_mut(id)?; doc.edit_notices.clear(); - doc.redaction_elsewhere = None; let name = doc.name.clone(); let is_xfa = doc.info.xfa.is_some(); let mut scope = scope_of(&edit); @@ -182,7 +181,9 @@ impl Session { if signed && ((rewrites(&next) && !rewrites(&editor.cos)) || (next.is_modified() && next.revisions().is_empty())) { return Err(EditError::SignedRewrite(edit.label().to_string())); } - let notices = edit_notices(&edit, &editor.cos, &next); + let mut notices = edit_notices(&edit, &editor.cos, &next); + // Places applied redactions left that still hold the removed text: said, not silent. + notices.extend(cx.redaction_elsewhere.filter(|e| e.total() > 0 || e.unsearched).map(EditNotice::RedactionElsewhere)); let previous = std::mem::replace(&mut editor.cos, next); let keys = keys_after(&edit).unwrap_or_else(|| editor.keys.clone()); let previous_keys = std::mem::replace(&mut editor.keys, keys); @@ -211,7 +212,6 @@ impl Session { doc.dirty = true; doc.generation += 1; doc.edit_notices = notices; - doc.redaction_elsewhere = cx.redaction_elsewhere; if !cx.xfa_removed.is_empty() { let removed: Vec = cx.xfa_removed.iter().map(|n| format!("{XFA_REMOVED_PREFIX}{n}")).collect(); doc.xfa_warnings.retain(|w| w.starts_with(XFA_REMOVED_PREFIX)); @@ -257,7 +257,6 @@ impl Session { return Err(e); } doc.edit_notices.clear(); - doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) @@ -287,7 +286,6 @@ impl Session { return Err(e); } doc.edit_notices.clear(); - doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8925bff358..91ad63e783 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2992,13 +2992,16 @@ fn applied_redactions_report_a_bookmark_that_still_holds_the_removed_text() { s.apply(id, Edit::AddBookmark { parent: vec![], index: 0, title: "Bluefin plans".into(), page: 0, left: None, top: None }).unwrap(); s.apply(id, Edit::AddBookmark { parent: vec![], index: 1, title: "Annual figures".into(), page: 0, left: None, top: None }).unwrap(); mark_shown_text(&mut s, id, "BLUEFIN"); - assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "only applying redactions reports it"); + assert_eq!(s.get(id).unwrap().redaction_elsewhere(), None, "only applying redactions reports it"); s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); - let found = s.get(id).unwrap().redaction_elsewhere.expect("applying reports what it left"); + let found = s.get(id).unwrap().redaction_elsewhere().expect("applying reports what it left"); assert_eq!(found, crate::RedactionElsewhere { bookmarks: 1, ..Default::default() }); + // One of the edit notices, so tools and the interface say it with the others. + let said: Vec = s.get(id).unwrap().edit_notices.iter().map(ToString::to_string).collect(); + assert_eq!(said, ["the redacted text also appears in 1 bookmark title, which redaction doesn't change (Remove hidden information removes them)"]); assert_eq!(outline_titles(&s.get(id).unwrap().info.outline), ["Bluefin plans→1", "Annual figures→1"], "the bookmark is reported, not changed"); s.undo(id).unwrap(); - assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "an undo forgets it"); + assert_eq!(s.get(id).unwrap().redaction_elsewhere(), None, "an undo forgets it"); } /// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index dc89a87a12..75f6e1463c 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1726,7 +1726,7 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { }; // Where the removed text is still held that applying leaves as it is: said in the // same notification, so it can't replace the first sentence. - let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere); + let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere()); match left.and_then(|e| still_holding_note(&e)) { Some(note) => app.notify(format!("{applied} {note}")), None => app.notify(applied), @@ -1782,7 +1782,7 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { /// The sentence telling the user where the removed text is still held that applying redactions /// leaves as it is, or `None` when it is nowhere else. When the text couldn't be searched for, /// the places are those holding any text, and the sentence says they may hold it. -fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Option { +pub(crate) fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Option { let count = |n: usize, one: &str, many: &str| match n { 0 => None, 1 => Some(tl!(one).to_string()), diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 8def569f58..517609d0fa 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -171,12 +171,14 @@ impl PdfCraftApp { self.quick_tool = crate::QuickTool::Select; } // Text saved as typed but drawn with `?` (its font can't draw it), or pages whose - // own backdrop stays off: said, not silent. Before the scripts' output, so an alert - // they raise still shows. + // own backdrop stays off, or places applied redactions left holding the removed text + // (the Apply redactions dialog then says it after its count): said, not silent. + // Before the scripts' output, so an alert they raise still shows. if let Some(n) = self.session.get(id).and_then(|d| d.edit_notices.first()).cloned() { self.notify(match &n { pdfcraft_engine::EditNotice::Undrawn(t) => crate::content_ui::undrawn_message(t), pdfcraft_engine::EditNotice::BackdropsLeftOff { pages } => crate::marks_ui::backdrops_left_off_message(pages), + pdfcraft_engine::EditNotice::RedactionElsewhere(e) => crate::dialogs::still_holding_note(e).unwrap_or_else(|| n.to_string()), }); } let out = self.session.take_js_output(id);