fix(annot,ui-egui): guard comment edits against non-comments and stale state (#3916) (#4673)

crates/annot/src/info.rs

@@ -123,7 +123,7 @@ pub fn is_comment_subtype(subtype: &str) -> bool {
 
 /// Where a reference to a reference (`5 0 obj 6 0 R endobj`) ends, as `pdfcraft_render::inspect`
 /// follows it, within a few steps (a cycle stops there).
-fn final_ref(doc: &Document, mut r: ObjRef) -> ObjRef {
+pub(crate) fn final_ref(doc: &Document, mut r: ObjRef) -> ObjRef {
     for _ in 0..8 {
         match &*doc.get(r) {
             Object::Ref(next) => r = *next,
@@ -340,6 +340,9 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option<Props> {
     let direct = crate::draw::with_direct_values(doc, raw);
     let d = direct.as_ref();
     let subtype = String::from_utf8_lossy(d.name(b"Subtype")?).into_owned();
+    if !is_comment_subtype(&subtype) {
+        return None;
+    }
     // A text box reports the colour and size it is drawn in (`/DA`, or `/DS` where it sets one).
     let style = (subtype == "FreeText").then(|| text_style::text_style(d, None));
     let color = match &style {

crates/annot/src/lookup.rs

@@ -1,4 +1,4 @@
-use super::info::text_value;
+use super::info::{is_comment_subtype, text_value};
 use super::types::{FLAG_HIDDEN, FLAG_NO_ROTATE, FLAG_NO_VIEW, Shape};
 use super::{AnnotError, Dict, Document, ObjRef, Object, appearance};
 
@@ -106,16 +106,23 @@ pub fn set_annots(doc: &mut Document, page: ObjRef, list: Vec<Object>) -> Result
     Ok(())
 }
 
-/// The annotation at `(page, index)` as an indirect object (inline dictionaries are promoted).
+/// Whether the annotation dictionary `d` has a comment `/Subtype` ([`is_comment_subtype`]).
+pub(crate) fn is_comment_dict(d: &Dict) -> bool {
+    d.name(b"Subtype").is_some_and(|s| is_comment_subtype(&String::from_utf8_lossy(s)))
+}
+
+/// The comment annotation at `(page, index)` as an indirect object (inline dictionaries are promoted).
 pub(crate) fn annot_ref(doc: &mut Document, page: usize, index: usize) -> Result<(ObjRef, ObjRef), AnnotError> {
     let p = page_ref(doc, page)?;
     let mut list = annots(doc, p);
     let entry = list.get(index).cloned().ok_or(AnnotError::NoSuchAnnotation { page, index })?;
     let r = match entry {
-        Object::Ref(r) if doc.get(r).as_dict().is_some() => r,
-        Object::Dict(d) => {
+        Object::Ref(r) if doc.get(r).as_dict().is_some_and(is_comment_dict) => r,
+        Object::Dict(d) if is_comment_dict(&d) => {
             let r = doc.add(Object::Dict(d));
-            list[index] = Object::Ref(r);
+            if let Some(slot) = list.get_mut(index) {
+                *slot = Object::Ref(r);
+            }
             set_annots(doc, p, list)?;
             r
         }

crates/annot/src/modify.rs

@@ -133,6 +133,10 @@ fn deleted_index(doc: &Document, page: usize, index: usize) -> Result<usize, Ann
     let list = annots(doc, page_ref(doc, page)?);
     let mut target = list.get(index).ok_or(AnnotError::NoSuchAnnotation { page, index })?;
     let mut index = index;
+    let target_obj = target.as_ref().map_or_else(|| doc.resolve(target), |r| doc.get(super::info::final_ref(doc, r)));
+    if !target_obj.as_dict().is_some_and(super::lookup::is_comment_dict) {
+        return Err(AnnotError::NoSuchAnnotation { page, index });
+    }
     if let Some(r) = target.as_ref() {
         unlocked(doc, r)?;
     }

crates/annot/src/tests.rs

@@ -2544,6 +2544,28 @@ fn non_markup_annotations_are_not_comments() {
     for s in ["Text", "FreeText", "Highlight", "Ink", "Stamp", "FileAttachment", "Redact", "Sound"] {
         assert!(is_comment_subtype(s), "{s}");
     }
+    // #3916: comment edits must refuse non-comment annotations (form widgets, links, pop-ups)
+    // when a stale `/Annots` index points at one.
+    for subtype in ["Widget", "Link", "Popup", "Watermark"] {
+        let mut doc = fixture();
+        let mut d = Dict::new();
+        d.set(b"Type".to_vec(), Object::name("Annot"));
+        d.set(b"Subtype".to_vec(), Object::name(subtype));
+        d.set(b"Rect".to_vec(), num_array(&[10.0, 10.0, 50.0, 30.0]));
+        d.set(b"Contents".to_vec(), PdfString::text("keep me"));
+        let w = doc.add(Object::Dict(d));
+        let p = page_refs(&doc).unwrap()[0];
+        let mut a = annots(&doc, p);
+        a.push(Object::Ref(w));
+        let idx = a.len() - 1;
+        set_annots(&mut doc, p, a).unwrap();
+
+        assert_eq!(delete_annotation(&mut doc, 0, idx), Err(AnnotError::NoSuchAnnotation { page: 0, index: idx }), "{subtype}");
+        assert_eq!(set_contents(&mut doc, 0, idx, "overwritten", &meta("m")), Err(AnnotError::NoSuchAnnotation { page: 0, index: idx }), "{subtype}");
+        assert_eq!(move_annotation(&mut doc, 0, idx, 5.0, 5.0, &meta("m")), Err(AnnotError::NoSuchAnnotation { page: 0, index: idx }), "{subtype}");
+        assert_eq!(props(&doc, 0, idx), None, "{subtype}");
+        assert_eq!(text(&list(&doc, 0)[idx], b"Contents"), "keep me", "{subtype} contents must be untouched");
+    }
 }
 
 /// A drawing (a drawn Fill & Sign signature is one) resizes by scaling its strokes into the new

crates/ui-egui/src/canvas.rs

@@ -1134,7 +1134,7 @@ impl DocView {
             page_widget: None,
             pan_hold_until: 0.0,
             slow_pages: HashSet::new(),
-            comments: Default::default(),
+            comments: crate::comments::CommentView::from_info(info),
             measure: Default::default(),
             forms: Default::default(),
             prepare: Default::default(),
@@ -1192,6 +1192,7 @@ impl DocView {
         // Thumbnail drags hold page indexes from before the change.
         self.panel_drag = None;
         self.org_drag = None;
+        self.comments.document_changed(info);
         self.reset_coarse_state();
     }
 
@@ -3465,7 +3466,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) {
     // One crop, then back to selecting (as Acrobat does).
     let cropped = view.pending_edit.as_ref().is_some_and(|e| matches!(e, pdfcraft_engine::Edit::SetPageBox { .. }));
     let mut tool = app.quick_tool;
-    comments::keys(ui.ctx(), view, &mut tool, allowed);
+    comments::keys(ui.ctx(), view, info, &mut tool, allowed);
     if preparing {
         crate::prepare::keys(ui.ctx(), view);
     }

crates/ui-egui/src/editing.rs

@@ -28,6 +28,16 @@ fn new_text_typed(v: &crate::DocView) -> bool {
     v.content.draft.as_ref().is_some_and(|d| d.index.is_none() && !d.text.trim().is_empty())
 }
 
+/// Clear an unfinished page-anchored drawing or composer when a page edit shifts or removes `page`.
+fn clear_shifted_page_comment_state(view: &mut crate::DocView, shifted_from: usize) {
+    if matches!(view.comments.gesture, Some(crate::comments::Gesture::Draw { page, .. }) if page >= shifted_from) {
+        view.comments.gesture = None;
+    }
+    if view.comments.composer.as_ref().is_some_and(|c| !matches!(c.kind, crate::comments::ComposerKind::Edit(_)) && c.page >= shifted_from) {
+        view.comments.composer = None;
+    }
+}
+
 /// Keeps the pages (or comment) the user acted on selected, where they now are. `annots_before`
 /// is the page's annotations before a delete or a paste of comments (empty for other edits).
 fn select_after_edit(
@@ -39,17 +49,27 @@ fn select_after_edit(
 ) {
     match edit {
         Edit::MovePages { pages, to } => {
+            clear_shifted_page_comment_state(view, 0);
             let n = pages.iter().collect::<std::collections::BTreeSet<_>>().len();
             let to = (*to).min(info.pages.len() - n);
             view.select_pages(&(to..to + n).collect::<Vec<_>>());
         }
         Edit::DuplicatePages { pages, at: Some(at) } => {
+            clear_shifted_page_comment_state(view, *at);
             let n = pages.iter().collect::<std::collections::BTreeSet<_>>().len();
             let at = (*at).min(info.pages.len().saturating_sub(n));
             view.select_pages(&(at..at + n).collect::<Vec<_>>());
         }
-        Edit::InsertBlankPage { at, .. } => view.select_pages(&[(*at).min(info.pages.len() - 1)]),
-        Edit::DeletePages { .. } => view.select_pages(&[]),
+        Edit::InsertBlankPage { at, .. } => {
+            clear_shifted_page_comment_state(view, *at);
+            view.select_pages(&[(*at).min(info.pages.len() - 1)]);
+        }
+        Edit::DeletePages { pages } => {
+            if let Some(&first) = pages.iter().min() {
+                clear_shifted_page_comment_state(view, first);
+            }
+            view.select_pages(&[]);
+        }
         // A stroke drawn with the pen stays unselected, so the selection box and
         // author popup don't sit over the next stroke (#429).
         Edit::AddAnnotation(a)
@@ -76,7 +96,6 @@ fn select_after_edit(
                 view.comments.open_editor(a.page, n, String::new());
             }
         }
-        Edit::DeleteAnnotation { page, index } => view.comments.annotation_deleted(annots_before, &info.annotations, (*page, *index)),
         // A group moved together stays selected (its positions don't change), so it
         // can be moved or copied again; anything else in a batch clears the selection.
         Edit::Batch { edits, .. } if edits.iter().all(|e| matches!(e, Edit::MoveAnnotation { .. })) => {}
@@ -207,25 +226,15 @@ impl PdfCraftApp {
                     .map(|before| form_edit_pages(&before, &doc.form));
                 let one_page = comment_page(&edit).or_else(|| content_page(&edit));
                 if signatures_before.is_some_and(|before| crate::canvas::signature_states_changed(&before, &doc.signatures)) {
-                    // A signature shows its state on its own page, which an edit elsewhere can
-                    // change (a certification forbidding it, say): every page redraws. An edit that
-                    // moves no page (a form edit, or one to a page's comments or content) keeps the
-                    // values typed, in it and before it, showing until their pages redraw.
-                    let waiting = (form_edit.is_some() || one_page.is_some()).then(|| std::mem::take(&mut view.forms.unbaked));
-                    view.document_changed(info);
-                    if let Some(waiting) = waiting {
-                        view.forms.unbaked = waiting;
-                    }
-                    if let Some((_, unbaked)) = form_edit {
-                        view.forms.unbaked.extend(unbaked);
-                    }
+                    signatures_changed_after_edit(view, &edit, info, &annots_before, form_edit.map(|(_, u)| u), one_page);
                 } else if let Some(page) = one_page {
-                    one_page_changed(view, &edit, page);
+                    one_page_changed(view, &edit, page, &annots_before, info);
                 } else if let Some((changed_pages, unbaked)) = form_edit {
                     view.forms.unbaked.extend(unbaked);
                     for page in changed_pages {
                         view.page_changed(page);
                     }
+                    view.comments.document_changed(info);
                 } else {
                     view.document_changed(info);
                 }
@@ -1109,13 +1118,51 @@ fn content_page(edit: &Edit) -> Option<usize> {
     }
 }
 
+/// A signature shows its state on its own page, which an edit elsewhere can change (a
+/// certification forbidding it, say): every page redraws. An edit that moves no page (a form edit,
+/// or one to a page's comments or content) keeps the values typed, in it and before it, showing
+/// until their pages redraw.
+fn signatures_changed_after_edit(
+    view: &mut crate::DocView,
+    edit: &Edit,
+    info: &pdfcraft_render::DocInfo,
+    annots_before: &[pdfcraft_render::Annotation],
+    form_unbaked: Option<Vec<(String, usize)>>,
+    one_page: Option<usize>,
+) {
+    let waiting = (form_unbaked.is_some() || one_page.is_some()).then(|| std::mem::take(&mut view.forms.unbaked));
+    if let Edit::DeleteAnnotation { page, index } = edit {
+        // Remap surviving comments before generic slot pruning loses their state.
+        view.comments.annotation_deleted(annots_before, info, (*page, *index));
+    }
+    view.document_changed(info);
+    if let Some(waiting) = waiting {
+        view.forms.unbaked = waiting;
+    }
+    if let Some(unbaked) = form_unbaked {
+        view.forms.unbaked.extend(unbaked);
+    }
+}
+
 /// A comment or content edit changes one page, `page`: keep every other raster. Opening, closing
 /// or moving a pop-up changes none: the window is drawn over the page, and a /Popup annotation
 /// paints nothing in the raster (ISO 32000-2 §12.5.6.14).
-fn one_page_changed(view: &mut crate::DocView, edit: &Edit, page: usize) {
+fn one_page_changed(
+    view: &mut crate::DocView,
+    edit: &Edit,
+    page: usize,
+    annots_before: &[pdfcraft_render::Annotation],
+    info: &pdfcraft_render::DocInfo,
+) {
     if !matches!(edit, Edit::SetAnnotationPopup { .. }) {
         view.page_changed(page);
     }
+    if let Edit::DeleteAnnotation { page, index } = edit {
+        // Remap surviving comments before generic slot pruning loses their state.
+        view.comments.annotation_deleted(annots_before, info, (*page, *index));
+    } else {
+        view.comments.document_changed(info);
+    }
 }
 
 fn comment_page(edit: &Edit) -> Option<usize> {

crates/ui-egui/src/editing/tests.rs

@@ -375,6 +375,40 @@ fn save_commits_the_open_text_box() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+#[test]
+fn moving_pages_cancels_an_in_progress_drawing() {
+    let objs = [
+        "<< /Type /Catalog /Pages 2 0 R >>",
+        "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>",
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] >>",
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] >>",
+    ];
+    let mut pdf = b"%PDF-1.7\n".to_vec();
+    let mut offsets = Vec::new();
+    for (i, o) in objs.iter().enumerate() {
+        offsets.push(pdf.len());
+        pdf.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes());
+    }
+    let xref = pdf.len();
+    pdf.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes());
+    for o in offsets {
+        pdf.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes());
+    }
+    pdf.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
+
+    let mut app = crate::PdfCraftApp::new();
+    app.set_option("language", "en").unwrap();
+    app.open_bytes("a.pdf", None, pdf).unwrap();
+    let drawing =
+        || Some(crate::comments::Gesture::Draw { page: 0, tool: crate::comments::CommentTool::Ink, points: vec![[10.0, 10.0], [30.0, 20.0]] });
+    app.views[0].comments.gesture = drawing();
+    assert!(app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 }));
+    assert!(app.views[0].comments.gesture.is_some(), "rotating the page keeps the drawing on it");
+    app.views[0].comments.gesture = drawing();
+    assert!(app.apply_edit(pdfcraft_engine::Edit::MovePages { pages: vec![1], to: 0 }));
+    assert!(app.views[0].comments.gesture.is_none(), "the drawing's page index would now be a different page");
+}
+
 #[test]
 fn save_asks_where_to_keep_an_email_attachment_and_leaves_the_temporary_copy_alone() {
     let dir = staging_dir("mail-temp");

crates/ui-egui/src/tools/comments/composer.rs

@@ -116,7 +116,9 @@ pub(crate) fn composer(ctx: &egui::Context, view: &mut DocView, info: &DocInfo,
     ));
     let text = if typewriter_edit { c.text } else { c.text.trim_end().to_string() };
     match c.kind {
-        ComposerKind::Edit(index) => Some(Edit::SetAnnotationContents { page, index, text }),
+        ComposerKind::Edit(index) => {
+            info.annotations.iter().any(|a| a.page == page && a.index == index).then_some(Edit::SetAnnotationContents { page, index, text })
+        }
         ComposerKind::Note => {
             view.comments.tool_done = true;
             Some(new_comment(&cx, CommentTool::Note, Shape::Note { at: c.at, icon: NoteIcon::Comment }, text))

crates/ui-egui/src/tools/comments/input.rs

@@ -740,7 +740,7 @@ impl Gestures<'_> {
 }
 
 /// Delete / Escape handling for comments (only while no text field has focus).
-pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool, allowed: bool) {
+pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, tool: &mut QuickTool, allowed: bool) {
     if ctx.egui_wants_keyboard_input() {
         return;
     }
@@ -756,21 +756,19 @@ pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool
             *tool = QuickTool::Select;
         }
     }
-    if allowed
-        && cv.selected.is_some()
-        && ctx.input(|i| i.key_pressed(egui::Key::Delete) || i.key_pressed(egui::Key::Backspace))
-        && let Some(edit) = delete_selected(cv)
-    {
-        view.pending_edit = Some(edit);
+    if allowed && cv.selected.is_some() && ctx.input(|i| i.key_pressed(egui::Key::Delete) || i.key_pressed(egui::Key::Backspace)) {
+        view.pending_edit = delete_selected(cv, info);
     }
 }
 
 /// The edit that deletes every selected comment (one undo step for several), and clears the
 /// selection. Later positions go first, so each deletion leaves the earlier ones where they are.
-pub(crate) fn delete_selected(cv: &mut CommentView) -> Option<Edit> {
+pub(crate) fn delete_selected(cv: &mut CommentView, info: &DocInfo) -> Option<Edit> {
     // A group's member picked on the page, with its group the selection alone, goes by itself, as
     // Acrobat deletes the member clicked ([`CommentView::target`]).
-    let mut keys = if cv.also.is_empty() { cv.target().into_iter().collect() } else { cv.selection() };
+    let raw = if cv.also.is_empty() { cv.target().into_iter().collect() } else { cv.selection() };
+    let mut keys: Vec<(usize, usize)> =
+        raw.into_iter().filter(|(page, index)| info.annotations.iter().any(|a| a.page == *page && a.index == *index && !a.locked)).collect();
     cv.clear_selection();
     keys.sort_unstable_by(|a, b| b.cmp(a));
     keys.dedup();

crates/ui-egui/src/tools/comments/menu.rs

@@ -148,7 +148,7 @@ pub(crate) fn context_menu(
             let (dpage, dindex) = view.comments.target().unwrap_or((page, index));
             let deletes = deletable(dpage, dindex) && view.comments.also.iter().all(|&(p, i)| deletable(p, i));
             if ui.add_enabled(allowed && deletes, egui::Button::new(tl!(if several > 0 { "Delete comments" } else { "Delete" }))).clicked() {
-                action = delete_selected(&mut view.comments).map(|e| CanvasAction::Edit(Box::new(e)));
+                action = delete_selected(&mut view.comments, info).map(|e| CanvasAction::Edit(Box::new(e)));
                 ui.close();
             }
             if ui.add_enabled(allowed, egui::Button::new(tl!("Properties…"))).clicked() {

crates/ui-egui/src/tools/comments/state.rs

@@ -207,6 +207,17 @@ pub struct CommentView {
     pub tool_done: bool,
     /// What the inline editor (`editing`) does with the keyboard the next time it is drawn.
     pub(crate) editor_focus: Option<EditorFocus>,
+    /// Snapshot of `(page, index)` comment slot identity from the last `DocInfo`, used to detect
+    /// when an edit removes or shifts `/Annots` entries under an active selection or editor.
+    pub(crate) slots: std::collections::HashMap<(usize, usize), CommentSlot>,
+}
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub(crate) struct CommentSlot {
+    subtype: String,
+    name: Option<String>,
+    page_ord: usize,
+    page_count: usize,
 }
 
 /// A one-off change to the keyboard focus of the Comments panel's inline editor. The editor
@@ -221,6 +232,78 @@ pub(crate) enum EditorFocus {
 }
 
 impl CommentView {
+    /// Initial commenting state for `info`, recording its current annotation slots.
+    pub(crate) fn from_info(info: &DocInfo) -> Self {
+        Self { slots: Self::snapshot_slots(info), ..Default::default() }
+    }
+
+    /// Captures `(page, index)` comment slot identity for every comment in `info`.
+    fn snapshot_slots(info: &DocInfo) -> std::collections::HashMap<(usize, usize), CommentSlot> {
+        let mut by_page = std::collections::HashMap::<usize, Vec<usize>>::new();
+        for a in &info.annotations {
+            by_page.entry(a.page).or_default().push(a.index);
+        }
+        for indices in by_page.values_mut() {
+            indices.sort_unstable();
+            indices.dedup();
+        }
+        let mut slots = std::collections::HashMap::with_capacity(info.annotations.len());
+        for a in &info.annotations {
+            let indices = by_page.get(&a.page).map(Vec::as_slice).unwrap_or_default();
+            let page_ord = indices.binary_search(&a.index).unwrap_or(0);
+            let page_count = indices.len();
+            slots.insert((a.page, a.index), CommentSlot { subtype: a.subtype.clone(), name: a.name.clone(), page_ord, page_count });
+        }
+        slots
+    }
+
+    /// Prunes comment state whose `(page, index)` slot was removed or shifted by a document change.
+    pub(crate) fn document_changed(&mut self, info: &DocInfo) {
+        let next = Self::snapshot_slots(info);
+        let page_count = info.pages.len();
+        let same_slot = |page: usize, index: usize| -> bool {
+            let Some(cur) = next.get(&(page, index)) else {
+                return false;
+            };
+            if self.slots.is_empty() {
+                return true;
+            }
+            self.slots.get(&(page, index)).is_some_and(|old| {
+                cur.subtype == old.subtype && cur.name == old.name && cur.page_ord == old.page_ord && cur.page_count >= old.page_count
+            })
+        };
+        if self.selected.is_some_and(|(p, i)| !same_slot(p, i)) {
+            self.selected = None;
+        }
+        self.also.retain(|&(p, i)| same_slot(p, i));
+        if self.selected.is_none() {
+            self.selected = self.also.pop();
+        }
+        if self.member.is_some_and(|((mp, mi), (gp, gi))| Some((gp, gi)) != self.selected || !same_slot(mp, mi)) {
+            self.member = None;
+        }
+        if self.editing.as_ref().is_some_and(|(p, i, _)| !same_slot(*p, *i)) {
+            self.editing = None;
+            self.editor_focus = None;
+        }
+        if self.composer.as_ref().is_some_and(|c| match c.kind {
+            ComposerKind::Edit(i) => !same_slot(c.page, i),
+            _ => c.page >= page_count,
+        }) {
+            self.composer = None;
+        }
+        if self.gesture.as_ref().is_some_and(|g| match g {
+            Gesture::Draw { page, .. } => *page >= page_count,
+            Gesture::Move { page, index, .. } | Gesture::Resize { page, index, .. } | Gesture::Reshape { page, index, .. } => {
+                !same_slot(*page, *index)
+            }
+            Gesture::Place { .. } => false,
+        }) {
+            self.gesture = None;
+        }
+        self.slots = next;
+    }
+
     /// Every selected comment, the one the handles belong to first.
     pub fn selection(&self) -> Vec<(usize, usize)> {
         let mut all: Vec<(usize, usize)> = self.selected.into_iter().collect();
@@ -333,24 +416,33 @@ impl CommentView {
         }
     }
 
-    /// The annotation at `gone` was just deleted; `before` and `after` are the document's
+    /// The annotation at `gone` was just deleted; `before` and `info` hold the document's
     /// annotations until then and now. The selection and the inline editor stay on what they
     /// were on, where it now is (deleting a reply keeps its comment selected), and close when it
     /// went too.
-    pub(crate) fn annotation_deleted(&mut self, before: &[Annotation], after: &[Annotation], gone: (usize, usize)) {
-        self.selected = self.selected.and_then(|k| after_delete(before, after, gone, k));
-        self.also = std::mem::take(&mut self.also).into_iter().filter_map(|k| after_delete(before, after, gone, k)).collect();
-        self.member = None;
-        self.editing = self.editing.take().and_then(|(p, i, text)| after_delete(before, after, gone, (p, i)).map(|(p, i)| (p, i, text)));
-        if self.editing.is_none() {
-            self.editor_focus = None;
-        }
-        // What is typed in a pop-up, and a window being dragged or resized, follow theirs too.
+    pub(crate) fn annotation_deleted(&mut self, before: &[Annotation], info: &DocInfo, gone: (usize, usize)) {
+        let after = &info.annotations;
         let moved = |k: (usize, usize)| after_delete(before, after, gone, k);
-        self.popup_text = self.popup_text.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text)));
-        self.popup_reply = self.popup_reply.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text)));
-        self.popup_drag = self.popup_drag.and_then(|(p, i, at)| moved((p, i)).map(|(p, i)| (p, i, at)));
-        self.popup_resize = self.popup_resize.and_then(|(p, i, size)| moved((p, i)).map(|(p, i)| (p, i, size)));
+        let selected = self.selected.and_then(moved);
+        let also: Vec<(usize, usize)> = std::mem::take(&mut self.also).into_iter().filter_map(moved).collect();
+        let editing = self.editing.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text)));
+        let focus = self.editor_focus.take();
+        // What is typed in a pop-up, and a window being dragged or resized, follow theirs too.
+        let popup_text = self.popup_text.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text)));
+        let popup_reply = self.popup_reply.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text)));
+        let popup_drag = self.popup_drag.and_then(|(p, i, at)| moved((p, i)).map(|(p, i)| (p, i, at)));
+        let popup_resize = self.popup_resize.and_then(|(p, i, size)| moved((p, i)).map(|(p, i)| (p, i, size)));
+        // Other transient state still needs conservative pruning; refresh the slot snapshot too.
+        self.document_changed(info);
+        self.selected = selected;
+        self.also = also;
+        self.member = None;
+        self.editor_focus = editing.as_ref().and(focus);
+        self.editing = editing;
+        self.popup_text = popup_text;
+        self.popup_reply = popup_reply;
+        self.popup_drag = popup_drag;
+        self.popup_resize = popup_resize;
     }
 
     /// Undo or Redo just ran. Either may have put annotations back or taken them away, on any

