fix(redact): clearing marks leaves the page as it was and takes replies with them

Clear all wrote `/Annots []` on a page that had no annotations and replaced
an indirect `/Annots` array with an inline copy. It also left replies to a
mark in place, so their `/IRT` kept the cleared mark in the document (and in
the comment list, as a reply to nothing).

`clear_marks` now writes the list through the annot crate's `set_annots`
(made public), as deleting a comment does, and removes each mark's replies
and pop-ups with it. The "what goes with a deleted comment" walk moves from
`delete_annotation` into `with_replies_and_popups`, which both use; it visits
each annotation once instead of sweeping the list until nothing changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/annot/src/lib.rs

@@ -14,6 +14,8 @@
 
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
+use std::collections::{HashMap, HashSet};
+
 use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, Stream};
 
 pub mod appearance;
@@ -774,8 +776,9 @@ thread_local! {
     static ANNOTS_ENTRIES_COPIED: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
 }
 
-/// Replace a page's `/Annots`, writing into the shared array object when it is indirect.
-fn set_annots(doc: &mut Document, page: ObjRef, list: Vec<Object>) -> Result<(), AnnotError> {
+/// Replace a page's `/Annots`, writing into the shared array object when it is indirect. An empty
+/// list removes a direct `/Annots`.
+pub fn set_annots(doc: &mut Document, page: ObjRef, list: Vec<Object>) -> Result<(), pdfcraft_cos::CosError> {
     let existing = doc.get(page).as_dict().and_then(|d| d.reference(b"Annots"));
     match existing {
         Some(r) if doc.get(r).as_array().is_some() => doc.set(r, Object::Array(list)),
@@ -1647,36 +1650,35 @@ pub fn delete_annotation(doc: &mut Document, page: usize, index: usize) -> Resul
     if let Some(r) = target.as_ref() {
         unlocked(doc, r)?;
     }
-    let mut doomed: Vec<ObjRef> = target.as_ref().into_iter().collect();
-    // Replies (`/IRT`) and pop-ups (`/Parent`) of anything doomed, to a fixpoint.
-    loop {
-        let before = doomed.len();
-        for e in &list {
-            let Some(r) = e.as_ref() else { continue };
-            if doomed.contains(&r) {
-                continue;
-            }
-            let obj = doc.get(r);
-            let Some(d) = obj.as_dict() else { continue };
-            let points_at = |k: &[u8]| d.reference(k).is_some_and(|t| doomed.contains(&t));
-            if points_at(b"IRT") || points_at(b"Parent") {
-                doomed.push(r);
-            }
-        }
-        for r in doomed.clone() {
-            if let Some(pop) = doc.get(r).as_dict().and_then(|d| d.reference(b"Popup"))
-                && !doomed.contains(&pop)
-            {
-                doomed.push(pop);
-            }
-        }
-        if doomed.len() == before {
-            break;
-        }
-    }
+    let doomed = with_replies_and_popups(doc, &list, target.as_ref());
     let kept: Vec<Object> =
         list.into_iter().enumerate().filter(|(i, e)| *i != index && !e.as_ref().is_some_and(|r| doomed.contains(&r))).map(|(_, e)| e).collect();
-    set_annots(doc, p, kept)
+    Ok(set_annots(doc, p, kept)?)
+}
+
+/// `doomed` and what goes with it, as Acrobat deletes a comment: the entries of `list` (one or
+/// more pages' `/Annots`) that reply to (`/IRT`) or pop up for (`/Parent`) anything going, their
+/// own replies, and the pop-ups going annotations name (`/Popup`). Each annotation is visited
+/// once, so a long or cyclic chain costs no more than its length.
+pub fn with_replies_and_popups(doc: &Document, list: &[Object], doomed: impl IntoIterator<Item = ObjRef>) -> HashSet<ObjRef> {
+    let mut hanging: HashMap<ObjRef, Vec<ObjRef>> = HashMap::new();
+    for r in list.iter().filter_map(Object::as_ref) {
+        let obj = doc.get(r);
+        let Some(d) = obj.as_dict() else { continue };
+        for on in [d.reference(b"IRT"), d.reference(b"Parent")].into_iter().flatten() {
+            hanging.entry(on).or_default().push(r);
+        }
+    }
+    let mut going = HashSet::new();
+    let mut todo: Vec<ObjRef> = doomed.into_iter().collect();
+    while let Some(r) = todo.pop() {
+        if !going.insert(r) {
+            continue;
+        }
+        todo.extend(hanging.get(&r).into_iter().flatten());
+        todo.extend(doc.get(r).as_dict().and_then(|d| d.reference(b"Popup")));
+    }
+    going
 }
 
 /// Change a comment's text. Text boxes are redrawn to show it, and their rectangle follows the

crates/annot/src/links.rs

@@ -320,7 +320,7 @@ pub fn delete(doc: &mut Document, page: usize, index: usize) -> Result<(), Annot
     let (p, _) = link_at(doc, page, index)?;
     let mut list = annots(doc, p);
     list.remove(index);
-    set_annots(doc, p, list)
+    Ok(set_annots(doc, p, list)?)
 }
 
 /// Remove every link on `pages` (all when `None`). Returns how many went.

crates/redact/src/lib.rs

@@ -699,27 +699,24 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Redac
     Ok(report)
 }
 
