M5: a pop-up reads its thread by /IRT reference, as the Comments panel does (#2137)

Rebased onto main's reference-based threading (#4061, #4566). The pop-up
matched replies by /NM (in_reply_to == name), which need not be unique:
two notes sharing PyMuPDF's fitz-A0 both showed one note's replies, and a
reply to a reply was missing. It now files replies under a comment with
thread_tops over the settled parent links, as comments_panel does, and
has_popup asks Annotation::is_comment rather than in_reply_to (a reply
whose parent has no /NM read as a comment of its own).

Regression test a_pop_up_lists_its_replies_by_reference_when_names_repeat,
seen to fail with the /NM filter restored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC
This commit is contained in:
mtlcl410andClaude Opus 5.5 committed 2026-11-01 09:16:03 +08:00
1 parent 34871b61a5
commit 45f5a3c591
2 files changed
+45 -6

No files matched your search

+12 -6
View File
@@ -4,9 +4,10 @@ use super::{
theme,
};
/// A comment with a pop-up window of its own (not a text box, a Fill & Sign mark or a reply).
/// A comment with a pop-up window of its own: not a text box, a Fill & Sign mark, a reply or a
/// status (replies are found by `/IRT` reference, [`Annotation::is_comment`]).
pub(super) fn has_popup(a: &Annotation) -> bool {
a.subtype != "FreeText" && !a.fill_sign && a.in_reply_to.is_none()
a.subtype != "FreeText" && !a.fill_sign && a.is_comment()
}
/// The line from a comment to its open pop-up, in the comment's colour, while the pointer is on
@@ -62,6 +63,8 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hi
let mut edits: Vec<Edit> = Vec::new();
let mut rects = Vec::new();
let mut hovered = None;
// The comment each reply files under, as the Comments panel threads them (by reference).
let tops = pdfcraft_render::thread_tops(&info.annotations);
let xforms: Vec<(usize, PageXform)> = view.page_xforms();
for (page, xf) in xforms {
let shown: Vec<Annotation> =
@@ -167,13 +170,16 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hi
} else {
ui.label(a.contents.as_deref().unwrap_or_default());
}
// Replies, as the thread reads: text replies only, as in the Comments
// panel. A review status or a checkmark (`/State`) is a reply too, but
// not one to read here.
// Replies, as the thread reads: text replies only, at any depth, filed
// under this comment by `/IRT` reference as in the Comments panel. A
// review status or a checkmark (`/State`) is a reply too, but not one to
// read here.
let replies: Vec<&Annotation> = info
.annotations
.iter()
.filter(|r| r.page == page && r.in_reply_to.is_some() && r.in_reply_to == a.name && r.state.is_none())
.filter(|r| {
r.state.is_none() && r.parent.is_some_and(|parent| tops.get(&(r.page, r.index)).copied().unwrap_or(parent) == key)
})
.collect();
for r in replies {
ui.add_space(4.0);
+33
View File
@@ -3155,3 +3155,36 @@ fn a_double_click_opens_a_notes_pop_up_which_drags_and_closes() {
assert!(h.state().views[0].comments.popup_rects.is_empty());
assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Close comment pop-up"));
}
// Contributor-original PDF: two notes sharing one /NM (as PyMuPDF names them), both with an open
// pop-up; the second has a reply by reference and a reply to that reply, which has no /NM.
const POPUPS_SHARING_A_NAME: &[u8] = b"%PDF-1.7
1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj
3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Annots [4 0 R 5 0 R 6 0 R 7 0 R 8 0 R 9 0 R] >> endobj
4 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /NM (fitz-A0) /T (Alpha) /Contents (First note) /Popup 5 0 R /Open true >> endobj
5 0 obj << /Type /Annot /Subtype /Popup /Rect [60 110 160 190] /Parent 4 0 R /Open true >> endobj
6 0 obj << /Type /Annot /Subtype /Text /Rect [20 40 40 60] /NM (fitz-A0) /T (Beta) /Contents (Second note) /Popup 7 0 R /Open true >> endobj
7 0 obj << /Type /Annot /Subtype /Popup /Rect [180 10 290 90] /Parent 6 0 R /Open true >> endobj
8 0 obj << /Type /Annot /Subtype /Text /Rect [20 40 40 60] /NM (fitz-A1) /IRT 6 0 R /T (Gamma) /Contents (Reply by reference) >> endobj
9 0 obj << /Type /Annot /Subtype /Text /Rect [20 40 40 60] /IRT 8 0 R /T (Delta) /Contents (Reply to the reply) >> endobj
trailer << /Root 1 0 R >>
%%EOF";
/// A pop-up reads its comment's thread by `/IRT` reference, as the Comments panel does: the
/// replies show once, in the second note's window only, at any depth, though both notes share
/// one `/NM`.
#[test]
fn a_pop_up_lists_its_replies_by_reference_when_names_repeat() {
let h = harness_with_bytes(POPUPS_SHARING_A_NAME.to_vec(), |_| {});
let windows = h.state().views[0].comments.popup_rects.clone();
assert_eq!(windows.len(), 2, "both notes' pop-ups are open: {windows:?}");
let (_, second) = windows.iter().find(|(key, _)| *key == (0, 2)).copied().expect("the second note's pop-up");
for text in ["Reply by reference", "Reply to the reply"] {
// The Comments panel lists the thread too; count only what the windows show.
let shown: Vec<Pos2> =
h.query_all_by_label(text).map(|n| n.rect().center()).filter(|c| windows.iter().any(|(_, w)| w.contains(*c))).collect();
assert_eq!(shown.len(), 1, "{text:?} shows in one window: {shown:?}");
assert!(shown.iter().all(|c| second.contains(*c)), "{text:?} is in the second note's window");
}
}