crates/ui-egui/src/tools/comments/tests.rs

@@ -165,11 +165,17 @@ fn a_kept_comment_is_found_where_a_delete_moved_it() {
     let changed = vec![listed(0, 0, "something else", None), listed(0, 1, "B", Some((0, 0)))];
     assert_eq!(after_delete(&before, &changed, gone, (0, 2)), None);
 
-    let mut view = CommentView { selected: Some((0, 2)), editing: Some((0, 3, "typed".into())), ..Default::default() };
-    view.annotation_deleted(&before, &after, gone);
+    let info = DocInfo { annotations: before.clone(), ..Default::default() };
+    let mut view = CommentView::from_info(&info);
+    view.selected = Some((0, 2));
+    view.editing = Some((0, 3, "typed".into()));
+    view.annotation_deleted(&before, &DocInfo { annotations: after.clone(), ..Default::default() }, gone);
+    view.document_changed(&DocInfo { annotations: after.clone(), ..Default::default() });
     assert_eq!((view.selected, view.editing), (Some((0, 0)), Some((0, 1, "typed".into()))), "both follow");
-    let mut view = CommentView { editing: Some((0, 4, "typed".into())), editor_focus: Some(EditorFocus::Take), ..Default::default() };
-    view.annotation_deleted(&before, &after, gone);
+    let mut view = CommentView::from_info(&info);
+    view.editing = Some((0, 4, "typed".into()));
+    view.editor_focus = Some(EditorFocus::Take);
+    view.annotation_deleted(&before, &DocInfo { annotations: after.clone(), ..Default::default() }, gone);
     assert_eq!((view.editing, view.editor_focus), (None, None), "the editor closes with the reply it was open on");
 }
 