-/// Remove redaction marks without applying them (`None` = all).
+/// Remove redaction marks without applying them (`None` = all), with their replies and pop-ups as
+/// deleting a comment does: a reply left behind would keep its mark in the file through `/IRT`.
+/// Returns how many marks went.
 pub fn clear_marks(doc: &mut Document, pages: Option<&[usize]>) -> Result<usize, RedactError> {
-    let all = marks(doc);
-    let doomed: Vec<ObjRef> = all.iter().filter(|m| pages.is_none_or(|p| p.contains(&m.page))).map(|m| m.obj).collect();
-    if doomed.is_empty() {
+    let targets: HashSet<ObjRef> = marks(doc).iter().filter(|m| pages.is_none_or(|p| p.contains(&m.page))).map(|m| m.obj).collect();
+    if targets.is_empty() {
         return Err(RedactError::NothingToApply);
     }
+    let listed: Vec<Object> = pdfcraft_model::pages(doc).iter().flat_map(|p| annots_of(doc, &p.dict)).collect();
+    let doomed = pdfcraft_annot::with_replies_and_popups(doc, &listed, targets.iter().copied());
     for p in pdfcraft_model::pages(doc) {
         let annots = annots_of(doc, &p.dict);
-        let kept: Vec<Object> = annots
-            .iter()
-            .filter(|a| {
-                let Some(r) = a.as_ref() else { return true };
-                let parent = doc.get(r).as_dict().and_then(|d| d.get(b"Parent")).and_then(Object::as_ref);
-                !doomed.contains(&r) && !parent.is_some_and(|p| doomed.contains(&p))
-            })
-            .cloned()
-            .collect();
+        let kept: Vec<Object> = annots.iter().filter(|a| !a.as_ref().is_some_and(|r| doomed.contains(&r))).cloned().collect();
+        // As deleting a comment does: a page whose marks were all it had loses `/Annots` again,
+        // and an indirect array stays the one object the page points to.
         if kept.len() != annots.len() {
-            doc.update_dict(p.obj, |d| d.set(b"Annots".to_vec(), Object::Array(kept)))?;
+            pdfcraft_annot::set_annots(doc, p.obj, kept)?;
         }
     }
-    Ok(doomed.len())
+    Ok(targets.len())
 }

crates/redact/src/tests.rs

@@ -1041,6 +1041,34 @@ fn nothing_to_apply_and_clearing_marks() {
     assert!(content(&doc, 0).contains("(AB) Tj"));
 }
 
+/// Clearing marks puts a page's annotation list back as it was: a page that had none has no
+/// `/Annots` again, replies to the marks included, and one listed through an indirect array keeps
+/// pointing at that array.
+#[test]
+fn clearing_marks_leaves_the_annotation_list_as_it_was() {
+    let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (AB) Tj ET", "", vec![]);
+    let page = pdfcraft_model::pages(&doc)[0].obj;
+    let before = doc.get(page);
+    mark(&mut doc, 0, &[[0.0, 0.0, 50.0, 50.0]], "");
+    pdfcraft_annot::add_reply(&mut doc, 0, 0, "a reply", "Tester", &Meta::default()).unwrap();
+    assert_eq!(clear_marks(&mut doc, None), Ok(1), "one mark");
+    assert_eq!(doc.get(page), before, "the page has no /Annots again: the reply went with its mark");
+
+    let mut doc = pdf(vec![
+        b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(),
+        b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
+        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Annots 4 0 R >>".to_vec(),
+        b"[5 0 R]".to_vec(),
+        b"<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] >>".to_vec(),
+    ]);
+    let (page, list) = (ObjRef::new(3, 0), ObjRef::new(4, 0));
+    let before = (doc.get(page), doc.get(list));
+    mark(&mut doc, 0, &[[0.0, 0.0, 50.0, 50.0]], "");
+    assert_eq!(doc.get(list).as_array().map(|a| a.len()), Some(2), "the mark is listed in the shared array");
+    clear_marks(&mut doc, None).unwrap();
+    assert_eq!((doc.get(page), doc.get(list)), before, "the page points at its array, which lists the link alone");
+}
+
 #[test]
 fn unreadable_content_fails_closed() {
     let mut doc = one_page(b"", "", vec![]);