diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index 494b6350d3..e71139a775 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -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 = 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 = @@ -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); diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index dc0f29fcbc..6e96ffe260 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -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 = + 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"); + } +}