@@ -554,6 +560,137 @@ fn text_boxes_fit_their_text() {
     assert!(long[3] - long[1] > 50.0);
 }
 
+#[test]
+fn stale_comment_state_is_cleared_and_delete_never_hits_non_comment_slots() {
+    let annot = |index: usize, name: &str| Annotation {
+        page: 0,
+        index,
+        subtype: "Text".into(),
+        rect: [10.0, 10.0, 30.0, 30.0],
+        color: None,
+        author: None,
+        contents: Some(name.into()),
+        modified: None,
+        in_reply_to: None,
+        parent: None,
+        grouped: false,
+        name: Some(name.into()),
+        state: None,
+        quads: Vec::new(),
+        locked: false,
+        intent: None,
+        fill_sign: false,
+        no_rotate: false,
+        points: Vec::new(),
+        text_box: None,
+        popup: None,
+    };
+    // Page 0 has a Widget at index 0 (not in `info.annotations`), and Text comments at 1 and 2.
+    let info = DocInfo {
+        pages: vec![PageInfo { width: 300.0, height: 400.0, label: String::new(), crop: [0.0, 0.0, 300.0, 400.0], rotation: 0 }],
+        annotations: vec![annot(1, "c1"), annot(2, "c2")],
+        ..Default::default()
+    };
+    let mut view = DocView::new(pdfcraft_engine::DocId(1), &info, crate::canvas::ViewDefaults::default());
+    view.comments.selected = Some((0, 2));
+    view.comments.open_editor(0, 2, "draft".into());
+    view.comments.composer = Some(Composer { page: 0, at: [10.0, 10.0], kind: ComposerKind::Edit(2), text: "body".into(), focus: false });
+    view.comments.gesture = Some(Gesture::Move { page: 0, index: 2, from: pos2(0.0, 0.0) });
+
+    // Moving/editing in place or appending a comment at index 3 preserves (0, 2) selection/editing/gesture.
+    let mut appended = info.clone();
+    appended.annotations.push(annot(3, "c3"));
+    view.document_changed(&appended);
+    assert_eq!(view.comments.selected, Some((0, 2)));
+    assert!(view.comments.editing.is_some());
+    assert!(view.comments.composer.is_some());
+    assert!(view.comments.gesture.is_some());
+
+    // Deleting comment 1 shifts comment 2 down to index 1: stale state at (0, 2) and shifted (0, 1) must clear.
+    let mut shifted = info.clone();
+    shifted.annotations = vec![annot(1, "c2")];
+    view.comments.selected = Some((0, 1));
+    view.comments.open_editor(0, 1, "for c1, not c2".into());
+    view.comments.composer = Some(Composer { page: 0, at: [10.0, 10.0], kind: ComposerKind::Edit(1), text: "for c1".into(), focus: false });
+    view.comments.gesture = Some(Gesture::Move { page: 0, index: 1, from: pos2(0.0, 0.0) });
+    view.document_changed(&shifted);
+    assert_eq!(view.comments.selected, None);
+    assert_eq!(view.comments.editing, None);
+    assert_eq!(view.comments.composer, None);
+    assert_eq!(view.comments.gesture, None);
+
+    // Pressing Delete when `selected` holds a widget index (0, 0) or stale index (0, 2) emits no edit.
+    let ctx = egui::Context::default();
+    for bad_index in [0, 2] {
+        view.comments.selected = Some((0, bad_index));
+        let mut input = egui::RawInput::default();
+        input.events.push(egui::Event::Key {
+            key: egui::Key::Delete,
+            physical_key: None,
+            pressed: true,
+            repeat: false,
+            modifiers: egui::Modifiers::NONE,
+        });
+        let mut out = ctx.run_ui(input, |ui| {
+            let mut tool = QuickTool::Select;
+            keys(ui.ctx(), &mut view, &shifted, &mut tool, true);
+        });
+        out.textures_delta.clear();
+        assert_eq!(view.comments.selected, None);
+        assert_eq!(view.pending_edit, None, "index {bad_index} must not emit DeleteAnnotation");
+    }
+}
+
+/// An unnamed comment has no object name to tell it from the one that slid into its index.
+/// Deleting the earlier one still drops selection, editing, the composer and a gesture that
+/// pointed at that index.
+#[test]
+fn an_unnamed_shift_clears_state_for_the_old_index() {
+    let annot = |index: usize| Annotation {
+        page: 0,
+        index,
+        subtype: "Text".into(),
+        rect: [10.0, 10.0, 30.0, 30.0],
+        color: None,
+        author: None,
+        contents: None,
+        modified: None,
+        in_reply_to: None,
+        parent: None,
+        grouped: false,
+        name: None,
+        state: None,
+        quads: Vec::new(),
+        locked: false,
+        intent: None,
+        fill_sign: false,
+        no_rotate: false,
+        points: Vec::new(),
+        text_box: None,
+        popup: None,
+    };
+    let info = DocInfo {
+        pages: vec![
+            PageInfo { width: 300.0, height: 400.0, label: String::new(), crop: [0.0, 0.0, 300.0, 400.0], rotation: 0 },
+            PageInfo { width: 300.0, height: 400.0, label: String::new(), crop: [0.0, 0.0, 300.0, 400.0], rotation: 0 },
+        ],
+        annotations: vec![annot(1), annot(2)],
+        ..Default::default()
+    };
+    let mut view = DocView::new(pdfcraft_engine::DocId(1), &info, crate::canvas::ViewDefaults::default());
+    view.comments.selected = Some((0, 1));
+    view.comments.open_editor(0, 1, "the first note".into());
+    view.comments.composer = Some(Composer { page: 0, at: [10.0, 10.0], kind: ComposerKind::Edit(1), text: "the first note".into(), focus: false });
+    view.comments.gesture = Some(Gesture::Move { page: 0, index: 1, from: pos2(0.0, 0.0) });
+    let mut shifted = info.clone();
+    shifted.annotations = vec![annot(1)];
+    view.document_changed(&shifted);
+    assert_eq!(view.comments.selected, None);
+    assert_eq!(view.comments.editing, None);
+    assert_eq!(view.comments.composer, None);
+    assert_eq!(view.comments.gesture, None);
+}
+
 /// Review on #4358: a Replace Text group is selected by its caret, the group's primary (ISO
 /// 32000-2 §12.5.6.2), and Make Current Properties Default on it updates the Replace Text tool. A
 /// caret of its own (Insert Text) still updates the Insert Text tool.