mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-13 22:32:19 +00:00
fix(comments): thread comment replies by /IRT reference, count comments like Acrobat
Replies were matched to their parent by /NM across the whole document. /NM need not be unique: PyMuPDF names the first comment on every page `fitz-A0`, so every reply in such a file was filed under page 1's comment, and the other pages showed none. Both comment readers (render inspect on open, annot summaries after an edit) now resolve /IRT to the parent's page and /Annots position by object reference, and the Comments panel, comment properties, the canvas thread, the engine's comment summary and the comment_list tool thread by that. The Comments header now counts comments without their replies, as Acrobat does (28 rather than 44 for the reported file), and a reply's thread line runs from the avatar above it (the comment's or the previous reply's) to its own instead of a 12 px stub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KvugvPrjZtPVudhh7cMa54
This commit is contained in:
1 parent
57e531ccca
commit
be600950c9
13 files changed
+267
-164
No files matched your search
@@ -199,6 +199,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
|
||||
- **2026-10-20 (M3, #3535):** Escape with a menu open closes the menu and keeps the text selection; the next Escape clears it, as before. One keypress used to do both. A synthetic UI regression checks it. Estimates unchanged.
|
||||
- **2026-10-16 (M3, close prompt keyboard):** The "Save changes?" prompt opens with Save chosen (Enter saves, as before) and the arrow keys and Tab / Shift+Tab move the choice to Cancel or Don't save, shown with a focus ring; Enter or Space press the chosen button and Escape still cancels. Before, Enter always saved whatever was Tab-focused. Estimates unchanged.
|
||||
- **2026-10-21 (M3, #3808):** Page thumbnails in the Inspector can be resized independently from the document and Organize, with a slider, buttons and `thumbnail-zoom` control option. Selection, page following and reload are covered by regressions. Estimates unchanged.
|
||||
- **2026-10-22 (M5, comment threads):** Replies join the comment their `/IRT` references, not every comment sharing its `/NM`; PyMuPDF names each page's first comment `fitz-A0`, so all replies used to land under page 1. The Comments header counts comments without their replies (28, as Acrobat says, for a file it used to call 44), and a reply's thread line runs from the avatar above it to its own. UI, engine and automation regressions cover it. Estimates unchanged.
|
||||
- **2026-10-22 (M2, #3107 / PR #3438):** Inline-image parsing falls back to the existing EI heuristic when missing declared samples, headers or terminators exceed the content-window budget, instead of waiting for an impossible allocation. Exact-length parsing for available samples and ordinary large streamed images is retained. Estimates unchanged.
|
||||
- **2026-10-21 (M14, Brazilian Portuguese):** Completed the `pt-br` catalog against the current 2,674-key union, including signature validation and accessibility messages; no build or compilation run. Localization remains partial because help and unpredictable engine/OS error details are not translated.
|
||||
- **2026-10-21 (M14, #3557):** Preferences: the OK button stays below the scrolling settings instead of being the last thing in them, so it is in reach in a short window.
|
||||
|
||||
+16
-2
@@ -2254,6 +2254,8 @@ pub struct Summary {
|
||||
pub modified: Option<String>,
|
||||
pub name: Option<String>,
|
||||
pub in_reply_to: Option<String>,
|
||||
/// The page and `/Annots` position `/IRT` points at, by reference (`/NM` need not be unique).
|
||||
pub parent: Option<(usize, usize)>,
|
||||
pub rect: [f32; 4],
|
||||
pub color: Option<[f32; 3]>,
|
||||
pub state: Option<String>,
|
||||
@@ -2289,8 +2291,18 @@ pub fn is_comment_subtype(subtype: &str) -> bool {
|
||||
pub fn summaries(doc: &Document) -> Vec<Summary> {
|
||||
let mut out = Vec::new();
|
||||
let Ok(pages) = page_refs(doc) else { return out };
|
||||
for (page, p) in pages.iter().enumerate() {
|
||||
for (index, entry) in annots(doc, *p).iter().enumerate() {
|
||||
let lists: Vec<Vec<Object>> = pages.iter().map(|p| annots(doc, *p)).collect();
|
||||
// Where each annotation is first listed, so a reply's `/IRT` finds its parent by reference.
|
||||
let mut slots: std::collections::HashMap<ObjRef, (usize, usize)> = std::collections::HashMap::new();
|
||||
for (page, list) in lists.iter().enumerate() {
|
||||
for (index, entry) in list.iter().enumerate() {
|
||||
if let Some(r) = entry.as_ref() {
|
||||
slots.entry(r).or_insert((page, index));
|
||||
}
|
||||
}
|
||||
}
|
||||
for (page, list) in lists.iter().enumerate() {
|
||||
for (index, entry) in list.iter().enumerate() {
|
||||
let obj = doc.resolve(entry);
|
||||
let Some(d) = obj.as_dict() else { continue };
|
||||
let Some(subtype) = d.name(b"Subtype").map(|s| String::from_utf8_lossy(s).into_owned()) else { continue };
|
||||
@@ -2326,6 +2338,7 @@ pub fn summaries(doc: &Document) -> Vec<Summary> {
|
||||
_ => Vec::new(),
|
||||
};
|
||||
let in_reply_to = d.get(b"IRT").map(|o| doc.resolve(o)).and_then(|o| o.as_dict().cloned()).and_then(|p| text_value(doc, &p, b"NM"));
|
||||
let parent = d.get(b"IRT").and_then(Object::as_ref).and_then(|r| slots.get(&r).copied());
|
||||
out.push(Summary {
|
||||
page,
|
||||
index,
|
||||
@@ -2337,6 +2350,7 @@ pub fn summaries(doc: &Document) -> Vec<Summary> {
|
||||
modified: text_value(doc, d, b"M"),
|
||||
name: text_value(doc, d, b"NM"),
|
||||
in_reply_to,
|
||||
parent,
|
||||
rect,
|
||||
color,
|
||||
state: text_value(doc, d, b"State"),
|
||||
|
||||
@@ -200,26 +200,24 @@ impl Automation {
|
||||
};
|
||||
let roots: Vec<Value> = all
|
||||
.iter()
|
||||
.filter(|c| c.in_reply_to.is_none() && only.is_none_or(|p| c.page == p))
|
||||
.filter(|c| !c.is_reply() && only.is_none_or(|p| c.page == p))
|
||||
.map(|c| {
|
||||
let mut v = view(c);
|
||||
// Every reply under this comment, at any depth, in document order. A reply to a
|
||||
// reply keeps its `/IRT` on the direct parent, so following one level would drop it.
|
||||
let descends_from = |r: &Annotation, root: &str| -> bool {
|
||||
let mut parent = r.in_reply_to.as_deref();
|
||||
// Parents are found by reference: `/NM` need not be unique.
|
||||
let descends_from = |r: &Annotation, root: &Annotation| -> bool {
|
||||
let mut parent = r.parent;
|
||||
for _ in 0..64 {
|
||||
let Some(nm) = parent else { return false };
|
||||
if nm == root {
|
||||
let Some(at) = parent else { return false };
|
||||
if at == (root.page, root.index) {
|
||||
return true;
|
||||
}
|
||||
parent = all.iter().find(|x| x.name.as_deref() == Some(nm)).and_then(|x| x.in_reply_to.as_deref());
|
||||
parent = all.iter().find(|x| (x.page, x.index) == at).and_then(|x| x.parent);
|
||||
}
|
||||
false
|
||||
};
|
||||
let thread: Vec<&Annotation> = match c.name.as_deref() {
|
||||
Some(nm) => all.iter().filter(|r| r.in_reply_to.is_some() && descends_from(r, nm)).collect(),
|
||||
None => Vec::new(),
|
||||
};
|
||||
let thread: Vec<&Annotation> = all.iter().filter(|r| r.is_reply() && descends_from(r, c)).collect();
|
||||
let status = thread.iter().rev().filter(|r| !r.is_mark()).find_map(|r| r.state.clone());
|
||||
v["status"] = json!(status);
|
||||
v["marked"] = json!(thread.iter().rev().find(|r| r.is_mark()).is_some_and(|r| r.state.as_deref() == Some("Marked")));
|
||||
|
||||
@@ -2900,6 +2900,7 @@ fn comment_list(doc: &pdfcraft_cos::Document) -> Vec<pdfcraft_render::Annotation
|
||||
modified: s.modified.map(|m| pdfcraft_render::pretty_date(&m)),
|
||||
name: s.name,
|
||||
in_reply_to: s.in_reply_to,
|
||||
parent: s.parent,
|
||||
rect: s.rect,
|
||||
color: s.color,
|
||||
index: s.index,
|
||||
@@ -5327,24 +5328,22 @@ pub fn comment_kind(a: &pdfcraft_render::Annotation) -> &str {
|
||||
/// pop-ups; the number is the comment's position on its page.
|
||||
pub fn comment_summary(name: &str, all: &[pdfcraft_render::Annotation], sort: SummarySort) -> String {
|
||||
use std::fmt::Write;
|
||||
let top: Vec<&pdfcraft_render::Annotation> = all.iter().filter(|a| a.in_reply_to.is_none() && a.state.is_none()).collect();
|
||||
let top: Vec<&pdfcraft_render::Annotation> = all.iter().filter(|a| !a.is_reply() && a.state.is_none()).collect();
|
||||
// Write a comment's replies, indenting each level. A reply to a reply keeps its `/IRT` on the
|
||||
// direct parent, so following one level would leave the deeper replies out of the summary.
|
||||
fn write_replies(out: &mut String, all: &[pdfcraft_render::Annotation], parent: &str, depth: usize) {
|
||||
fn write_replies(out: &mut String, all: &[pdfcraft_render::Annotation], parent: &pdfcraft_render::Annotation, depth: usize) {
|
||||
use std::fmt::Write;
|
||||
if depth > 64 {
|
||||
return;
|
||||
}
|
||||
for r in all.iter().filter(|r| r.state.is_none() && r.in_reply_to.as_deref() == Some(parent)) {
|
||||
for r in all.iter().filter(|r| r.state.is_none() && r.replies_to(parent)) {
|
||||
let indent = " ".repeat(depth + 1);
|
||||
let _ =
|
||||
writeln!(out, "{indent}Author: {} Subject: Reply Date: {}", r.author.as_deref().unwrap_or(""), r.modified.as_deref().unwrap_or(""));
|
||||
for line in r.contents.as_deref().unwrap_or("").lines() {
|
||||
let _ = writeln!(out, "{indent}{line}");
|
||||
}
|
||||
if let Some(nm) = r.name.as_deref() {
|
||||
write_replies(out, all, nm, depth + 1);
|
||||
}
|
||||
write_replies(out, all, r, depth + 1);
|
||||
}
|
||||
}
|
||||
let mut numbered: Vec<(usize, &pdfcraft_render::Annotation)> = Vec::new();
|
||||
@@ -5383,9 +5382,7 @@ pub fn comment_summary(name: &str, all: &[pdfcraft_render::Annotation], sort: Su
|
||||
if let Some(c) = a.contents.as_deref().filter(|c| !c.is_empty()) {
|
||||
let _ = writeln!(out, "{c}");
|
||||
}
|
||||
if let Some(nm) = a.name.as_deref() {
|
||||
write_replies(&mut out, all, nm, 0);
|
||||
}
|
||||
write_replies(&mut out, all, a, 0);
|
||||
out.push('\n');
|
||||
}
|
||||
out
|
||||
|
||||
@@ -3419,6 +3419,37 @@ fn comments_lock_take_checkmarks_hide_and_summarize() {
|
||||
assert_eq!(comment_summary("x", &[], SummarySort::Page), "Summary of Comments on x\n\nThis document has no comments.\n");
|
||||
}
|
||||
|
||||
/// Replies thread by their `/IRT` reference, not `/NM`: PyMuPDF names the first comment on every
|
||||
/// page `fitz-A0`, which used to file every reply under page 1's comment. Both readers (inspect on
|
||||
/// open, and the comment list the engine refreshes after an edit) must agree.
|
||||
#[test]
|
||||
fn replies_thread_by_reference_when_names_repeat_across_pages() {
|
||||
let pdf = b"%PDF-1.7
|
||||
1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
|
||||
2 0 obj << /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >> endobj
|
||||
3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Annots [5 0 R] >> endobj
|
||||
4 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Annots [6 0 R 7 0 R] >> endobj
|
||||
5 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A0) /T (Alpha) /Contents (COVER) >> endobj
|
||||
6 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A0) /T (Beta) /Contents (SECOND) >> endobj
|
||||
7 0 obj << /Type /Annot /Subtype /Text /Rect [20 100 40 120] /NM (fitz-A1) /IRT 6 0 R /T (Gamma) /Contents (ANSWER) >> endobj
|
||||
trailer << /Root 1 0 R >>
|
||||
%%EOF";
|
||||
let mut s = Session::new().with_clock(|| 1_700_000_000);
|
||||
let id = s.open("fitz.pdf", None, Arc::new(pdf.to_vec()), None).expect("opens");
|
||||
let check = |s: &Session, replies: usize| {
|
||||
let all = &s.get(id).unwrap().info.annotations;
|
||||
let answer = all.iter().find(|a| a.contents.as_deref() == Some("ANSWER")).unwrap();
|
||||
assert_eq!(answer.parent, Some((1, 0)), "{answer:?}");
|
||||
let text = comment_summary("fitz.pdf", all, SummarySort::Page);
|
||||
let (cover, second) = text.split_once("SECOND").unwrap();
|
||||
assert!(!cover.contains("ANSWER"), "page 1's comment has no replies: {text}");
|
||||
assert_eq!(second.matches(" Author:").count(), replies, "{text}");
|
||||
};
|
||||
check(&s, 1);
|
||||
s.apply(id, Edit::ReplyToAnnotation { page: 1, index: 0, text: "LATER".into(), author: "Delta".into() }).unwrap();
|
||||
check(&s, 2);
|
||||
}
|
||||
|
||||
/// #820: the generated summary includes replies to replies, indented one level deeper.
|
||||
#[test]
|
||||
fn comment_summary_includes_replies_to_replies() {
|
||||
|
||||
+124
-103
@@ -6,6 +6,7 @@
|
||||
//! are empty and `warnings` says why.
|
||||
|
||||
use std::cell::OnceCell;
|
||||
use std::collections::hash_map::Entry;
|
||||
use std::collections::{BTreeMap, HashMap, HashSet};
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -417,9 +418,13 @@ pub struct Annotation {
|
||||
pub author: Option<String>,
|
||||
pub contents: Option<String>,
|
||||
pub modified: Option<String>,
|
||||
/// `/NM` of this annotation, and of the one it replies to (`/IRT`), for threading.
|
||||
/// `/NM` of this annotation, and of the one it replies to (`/IRT`).
|
||||
pub name: Option<String>,
|
||||
pub in_reply_to: Option<String>,
|
||||
/// The page and `/Annots` position of the annotation `/IRT` points at, found by object
|
||||
/// reference. Threads follow this rather than `/NM`, which need not be unique: PyMuPDF
|
||||
/// names the first comment on every page `fitz-A0`.
|
||||
pub parent: Option<(usize, usize)>,
|
||||
/// Rect in PDF user space: [x0, y0, x1, y1].
|
||||
pub rect: [f32; 4],
|
||||
pub color: Option<[f32; 3]>,
|
||||
@@ -446,6 +451,16 @@ impl Annotation {
|
||||
pub fn is_mark(&self) -> bool {
|
||||
matches!(self.state.as_deref(), Some("Marked" | "Unmarked"))
|
||||
}
|
||||
|
||||
/// A reply (or status reply) to another annotation rather than a comment of its own.
|
||||
pub fn is_reply(&self) -> bool {
|
||||
self.in_reply_to.is_some() || self.parent.is_some()
|
||||
}
|
||||
|
||||
/// Whether this annotation's `/IRT` points at `other`.
|
||||
pub fn replies_to(&self, other: &Annotation) -> bool {
|
||||
self.parent == Some((other.page, other.index))
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
@@ -1250,13 +1265,12 @@ impl<'a> Inspector<'a> {
|
||||
pages.sort_unstable_by_key(|&(_, page)| page);
|
||||
// An annotation, or a whole `/Annots` array, that several entries or pages list is on the
|
||||
// first of them (§12.5.2 allows one page per annotation). Both are told apart by the object
|
||||
// a reference resolves to, since an indirect object may itself hold a reference.
|
||||
// a reference resolves to, since an indirect object may itself hold a reference. Where each
|
||||
// annotation is listed is collected before any is read, so a reply's `/IRT` finds its
|
||||
// parent by reference even when the parent comes later.
|
||||
let mut seen_arrays = HashSet::new();
|
||||
let mut seen_annotations = HashSet::new();
|
||||
let mut kept = 0usize;
|
||||
// Once the comment budget is spent, comments are no longer built, but links and attachments
|
||||
// are still listed.
|
||||
let mut comments_full = false;
|
||||
let mut slots: HashMap<ObjectId, (usize, usize)> = HashMap::new();
|
||||
let mut entries = Vec::new();
|
||||
for (id, page) in pages {
|
||||
let Ok(page_dict) = self.doc.get_dictionary(id) else { continue };
|
||||
if let Ok(annots) = page_dict.get(b"Annots")
|
||||
@@ -1267,105 +1281,112 @@ impl<'a> Inspector<'a> {
|
||||
}
|
||||
let Ok(Object::Array(annots)) = page_dict.get(b"Annots").map(|o| self.resolve(o)) else { continue };
|
||||
for (index, a) in annots.iter().enumerate() {
|
||||
if let Some(r) = self.resolved_id(a)
|
||||
&& !seen_annotations.insert(r)
|
||||
{
|
||||
continue;
|
||||
if let Some(r) = self.resolved_id(a) {
|
||||
let Entry::Vacant(slot) = slots.entry(r) else { continue };
|
||||
slot.insert((page, index));
|
||||
}
|
||||
let Some(d) = self.dict(a) else { continue };
|
||||
let Some(subtype) = self.name(d, b"Subtype") else { continue };
|
||||
if subtype == "FileAttachment"
|
||||
&& let Ok(fs) = d.get(b"FS")
|
||||
{
|
||||
let fallback = |budget: &mut usize| self.budgeted_text(d, b"Contents", budget);
|
||||
info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback));
|
||||
}
|
||||
if subtype == "Link" {
|
||||
if let Some(link) = self.link(page, index, d, text_budget) {
|
||||
if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") {
|
||||
info.has_javascript = true;
|
||||
}
|
||||
info.links.push(link);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// Not comments: form widgets, pop-ups and non-markup annotations such as the
|
||||
// Screen annotation that drives a LaTeX `animate` player (keep in step with
|
||||
// `pdfcraft_annot::is_comment_subtype`).
|
||||
if matches!(subtype.as_str(), "Widget" | "Popup" | "Screen" | "Movie" | "RichMedia" | "3D" | "PrinterMark" | "TrapNet" | "Watermark")
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if comments_full {
|
||||
continue;
|
||||
}
|
||||
let rect = rect4(self.resolve(d.get(b"Rect").unwrap_or(&Object::Null)));
|
||||
let mut color = match d.get(b"C").map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(c)) if c.len() == 3 => {
|
||||
let f = |i: usize| c[i].as_float().unwrap_or(0.0);
|
||||
Some([f(0), f(1), f(2)])
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
// A text box's /C is its background; its text colour is in /DA.
|
||||
if subtype == "FreeText"
|
||||
&& let Some(da) = self.text(d, b"DA")
|
||||
{
|
||||
let t: Vec<&str> = da.split_whitespace().collect();
|
||||
if let Some(i) = t.iter().position(|x| *x == "rg")
|
||||
&& i >= 3
|
||||
{
|
||||
let f = |k: usize| t[k].parse::<f32>().unwrap_or(0.0);
|
||||
color = Some([f(i - 3), f(i - 2), f(i - 1)]);
|
||||
}
|
||||
}
|
||||
let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(q)) => {
|
||||
let v: Vec<f32> = q.iter().map(|o| o.as_float().unwrap_or(0.0)).collect();
|
||||
v.as_chunks::<8>().0.to_vec()
|
||||
}
|
||||
_ => Vec::new(),
|
||||
};
|
||||
let in_reply_to = d.get(b"IRT").ok().and_then(|o| self.dict(o)).and_then(|p| self.text(p, b"NM"));
|
||||
let fill_sign = self.is_fill_sign(d, &subtype);
|
||||
// Keep in step with `pdfcraft_annot::summaries`.
|
||||
let key: &[u8] = if subtype == "Line" { b"L" } else { b"Vertices" };
|
||||
let points = match d.get(key).map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(v)) if matches!(subtype.as_str(), "Line" | "PolyLine" | "Polygon") => {
|
||||
let v: Vec<f32> = v.iter().map(|o| o.as_float().unwrap_or(0.0)).collect();
|
||||
let pts: Vec<[f32; 2]> = v.as_chunks::<2>().0.to_vec();
|
||||
if subtype == "Line" && pts.len() != 2 { Vec::new() } else { pts }
|
||||
}
|
||||
_ => Vec::new(),
|
||||
};
|
||||
let annotation = Annotation {
|
||||
page,
|
||||
subtype,
|
||||
author: self.text(d, b"T"),
|
||||
// Acrobat ends lines with CR, which the interface would draw as nothing (keep
|
||||
// in step with `pdfcraft_annot::summaries`).
|
||||
contents: self.text(d, b"Contents").map(|s| s.replace("\r\n", "\n").replace('\r', "\n")),
|
||||
modified: self.text(d, b"M").map(|m| pretty_date(&m)),
|
||||
name: self.text(d, b"NM"),
|
||||
in_reply_to,
|
||||
rect,
|
||||
color,
|
||||
index,
|
||||
state: self.text(d, b"State"),
|
||||
quads,
|
||||
locked: d.get(b"F").ok().and_then(|f| self.resolve(f).as_i64().ok()).unwrap_or(0) & 128 != 0,
|
||||
intent: self.name(d, b"IT"),
|
||||
fill_sign,
|
||||
points,
|
||||
};
|
||||
kept = kept.saturating_add(retained(&annotation));
|
||||
if kept > MAX_ANNOTATION_BYTES {
|
||||
comments_full = true;
|
||||
continue;
|
||||
}
|
||||
info.annotations.push(annotation);
|
||||
entries.push((page, index, a));
|
||||
}
|
||||
}
|
||||
let mut kept = 0usize;
|
||||
// Once the comment budget is spent, comments are no longer built, but links and attachments
|
||||
// are still listed.
|
||||
let mut comments_full = false;
|
||||
for (page, index, a) in entries {
|
||||
let Some(d) = self.dict(a) else { continue };
|
||||
let Some(subtype) = self.name(d, b"Subtype") else { continue };
|
||||
if subtype == "FileAttachment"
|
||||
&& let Ok(fs) = d.get(b"FS")
|
||||
{
|
||||
let fallback = |budget: &mut usize| self.budgeted_text(d, b"Contents", budget);
|
||||
info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback));
|
||||
}
|
||||
if subtype == "Link" {
|
||||
if let Some(link) = self.link(page, index, d, text_budget) {
|
||||
if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") {
|
||||
info.has_javascript = true;
|
||||
}
|
||||
info.links.push(link);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// Not comments: form widgets, pop-ups and non-markup annotations such as the
|
||||
// Screen annotation that drives a LaTeX `animate` player (keep in step with
|
||||
// `pdfcraft_annot::is_comment_subtype`).
|
||||
if matches!(subtype.as_str(), "Widget" | "Popup" | "Screen" | "Movie" | "RichMedia" | "3D" | "PrinterMark" | "TrapNet" | "Watermark") {
|
||||
continue;
|
||||
}
|
||||
if comments_full {
|
||||
continue;
|
||||
}
|
||||
let rect = rect4(self.resolve(d.get(b"Rect").unwrap_or(&Object::Null)));
|
||||
let mut color = match d.get(b"C").map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(c)) if c.len() == 3 => {
|
||||
let f = |i: usize| c[i].as_float().unwrap_or(0.0);
|
||||
Some([f(0), f(1), f(2)])
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
// A text box's /C is its background; its text colour is in /DA.
|
||||
if subtype == "FreeText"
|
||||
&& let Some(da) = self.text(d, b"DA")
|
||||
{
|
||||
let t: Vec<&str> = da.split_whitespace().collect();
|
||||
if let Some(i) = t.iter().position(|x| *x == "rg")
|
||||
&& i >= 3
|
||||
{
|
||||
let f = |k: usize| t[k].parse::<f32>().unwrap_or(0.0);
|
||||
color = Some([f(i - 3), f(i - 2), f(i - 1)]);
|
||||
}
|
||||
}
|
||||
let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(q)) => {
|
||||
let v: Vec<f32> = q.iter().map(|o| o.as_float().unwrap_or(0.0)).collect();
|
||||
v.as_chunks::<8>().0.to_vec()
|
||||
}
|
||||
_ => Vec::new(),
|
||||
};
|
||||
let in_reply_to = d.get(b"IRT").ok().and_then(|o| self.dict(o)).and_then(|p| self.text(p, b"NM"));
|
||||
let parent = d.get(b"IRT").ok().and_then(|o| self.resolved_id(o)).and_then(|r| slots.get(&r).copied());
|
||||
let fill_sign = self.is_fill_sign(d, &subtype);
|
||||
// Keep in step with `pdfcraft_annot::summaries`.
|
||||
let key: &[u8] = if subtype == "Line" { b"L" } else { b"Vertices" };
|
||||
let points = match d.get(key).map(|o| self.resolve(o)) {
|
||||
Ok(Object::Array(v)) if matches!(subtype.as_str(), "Line" | "PolyLine" | "Polygon") => {
|
||||
let v: Vec<f32> = v.iter().map(|o| o.as_float().unwrap_or(0.0)).collect();
|
||||
let pts: Vec<[f32; 2]> = v.as_chunks::<2>().0.to_vec();
|
||||
if subtype == "Line" && pts.len() != 2 { Vec::new() } else { pts }
|
||||
}
|
||||
_ => Vec::new(),
|
||||
};
|
||||
let annotation = Annotation {
|
||||
page,
|
||||
subtype,
|
||||
author: self.text(d, b"T"),
|
||||
// Acrobat ends lines with CR, which the interface would draw as nothing (keep
|
||||
// in step with `pdfcraft_annot::summaries`).
|
||||
contents: self.text(d, b"Contents").map(|s| s.replace("\r\n", "\n").replace('\r', "\n")),
|
||||
modified: self.text(d, b"M").map(|m| pretty_date(&m)),
|
||||
name: self.text(d, b"NM"),
|
||||
in_reply_to,
|
||||
parent,
|
||||
rect,
|
||||
color,
|
||||
index,
|
||||
state: self.text(d, b"State"),
|
||||
quads,
|
||||
locked: d.get(b"F").ok().and_then(|f| self.resolve(f).as_i64().ok()).unwrap_or(0) & 128 != 0,
|
||||
intent: self.name(d, b"IT"),
|
||||
fill_sign,
|
||||
points,
|
||||
};
|
||||
kept = kept.saturating_add(retained(&annotation));
|
||||
if kept > MAX_ANNOTATION_BYTES {
|
||||
comments_full = true;
|
||||
continue;
|
||||
}
|
||||
info.annotations.push(annotation);
|
||||
}
|
||||
info.annotations.sort_by(|a, b| a.page.cmp(&b.page).then(b.rect[3].total_cmp(&a.rect[3])));
|
||||
}
|
||||
|
||||
|
||||
@@ -2271,7 +2271,7 @@ impl DocView {
|
||||
/// The hover popup for an annotation. Replies and Fill & Sign marks (typed text, checks, signatures)
|
||||
/// show none: they are page content being edited, not comments.
|
||||
fn annotation_hover_text(a: &pdfcraft_render::Annotation) -> Option<String> {
|
||||
if a.in_reply_to.is_some() || a.fill_sign {
|
||||
if a.is_reply() || a.fill_sign {
|
||||
return None;
|
||||
}
|
||||
let who = a.author.clone().unwrap_or_else(|| a.subtype.clone());
|
||||
@@ -5155,7 +5155,7 @@ fn quick_bar(app: &mut PdfCraftApp, area: Rect, ui: &mut egui::Ui) {
|
||||
.info
|
||||
.annotations
|
||||
.iter()
|
||||
.any(|a| a.page == page && a.index == annotation && a.in_reply_to.is_none() && comments::made_by(a, tool))
|
||||
.any(|a| a.page == page && a.index == annotation && !a.is_reply() && comments::made_by(a, tool))
|
||||
})
|
||||
{
|
||||
app.apply_edit(Edit::StyleAnnotation {
|
||||
@@ -5737,6 +5737,7 @@ mod tests {
|
||||
modified: None,
|
||||
name: None,
|
||||
in_reply_to: None,
|
||||
parent: None,
|
||||
rect: [0.0; 4],
|
||||
color: None,
|
||||
index: 0,
|
||||
|
||||
@@ -98,7 +98,7 @@ impl PdfCraftApp {
|
||||
pub fn make_props_default(&mut self, page: usize, index: usize, p: &CommentProps) {
|
||||
let Some((_, id)) = self.active_ids() else { return };
|
||||
let Some(doc) = self.session.get(id) else { return };
|
||||
let Some(a) = doc.info.annotations.iter().find(|a| a.page == page && a.index == index && a.in_reply_to.is_none()) else { return };
|
||||
let Some(a) = doc.info.annotations.iter().find(|a| a.page == page && a.index == index && !a.is_reply()) else { return };
|
||||
let Some(tool) = crate::comments::tool_for_default(a, p.endings.as_deref()) else { return };
|
||||
let mut style = self.comment_prefs.style(tool);
|
||||
if let Some(c) = p.color {
|
||||
@@ -167,12 +167,11 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (boo
|
||||
.and_then(|(_, id)| app.session.get(id))
|
||||
.zip(app.comment_props.as_ref())
|
||||
.map(|(doc, d)| {
|
||||
let me = doc.info.annotations.iter().find(|a| a.page == d.page && a.index == d.index && a.in_reply_to.is_none());
|
||||
let nm = me.and_then(|a| a.name.clone());
|
||||
let me = doc.info.annotations.iter().find(|a| a.page == d.page && a.index == d.index && !a.is_reply());
|
||||
doc.info
|
||||
.annotations
|
||||
.iter()
|
||||
.filter(|r| r.state.is_some() && nm.is_some() && r.in_reply_to == nm)
|
||||
.filter(|r| r.state.is_some() && me.is_some_and(|a| r.replies_to(a)))
|
||||
.map(|r| (r.state.clone().unwrap_or_default(), r.author.clone().unwrap_or_default(), r.modified.clone().unwrap_or_default()))
|
||||
.collect()
|
||||
})
|
||||
@@ -180,7 +179,7 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (boo
|
||||
let default_tool = app.comment_props.as_ref().and_then(|d| {
|
||||
let (_, id) = app.active_ids()?;
|
||||
let doc = app.session.get(id)?;
|
||||
let a = doc.info.annotations.iter().find(|a| a.page == d.page && a.index == d.index && a.in_reply_to.is_none())?;
|
||||
let a = doc.info.annotations.iter().find(|a| a.page == d.page && a.index == d.index && !a.is_reply())?;
|
||||
crate::comments::tool_for_default(a, d.edited.endings.as_deref())
|
||||
});
|
||||
let Some(d) = app.comment_props.as_mut() else { return (false, true) };
|
||||
|
||||
@@ -624,7 +624,7 @@ impl PageCx<'_> {
|
||||
|
||||
/// Top-level comments on this page, in paint order.
|
||||
fn comments(&self) -> impl Iterator<Item = &Annotation> {
|
||||
self.info.annotations.iter().filter(move |a| !self.hidden && a.page == self.page && a.in_reply_to.is_none() && a.subtype != "Popup")
|
||||
self.info.annotations.iter().filter(move |a| !self.hidden && a.page == self.page && !a.is_reply() && a.subtype != "Popup")
|
||||
}
|
||||
|
||||
/// Where a comment is on screen: one rectangle, or one per marked line for text markup.
|
||||
@@ -1843,7 +1843,7 @@ pub(crate) fn attachment_items(ui: &mut egui::Ui, view: &mut DocView, info: &Doc
|
||||
/// Items of the canvas context menu for the selected comment (or the page).
|
||||
pub(crate) fn context_menu(ui: &mut egui::Ui, view: &mut DocView, info: &DocInfo, prefs: &CommentPrefs, allowed: bool) -> Option<CanvasAction> {
|
||||
let mut action = None;
|
||||
let selected = view.comments.selected.and_then(|(p, i)| info.annotations.iter().find(|a| a.page == p && a.index == i && a.in_reply_to.is_none()));
|
||||
let selected = view.comments.selected.and_then(|(p, i)| info.annotations.iter().find(|a| a.page == p && a.index == i && !a.is_reply()));
|
||||
match selected {
|
||||
Some(a) => {
|
||||
let (page, index) = (a.page, a.index);
|
||||
@@ -1888,7 +1888,7 @@ pub(crate) fn context_menu(ui: &mut egui::Ui, view: &mut DocView, info: &DocInfo
|
||||
}
|
||||
});
|
||||
});
|
||||
let thread: Vec<&pdfcraft_render::Annotation> = info.annotations.iter().filter(|r| a.name.is_some() && r.in_reply_to == a.name).collect();
|
||||
let thread: Vec<&pdfcraft_render::Annotation> = info.annotations.iter().filter(|r| r.replies_to(a)).collect();
|
||||
let marked = crate::comments_panel::is_marked(&thread);
|
||||
if ui.add_enabled(allowed, egui::Button::new(tl!(if marked { "Remove checkmark" } else { "Mark with checkmark" }))).clicked() {
|
||||
action = Some(CanvasAction::Edit(Box::new(Edit::MarkAnnotation { page, index, marked: !marked, author: prefs.author.clone() })));
|
||||
@@ -2397,6 +2397,8 @@ mod tests {
|
||||
modified: None,
|
||||
name: None,
|
||||
in_reply_to: None,
|
||||
parent: None,
|
||||
grouped: false,
|
||||
rect: [10.0, 10.0, 50.0, 50.0],
|
||||
color: None,
|
||||
index: 0,
|
||||
|
||||
@@ -87,9 +87,10 @@ pub fn is_marked(thread: &[&Annotation]) -> bool {
|
||||
thread.iter().rev().find(|r| r.is_mark()).is_some_and(|r| r.state.as_deref() == Some("Marked"))
|
||||
}
|
||||
|
||||
/// The number shown in the header: comments and their replies (not status changes).
|
||||
/// The number shown in the header: comments, not their replies or status changes (as Acrobat
|
||||
/// counts them).
|
||||
pub fn count(info: &DocInfo) -> usize {
|
||||
info.annotations.iter().filter(|a| a.state.is_none()).count()
|
||||
info.annotations.iter().filter(|a| !a.is_reply() && a.state.is_none()).count()
|
||||
}
|
||||
|
||||
fn matches(a: &Annotation, q: &str) -> bool {
|
||||
@@ -157,25 +158,28 @@ pub(crate) fn show(
|
||||
return edit;
|
||||
}
|
||||
let query = view.comments.search.clone().filter(|q| !q.trim().is_empty());
|
||||
let mut replies: HashMap<&str, Vec<&Annotation>> = HashMap::new();
|
||||
let mut replies: HashMap<(usize, usize), Vec<&Annotation>> = HashMap::new();
|
||||
let mut page_counts: HashMap<usize, usize> = HashMap::new();
|
||||
// Keep global name matching and source reply order. Page badges count roots before filtering.
|
||||
// Replies find their parent by reference (`/NM` need not be unique) and keep source order.
|
||||
// Page badges count roots before filtering.
|
||||
for a in &info.annotations {
|
||||
if let Some(parent) = a.in_reply_to.as_deref() {
|
||||
replies.entry(parent).or_default().push(a);
|
||||
if a.is_reply() {
|
||||
if let Some(parent) = a.parent {
|
||||
replies.entry(parent).or_default().push(a);
|
||||
}
|
||||
} else {
|
||||
let count = page_counts.entry(a.page).or_default();
|
||||
*count = count.saturating_add(1);
|
||||
}
|
||||
}
|
||||
let replies_of = |a: &Annotation| -> &[&Annotation] { a.name.as_deref().and_then(|name| replies.get(name)).map(Vec::as_slice).unwrap_or(&[]) };
|
||||
let replies_of = |a: &Annotation| -> &[&Annotation] { replies.get(&(a.page, a.index)).map(Vec::as_slice).unwrap_or(&[]) };
|
||||
let cv = &view.comments;
|
||||
let status_of =
|
||||
|a: &Annotation| replies_of(a).iter().rev().filter(|r| !r.is_mark()).find_map(|r| r.state.clone()).unwrap_or_else(|| "None".into());
|
||||
let mut roots: Vec<&Annotation> = info
|
||||
.annotations
|
||||
.iter()
|
||||
.filter(|a| a.in_reply_to.is_none())
|
||||
.filter(|a| !a.is_reply())
|
||||
.filter(|a| query.as_deref().is_none_or(|q| matches(a, q) || replies_of(a).iter().any(|r| matches(r, q))))
|
||||
.filter(|a| !cv.hidden_types.iter().any(|t| t == kind_label(a)))
|
||||
.filter(|a| !cv.hidden_authors.contains(&a.author.clone().unwrap_or_default()))
|
||||
@@ -301,8 +305,9 @@ fn card(
|
||||
.show(ui, |ui| {
|
||||
ui.set_width(ui.available_width());
|
||||
let mut buttons_left = f32::INFINITY;
|
||||
let mut above = egui::Rect::NOTHING;
|
||||
let head = ui.horizontal(|ui| {
|
||||
avatar(ui, t, Some(subtype_icon(&a.subtype)), None, color);
|
||||
above = avatar(ui, t, Some(subtype_icon(&a.subtype)), None, color);
|
||||
let author = a.author.as_deref().map_or_else(|| tl!("Unknown author").into(), crate::bidi::visual_auto);
|
||||
ui.label(egui::RichText::new(author).font(theme::semibold(12.5)).color(t.text));
|
||||
if let Some(m) = &a.modified {
|
||||
@@ -373,11 +378,13 @@ fn card(
|
||||
});
|
||||
}
|
||||
});
|
||||
// Replies, joined to the parent by a thread line.
|
||||
// Replies, each joined to the avatar above it (the comment's or the previous reply's)
|
||||
// by a thread line that runs the whole gap.
|
||||
for r in &replies {
|
||||
ui.add_space(8.0);
|
||||
let resp = ui.horizontal(|ui| {
|
||||
avatar(ui, t, None, Some(&initials(r.author.as_deref().unwrap_or("?"))), t.text_faint);
|
||||
let mut me = egui::Rect::NOTHING;
|
||||
ui.horizontal(|ui| {
|
||||
me = avatar(ui, t, None, Some(&initials(r.author.as_deref().unwrap_or("?"))), t.text_faint);
|
||||
ui.vertical(|ui| {
|
||||
ui.spacing_mut().item_spacing.y = 2.0;
|
||||
ui.horizontal(|ui| {
|
||||
@@ -390,8 +397,12 @@ fn card(
|
||||
widgets::wrapped_text(ui, r.contents.as_deref().unwrap_or(""), theme::regular(12.5), t.text, false);
|
||||
});
|
||||
});
|
||||
let x = resp.response.rect.left() + 12.0;
|
||||
ui.painter().vline(x, (resp.response.rect.top() - 14.0)..=(resp.response.rect.top() - 2.0), Stroke::new(1.5, t.border));
|
||||
// The circles are 12 px in radius; leave a 1 px gap at each end.
|
||||
let (top, bottom) = (above.center().y + 13.0, me.center().y - 13.0);
|
||||
if above.is_positive() && top < bottom {
|
||||
ui.painter().vline(me.center().x, top..=bottom, Stroke::new(1.5, t.border));
|
||||
}
|
||||
above = me;
|
||||
}
|
||||
if selected && allowed {
|
||||
ui.add_space(8.0);
|
||||
@@ -499,7 +510,8 @@ fn card(
|
||||
}
|
||||
|
||||
/// A 26 pt round badge: a type glyph (comments) or initials (replies).
|
||||
fn avatar(ui: &mut egui::Ui, t: &Tokens, icon: Option<&str>, text: Option<&str>, color: Color32) {
|
||||
/// A round avatar; returns where it was drawn.
|
||||
fn avatar(ui: &mut egui::Ui, t: &Tokens, icon: Option<&str>, text: Option<&str>, color: Color32) -> egui::Rect {
|
||||
let (r, _) = ui.allocate_exact_size(vec2(26.0, 26.0), Sense::hover());
|
||||
let bg = if t.dark() { Color32::from_gray(0x55) } else { Color32::from_gray(0xD8) };
|
||||
ui.painter().circle_filled(r.center(), 12.0, bg);
|
||||
@@ -509,6 +521,7 @@ fn avatar(ui: &mut egui::Ui, t: &Tokens, icon: Option<&str>, text: Option<&str>,
|
||||
if let Some(s) = text {
|
||||
ui.painter().text(r.center(), egui::Align2::CENTER_CENTER, s, theme::semibold(10.5), t.text);
|
||||
}
|
||||
r
|
||||
}
|
||||
|
||||
/// Darken very light annotation colours (e.g. note yellow) so their glyph stays legible.
|
||||
@@ -527,15 +540,13 @@ fn legible(c: Color32, t: &Tokens) -> Color32 {
|
||||
pub(crate) fn header_controls(ui: &mut egui::Ui, info: &DocInfo, view: &mut DocView, hidden: bool) -> Option<&'static str> {
|
||||
let mut command = None;
|
||||
let cv = &mut view.comments;
|
||||
let mut types: Vec<String> = info.annotations.iter().filter(|a| a.in_reply_to.is_none()).map(|a| kind_label(a).to_string()).collect();
|
||||
let mut types: Vec<String> = info.annotations.iter().filter(|a| !a.is_reply()).map(|a| kind_label(a).to_string()).collect();
|
||||
types.sort();
|
||||
types.dedup();
|
||||
let mut authors: Vec<String> =
|
||||
info.annotations.iter().filter(|a| a.in_reply_to.is_none()).map(|a| a.author.clone().unwrap_or_default()).collect();
|
||||
let mut authors: Vec<String> = info.annotations.iter().filter(|a| !a.is_reply()).map(|a| a.author.clone().unwrap_or_default()).collect();
|
||||
authors.sort();
|
||||
authors.dedup();
|
||||
let mut colors: Vec<(String, Option<[f32; 3]>)> =
|
||||
info.annotations.iter().filter(|a| a.in_reply_to.is_none()).map(|a| (color_key(a), a.color)).collect();
|
||||
let mut colors: Vec<(String, Option<[f32; 3]>)> = info.annotations.iter().filter(|a| !a.is_reply()).map(|a| (color_key(a), a.color)).collect();
|
||||
colors.sort_by(|a, b| a.0.cmp(&b.0));
|
||||
colors.dedup_by(|a, b| a.0 == b.0);
|
||||
let active = !(cv.hidden_types.is_empty()
|
||||
|
||||
@@ -155,7 +155,7 @@ impl PdfCraftApp {
|
||||
}
|
||||
Edit::AddAnnotation(a) => {
|
||||
// Select the new comment (appended last among the page's comments).
|
||||
let newest = info.annotations.iter().filter(|x| x.page == a.page && x.in_reply_to.is_none()).map(|x| x.index).max();
|
||||
let newest = info.annotations.iter().filter(|x| x.page == a.page && !x.is_reply()).map(|x| x.index).max();
|
||||
view.comments.selected = newest.map(|n| (a.page, n));
|
||||
view.comments.reveal = true;
|
||||
// A new highlight opens its note for typing straight away, as in Acrobat,
|
||||
@@ -500,7 +500,7 @@ impl PdfCraftApp {
|
||||
let newest = self
|
||||
.session
|
||||
.get(view.id)
|
||||
.and_then(|d| d.info.annotations.iter().filter(|a| a.page == page && a.in_reply_to.is_none()).map(|a| a.index).max());
|
||||
.and_then(|d| d.info.annotations.iter().filter(|a| a.page == page && !a.is_reply()).map(|a| a.index).max());
|
||||
view.comments.selected = newest.map(|index| (page, index));
|
||||
view.comments.reveal = true;
|
||||
self.quick_tool = crate::QuickTool::Select;
|
||||
|
||||
@@ -1085,8 +1085,9 @@ fn the_panel_filters_and_sorts() {
|
||||
assert_eq!(h.query_all_by_label("Oval").count(), ovals + 1, "a group header was added");
|
||||
}
|
||||
|
||||
// Contributor-original PDF. Two roots deliberately share /NM. Their replies are on
|
||||
// another page, with dates opposite to source order. An unnamed root has no thread.
|
||||
// Contributor-original PDF. Two roots deliberately share /NM; the replies (on another page,
|
||||
// with dates opposite to source order) point at the first by reference. An unnamed root has
|
||||
// no thread.
|
||||
const REVIEW_FIXTURE: &[u8] = b"%PDF-1.7
|
||||
1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
|
||||
2 0 obj << /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >> endobj
|
||||
@@ -1107,7 +1108,7 @@ trailer << /Root 1 0 R >>
|
||||
%%EOF";
|
||||
|
||||
#[test]
|
||||
fn the_panel_preserves_global_threads_source_order_and_unfiltered_page_counts() {
|
||||
fn the_panel_preserves_threads_source_order_and_unfiltered_page_counts() {
|
||||
let mut h = harness_with_bytes(REVIEW_FIXTURE.to_vec(), |app| {
|
||||
app.set_option("panel", "comments").unwrap();
|
||||
app.views[0].comments.search = Some("needle".into());
|
||||
@@ -1129,14 +1130,16 @@ fn the_panel_preserves_global_threads_source_order_and_unfiltered_page_counts()
|
||||
|
||||
h.state_mut().views[0].comments.hidden_authors.clear();
|
||||
h.run_steps(2);
|
||||
h.get_by_label("Duplicate root");
|
||||
assert_eq!(h.query_all_by_label("Needle first reply").count(), 2, "duplicate names share the same global thread");
|
||||
// The replies point at the first root: sharing its /NM doesn't give the other its thread
|
||||
// (or a search match on that thread).
|
||||
assert_eq!(h.query_all_by_label("Duplicate root").count(), 0);
|
||||
assert_eq!(h.query_all_by_label("Needle first reply").count(), 1);
|
||||
h.state_mut().views[0].comments.search = Some("status-only token".into());
|
||||
h.state_mut().views[0].comments.hidden_statuses.push("Rejected".into());
|
||||
h.state_mut().views[0].comments.hidden_checks.push("Checked".into());
|
||||
h.run_steps(2);
|
||||
h.get_by_label("First root");
|
||||
h.get_by_label("Duplicate root");
|
||||
assert_eq!(h.query_all_by_label("Duplicate root").count(), 0);
|
||||
|
||||
h.state_mut().views[0].comments.hidden_statuses.push("Accepted".into());
|
||||
h.run_steps(2);
|
||||
@@ -1153,6 +1156,31 @@ fn the_panel_preserves_global_threads_source_order_and_unfiltered_page_counts()
|
||||
assert_eq!(h.query_all_by_label("Needle first reply").count(), 0, "an unnamed root has no replies");
|
||||
}
|
||||
|
||||
// Contributor-original PDF, shaped like PyMuPDF output: every page's first comment is named
|
||||
// `fitz-A0`, so only the `/IRT` reference tells which comment a reply belongs to.
|
||||
const PER_PAGE_NAMES: &[u8] = b"%PDF-1.7
|
||||
1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
|
||||
2 0 obj << /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >> endobj
|
||||
3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Annots [5 0 R] >> endobj
|
||||
4 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Annots [6 0 R 7 0 R 8 0 R] >> endobj
|
||||
5 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A0) /T (Alpha) /Contents (Cover note) >> endobj
|
||||
6 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A0) /T (Beta) /Contents (Second page note) >> endobj
|
||||
7 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A1) /IRT 6 0 R /T (Gamma) /Contents (Reply on page two) >> endobj
|
||||
8 0 obj << /Type /Annot /Subtype /Text /Rect [20 140 40 160] /NM (fitz-A2) /IRT 6 0 R /T (Beta) /Contents (Answer on page two) >> endobj
|
||||
trailer << /Root 1 0 R >>
|
||||
%%EOF";
|
||||
|
||||
#[test]
|
||||
fn replies_follow_irt_references_when_names_repeat_on_every_page() {
|
||||
let h = harness_with_bytes(PER_PAGE_NAMES.to_vec(), |app| app.set_option("panel", "comments").unwrap());
|
||||
let reply = h.get_by_label("Reply on page two");
|
||||
assert!(reply.rect().top() > h.get_by_label("Second page note").rect().top(), "the reply sits under page 2's comment");
|
||||
assert!(h.get_by_label("Page 2").rect().top() < reply.rect().top(), "not under page 1's comment, which shares the /NM");
|
||||
h.get_by_label("Answer on page two");
|
||||
// The header counts the two comments, not their replies (Acrobat says 2, not 4).
|
||||
assert_eq!(h.query_all_by_label("4").count(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_panel_reveals_edits_and_reopens_a_late_thread_without_changing_its_replies() {
|
||||
let mut h = harness_with_bytes(REVIEW_FIXTURE.to_vec(), |app| {
|
||||
|
||||
@@ -3560,8 +3560,8 @@ milestone = "M3"
|
||||
status = "shipped"
|
||||
commands = ["comment.list"]
|
||||
tools = ["comment_list"]
|
||||
evidence = ["crates/ui-egui/tests/it/ui.rs::opening_a_pdf_shows_comments_and_bookmarks", "crates/ui-egui/tests/it/comments.rs::the_panel_posts_comments_and_replies", "crates/ui-egui/tests/it/comments.rs::page_groups_collapse_and_expand"]
|
||||
notes = "cards grouped by page with replies, status badges, selection, reply box and an Add a comment box; page groups fold and unfold from their chevron or with Collapse/Expand all pages in the panel's \"…\" menu (a comment selected on a folded page opens its group). Groups by author, type or colour don't fold yet"
|
||||
evidence = ["crates/ui-egui/tests/it/ui.rs::opening_a_pdf_shows_comments_and_bookmarks", "crates/ui-egui/tests/it/comments.rs::the_panel_posts_comments_and_replies", "crates/ui-egui/tests/it/comments.rs::page_groups_collapse_and_expand", "crates/ui-egui/tests/it/comments.rs::replies_follow_irt_references_when_names_repeat_on_every_page", "crates/engine/src/tests.rs::replies_thread_by_reference_when_names_repeat_across_pages"]
|
||||
notes = "cards grouped by page with replies (threaded by the `/IRT` reference, so files whose `/NM` repeats per page, as PyMuPDF writes them, thread correctly; the header counts comments without replies, as Acrobat does), status badges, selection, reply box and an Add a comment box; page groups fold and unfold from their chevron or with Collapse/Expand all pages in the panel's \"…\" menu (a comment selected on a folded page opens its group). Groups by author, type or colour don't fold yet"
|
||||
|
||||
[[feature]]
|
||||
id = "comment.appearance-generation"
|
||||
|
||||
Reference in new issue
Block a user