From 72edd945e56724ece07d72625964a592f83344bb Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 28 Oct 2026 12:14:42 +0100 Subject: [PATCH] fix(comments): a locked reply keeps the comment or reply it answers Deleting a comment or a reply took every reply to it, locked or not: only the target's own Locked flag was checked. A locked reply can't be deleted on its own, so delete_annotation now refuses while anything that would go with the target is locked (pop-ups follow their comment). Co-Authored-By: Claude Opus 5.5 --- crates/annot/src/modify.rs | 10 ++++++++++ crates/annot/src/tests.rs | 19 +++++++++++++++++++ parity/acrobat-features.toml | 4 ++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index fc81348075..92807babd0 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -80,6 +80,16 @@ pub fn delete_annotation(doc: &mut Document, page: usize, index: usize) -> Resul // An inline comment has no reference to collect; the pop-up it names still goes. let seed = target.as_ref().or_else(|| target.as_dict().and_then(|d| d.reference(b"Popup"))); let doomed = with_replies_and_popups(doc, &list, seed); + // A locked reply can't be deleted on its own, so it doesn't go with the comment or reply it + // answers either. Pop-ups follow their comment. + for (_, e) in list.iter().enumerate().filter(|(i, e)| *i != index && goes_with(e, &doomed)) { + let obj = doc.resolve(e); + if let Some(d) = obj.as_dict().filter(|d| d.name(b"Subtype") != Some(b"Popup".as_slice())) + && annotation_flags(doc, d) & FLAG_LOCKED != 0 + { + return Err(AnnotError::Invalid("a reply to it is locked".into())); + } + } let kept: Vec = list.into_iter().enumerate().filter(|(i, e)| *i != index && !goes_with(e, &doomed)).map(|(_, e)| e).collect(); Ok(set_annots(doc, p, kept)?) } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index eb8d1a0f15..d1e2f97e89 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -229,6 +229,25 @@ fn delete_takes_popup_and_reply_chain() { assert_eq!(delete_annotation(&mut doc, 0, 9), Err(AnnotError::NoSuchAnnotation { page: 0, index: 9 })); } +/// A locked reply can't be deleted, so it doesn't go with what it answers either: deleting the +/// reply above it, or the comment, is refused and changes nothing; once it is unlocked they go. +#[test] +fn a_locked_reply_keeps_what_it_answers() { + let mut doc = fixture(); + let note = add_annotation(&mut doc, &new(0, Shape::Note { at: [100.0, 100.0], icon: NoteIcon::Comment }), &meta("n")).unwrap(); + let reply = add_reply(&mut doc, 0, note, "first", "Bob", &meta("r1")).unwrap(); + let answer = add_reply(&mut doc, 0, reply, "locked answer", "Ada", &meta("r2")).unwrap(); + set_locked(&mut doc, 0, answer, true).unwrap(); + let before = list(&doc, 0).len(); + assert_eq!(delete_annotation(&mut doc, 0, reply), Err(AnnotError::Invalid("a reply to it is locked".into()))); + assert_eq!(delete_annotation(&mut doc, 0, note), Err(AnnotError::Invalid("a reply to it is locked".into()))); + assert_eq!(list(&doc, 0).len(), before, "nothing went"); + set_locked(&mut doc, 0, answer, false).unwrap(); + delete_annotation(&mut doc, 0, reply).unwrap(); + let left: Vec = list(&doc, 0).iter().map(|d| text(d, b"Contents")).collect(); + assert!(!left.iter().any(|t| t == "first" || t == "locked answer"), "the reply and its answer went: {left:?}"); +} + /// A reply or pop-up written inline in `/Annots` (a file can, though the standard asks for /// references) goes with the comment it points at, and so does a pop-up an inline reply or an /// inline comment names. An unrelated inline comment stays until it is deleted itself. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 1d15a25afe..760fa0a338 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4003,8 +4003,8 @@ milestone = "M5" status = "shipped" commands = ["comment.list"] tools = ["comment_delete"] -evidence = ["crates/ui-egui/tests/it/comments.rs::comments_are_selected_moved_and_deleted_with_the_select_tool", "crates/annot/src/tests.rs::delete_takes_popup_and_reply_chain", "crates/ui-egui/tests/it/comments.rs::one_reply_is_deleted_from_its_menu_and_the_thread_stays", "crates/ui-egui/src/tools/comments/tests.rs::a_kept_comment_is_found_where_a_delete_moved_it", "crates/automation/tests/it/automation.rs::deleting_a_reply_keeps_its_comment_and_the_other_replies"] -notes = "deletes the pop-up and the reply thread. One reply is deleted from its own \"…\" in the Comments panel (comment_delete with the reply's id, or page and index), with its pop-up and the replies to it: the comment and its other replies stay, one undo step, and the card stays selected. A locked reply can't be deleted." +evidence = ["crates/ui-egui/tests/it/comments.rs::comments_are_selected_moved_and_deleted_with_the_select_tool", "crates/annot/src/tests.rs::delete_takes_popup_and_reply_chain", "crates/ui-egui/tests/it/comments.rs::one_reply_is_deleted_from_its_menu_and_the_thread_stays", "crates/ui-egui/src/tools/comments/tests.rs::a_kept_comment_is_found_where_a_delete_moved_it", "crates/automation/tests/it/automation.rs::deleting_a_reply_keeps_its_comment_and_the_other_replies", "crates/annot/src/tests.rs::a_locked_reply_keeps_what_it_answers"] +notes = "deletes the pop-up and the reply thread. One reply is deleted from its own \"…\" in the Comments panel (comment_delete with the reply's id, or page and index), with its pop-up and the replies to it: the comment and its other replies stay, one undo step, and the card stays selected. A locked reply can't be deleted, nor the comment or reply it answers while it is locked." [[feature]] id = "comment.keyboard-nudge"