mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-10 13:01:29 +00:00
M5: Delete deletes the group member clicked, as Acrobat does
Acrobat desktop deletes only the member of a group (/RT /Group) that was clicked: deleting the primary keeps its subordinates, on its page or another, and deleting a subordinate keeps the rest of its group. ISO 32000-2 §12.5.6.2 lists deletion among the operations that act on the entire group; this follows Acrobat. A subordinate left without its primary is a comment of its own. Replace Text stays one comment: its strikeout and caret go together from either, in both orientations (the caret as primary, and the strikeout as primary, as earlier PdfCraft builds wrote it), as Acrobat deletes the pair. On the page, a click on a subordinate still selects its group, and Delete (the key or the menu) acts on the member clicked. The group-primary lookup that deletion used is gone with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv
This commit is contained in:
1 parent
bcbc122de5
commit
8845066ab7
11 files changed
+173
-101
No files matched your search
@@ -89,29 +89,6 @@ pub fn is_group_subordinate(d: &Dict) -> bool {
|
||||
d.name(b"RT") == Some(b"Group") && d.reference(b"IRT").is_some()
|
||||
}
|
||||
|
||||
/// How far a group subordinate's `/IRT` chain is followed to its primary.
|
||||
pub(crate) const MAX_GROUP_DEPTH: usize = 64;
|
||||
|
||||
/// The primary of the group `r` belongs to: up the `/IRT` chain while it leads to another
|
||||
/// subordinate in `list` (one page's `/Annots`). `r` itself when it is not a subordinate, or
|
||||
/// when its primary is not on the page; the chain stops at a loop. `None` when it is still
|
||||
/// on a subordinate after [`MAX_GROUP_DEPTH`] annotations: the group's primary is unknown, as
|
||||
/// `pdfcraft_render::inspect::group_primaries` reads it (keep the two in step).
|
||||
pub(crate) fn group_primary(doc: &Document, list: &[Object], r: ObjRef) -> Option<ObjRef> {
|
||||
let mut seen = vec![r];
|
||||
let mut at = r;
|
||||
for _ in 0..MAX_GROUP_DEPTH {
|
||||
let obj = doc.get(at);
|
||||
let Some(next) = obj.as_dict().filter(|d| is_group_subordinate(d)).and_then(|d| d.reference(b"IRT")) else { return Some(at) };
|
||||
if seen.contains(&next) || !list.iter().any(|e| e.as_ref() == Some(next)) {
|
||||
return Some(at);
|
||||
}
|
||||
seen.push(next);
|
||||
at = next;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Every comment (see [`is_comment_subtype`]), ordered by page and then top edge.
|
||||
pub fn summaries(doc: &Document) -> Vec<Summary> {
|
||||
let mut out = Vec::new();
|
||||
|
||||
+49
-29
@@ -2,7 +2,7 @@ use super::create::{
|
||||
add_annotation, base_dict, bounds, callout_attach, ending_draws, ending_pair, finite, grow, normalize, num_array, rect_for, rgb,
|
||||
};
|
||||
use super::draw::{draw_appearance, redraw_or_restore, set_appearance};
|
||||
use super::info::{MAX_GROUP_DEPTH, group_primary, is_group_subordinate, line_endings_of, text_value};
|
||||
use super::info::{is_group_subordinate, line_endings_of, text_value};
|
||||
use super::lookup::{annot_dict, annot_ref, annotation_flags, annots, page_ref, set_annots};
|
||||
use super::types::{
|
||||
FLAG_HIDDEN, FLAG_LOCKED, FLAG_NO_ROTATE, FLAG_NO_ZOOM, FLAG_PRINT, LineEnding, Markup, Meta, NewAnnotation, ReviewState, Rgb, Shape, Style,
|
||||
@@ -70,11 +70,11 @@ pub(crate) fn touch(d: &mut Dict, meta: &Meta) {
|
||||
}
|
||||
|
||||
/// Delete a comment together with its pop-up and its replies (and theirs), as Acrobat does.
|
||||
/// Refused while it, or a reply that would go with it, is locked. A comment in a group
|
||||
/// (`/RT /Group`) takes the whole group with it: deleting a subordinate deletes its primary, and
|
||||
/// with it every member (ISO 32000-2 §12.5.6.2). A subordinate whose primary is too far up its
|
||||
/// `/IRT` chain to find is refused rather than deleted with only part of its group, and so is a
|
||||
/// group with a locked member.
|
||||
/// Refused while it, or a reply that would go with it, is locked. A member of a group (`/RT
|
||||
/// /Group`), its primary included, goes alone, as Acrobat deletes the member clicked and keeps the
|
||||
/// rest of its group, although ISO 32000-2 §12.5.6.2 lists deletion among the operations that act
|
||||
/// on the entire group; a subordinate left without its primary is a comment of its own. Replace
|
||||
/// Text's strikeout and caret are one comment and go together from either ([`own_deletion`]).
|
||||
pub fn delete_annotation(doc: &mut Document, page: usize, index: usize) -> Result<(), AnnotError> {
|
||||
let index = deleted_index(doc, page, index)?;
|
||||
lift_annotation(doc, page, index)
|
||||
@@ -82,41 +82,42 @@ pub fn delete_annotation(doc: &mut Document, page: usize, index: usize) -> Resul
|
||||
|
||||
/// Whether [`delete_annotation`] deletes the annotation at `(page, index)`, or why not, without
|
||||
/// changing anything: what a Delete command asks before it is offered. It is refused while the
|
||||
/// annotation is locked, or a reply (or group member) that would go with it is.
|
||||
/// annotation is locked, or a reply that would go with it is.
|
||||
pub fn deletable(doc: &Document, page: usize, index: usize) -> Result<(), AnnotError> {
|
||||
deleted_index(doc, page, index).map(|_| ())
|
||||
}
|
||||
|
||||
/// [`deletable`], returning the index [`delete_annotation`] deletes from: the group's primary for
|
||||
/// a group subordinate, else `index`.
|
||||
/// [`deletable`], returning the index [`delete_annotation`] deletes from: the caret for Replace
|
||||
/// Text's strikeout, else `index`.
|
||||
fn deleted_index(doc: &Document, page: usize, index: usize) -> Result<usize, AnnotError> {
|
||||
let list = annots(doc, page_ref(doc, page)?);
|
||||
let mut target = list.get(index).ok_or(AnnotError::NoSuchAnnotation { page, index })?;
|
||||
let mut index = index;
|
||||
if let Some(r) = target.as_ref() {
|
||||
unlocked(doc, r)?;
|
||||
let Some(primary) = group_primary(doc, &list, r) else {
|
||||
return Err(AnnotError::Invalid(format!(
|
||||
"this comment can't be deleted: its group's primary is more than {MAX_GROUP_DEPTH} comments up its chain"
|
||||
)));
|
||||
};
|
||||
if primary != r
|
||||
&& let Some((i, e)) = list.iter().enumerate().find(|(_, e)| e.as_ref() == Some(primary))
|
||||
{
|
||||
unlocked(doc, primary)?;
|
||||
(target, index) = (e, i);
|
||||
}
|
||||
}
|
||||
// Replace Text is one comment: its strikeout goes with its caret, the group's primary, and
|
||||
// the caret with it, as Acrobat deletes the pair from either.
|
||||
let caret = doc
|
||||
.resolve(target)
|
||||
.as_dict()
|
||||
.filter(|d| is_group_subordinate(d) && !own_deletion(doc, d))
|
||||
.and_then(|d| d.reference(b"IRT"))
|
||||
.and_then(|irt| list.iter().enumerate().find(|(_, e)| e.as_ref() == Some(irt)));
|
||||
if let Some((i, e)) = caret {
|
||||
(target, index) = (e, i);
|
||||
}
|
||||
let doomed = with_replies_and_popups(doc, &list, seed(target));
|
||||
// A locked reply can't be deleted on its own, so it doesn't go with the comment or reply it
|
||||
// answers either; nor does a locked group member keep its place while its group goes
|
||||
// (ISO 32000-2 §12.5.3, Table 167: Locked). Pop-ups follow their comment.
|
||||
for (_, e) in list.iter().enumerate().filter(|(i, e)| *i != index && goes_with(e, &doomed)) {
|
||||
// answers either (ISO 32000-2 §12.5.3, Table 167: Locked); nor does the other half of Replace
|
||||
// Text. Pop-ups follow their comment.
|
||||
for (i, e) in list.iter().enumerate().filter(|(i, e)| (*i != index || caret.is_some()) && goes_with(doc, 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
|
||||
{
|
||||
let what = if is_group_subordinate(d) { "a comment in its group is locked" } else { "a reply to it is locked" };
|
||||
let grouped = is_group_subordinate(d) || caret.is_some_and(|(c, _)| c == i);
|
||||
let what = if grouped { "a comment in its group is locked" } else { "a reply to it is locked" };
|
||||
return Err(AnnotError::Invalid(what.into()));
|
||||
}
|
||||
}
|
||||
@@ -131,7 +132,7 @@ pub fn lift_annotation(doc: &mut Document, page: usize, index: usize) -> Result<
|
||||
let list = annots(doc, p);
|
||||
let target = list.get(index).ok_or(AnnotError::NoSuchAnnotation { page, index })?;
|
||||
let doomed = with_replies_and_popups(doc, &list, seed(target));
|
||||
let kept: Vec<Object> = list.iter().enumerate().filter(|(i, e)| *i != index && !goes_with(e, &doomed)).map(|(_, e)| e.clone()).collect();
|
||||
let kept: Vec<Object> = list.iter().enumerate().filter(|(i, e)| *i != index && !goes_with(doc, e, &doomed)).map(|(_, e)| e.clone()).collect();
|
||||
Ok(set_annots(doc, p, kept)?)
|
||||
}
|
||||
|
||||
@@ -145,16 +146,35 @@ fn seed(target: &Object) -> Option<ObjRef> {
|
||||
/// one of them, or it is written inline (a file can, though the standard asks for references) and
|
||||
/// replies to (`/IRT`) or pops up for (`/Parent`) one of them. An inline entry has no reference
|
||||
/// for the walk to collect, so it is matched here.
|
||||
pub fn goes_with(entry: &Object, going: &HashSet<ObjRef>) -> bool {
|
||||
pub fn goes_with(doc: &Document, entry: &Object, going: &HashSet<ObjRef>) -> bool {
|
||||
match entry.as_ref() {
|
||||
Some(r) => going.contains(&r),
|
||||
None => entry.as_dict().is_some_and(|d| [d.reference(b"IRT"), d.reference(b"Parent")].into_iter().flatten().any(|r| going.contains(&r))),
|
||||
None => entry.as_dict().is_some_and(|d| hangs_off(doc, d).into_iter().flatten().any(|r| going.contains(&r))),
|
||||
}
|
||||
}
|
||||
|
||||
/// What the annotation `d` goes with when it is deleted: what it replies to (`/IRT`) and what it
|
||||
/// pops up for (`/Parent`). A group subordinate (`/RT /Group`) is deleted on its own
|
||||
/// ([`own_deletion`]), so its `/IRT` doesn't count.
|
||||
fn hangs_off(doc: &Document, d: &Dict) -> [Option<ObjRef>; 2] {
|
||||
[d.reference(b"IRT").filter(|_| !own_deletion(doc, d)), d.reference(b"Parent")]
|
||||
}
|
||||
|
||||
/// Whether the annotation `d` stays when its group's primary is deleted: a group subordinate
|
||||
/// (`/RT /Group`) does, as Acrobat deletes only the member clicked, unless it is half of Replace
|
||||
/// Text, which Acrobat deletes as one comment: the strikeout (`/IT /StrikeOutTextEdit`) under its
|
||||
/// caret, or a caret under such a strikeout, as earlier PdfCraft builds wrote the pair.
|
||||
fn own_deletion(doc: &Document, d: &Dict) -> bool {
|
||||
let edit = |d: &Dict| d.name(b"IT") == Some(b"StrikeOutTextEdit".as_slice());
|
||||
let under_edit =
|
||||
|| d.name(b"Subtype") == Some(b"Caret".as_slice()) && d.reference(b"IRT").is_some_and(|r| doc.get(r).as_dict().is_some_and(edit));
|
||||
is_group_subordinate(d) && !edit(d) && !under_edit()
|
||||
}
|
||||
|
||||
/// `doomed` and what goes with it, as Acrobat deletes a comment: the entries of `list` (one or
|
||||
/// more pages' `/Annots`) that reply to (`/IRT`) or pop up for (`/Parent`) anything going, their
|
||||
/// own replies, and the pop-ups going annotations name (`/Popup`). An inline reply has no
|
||||
/// own replies, and the pop-ups going annotations name (`/Popup`). A group subordinate stays
|
||||
/// ([`own_deletion`]). An inline reply has no
|
||||
/// reference to collect, but the pop-up it names is. Each annotation is visited once, so a long or
|
||||
/// cyclic chain costs no more than its length. Filter a list with [`goes_with`], which also
|
||||
/// matches inline entries.
|
||||
@@ -166,7 +186,7 @@ pub fn with_replies_and_popups(doc: &Document, list: &[Object], doomed: impl Int
|
||||
// What goes with the annotation this entry hangs off: the entry, or for an inline one the
|
||||
// pop-up it names.
|
||||
let Some(goes) = e.as_ref().or_else(|| d.reference(b"Popup")) else { continue };
|
||||
for on in [d.reference(b"IRT"), d.reference(b"Parent")].into_iter().flatten() {
|
||||
for on in hangs_off(doc, d).into_iter().flatten() {
|
||||
hanging.entry(on).or_default().push(goes);
|
||||
}
|
||||
}
|
||||
|
||||
+31
-34
@@ -913,7 +913,8 @@ fn replace_text_groups_a_strikeout_and_a_caret() {
|
||||
assert!(r[0] < 200.0 && r[2] > 200.0 && r[1] < 706.0, "at the end of the line, below its baseline: {r:?}");
|
||||
let summary = summaries(&doc);
|
||||
assert_eq!(summary.iter().filter(|s| s.page == 0 && !s.is_group_member()).count(), 2, "the fixture's Square and the caret: {summary:?}");
|
||||
// Deleting either member deletes the group.
|
||||
// Replace Text is one comment: deleting either the caret or the strikeout deletes both, as
|
||||
// Acrobat does.
|
||||
for member in [i, a.iter().position(|d| d.name(b"Subtype") == Some(b"StrikeOut")).unwrap()] {
|
||||
let mut doc = doc.clone();
|
||||
delete_annotation(&mut doc, 0, member).unwrap();
|
||||
@@ -942,9 +943,11 @@ fn replace_text_groups_a_strikeout_and_a_caret() {
|
||||
|
||||
/// ISO 32000-2 §12.5.6.2: `/RT /Group` makes an annotation a subordinate of the group its
|
||||
/// `/IRT` annotation is primary of. Another `/RT` reads as a reply, and so does a bare `/IRT`.
|
||||
/// Deleting a subordinate deletes the whole group, with the primary's replies.
|
||||
/// Deleting a member deletes just it, as Acrobat does, although §12.5.6.2 lists deletion among the
|
||||
/// operations that act on the entire group: the subordinate goes alone, and the primary goes with
|
||||
/// its replies and leaves its subordinate, which reads as a comment of its own.
|
||||
#[test]
|
||||
fn a_group_reads_as_one_and_deletes_as_one() {
|
||||
fn a_group_reads_as_one_and_deletes_by_member() {
|
||||
let mut doc = fixture();
|
||||
let square = add_annotation(&mut doc, &new(0, Shape::Rectangle { rect: [100.0, 100.0, 200.0, 200.0] }), &meta("sq")).unwrap();
|
||||
let (_, sr) = annot_ref(&mut doc, 0, square).unwrap();
|
||||
@@ -972,21 +975,23 @@ fn a_group_reads_as_one_and_deletes_as_one() {
|
||||
[("member".into(), true), ("reply".into(), false), ("explicit".into(), false), ("odd".into(), false)],
|
||||
"only /RT /Group groups"
|
||||
);
|
||||
let mut alone = doc.clone();
|
||||
delete_annotation(&mut alone, 0, member).unwrap();
|
||||
assert_eq!(list(&alone, 0).len(), list(&doc, 0).len() - 1, "the subordinate goes alone");
|
||||
let mut doc = doc;
|
||||
delete_annotation(&mut doc, 0, member).unwrap();
|
||||
let left = list(&doc, 0);
|
||||
assert_eq!(left.len(), 1, "the group, its primary and the primary's replies are gone; the unrelated Square stays");
|
||||
assert_eq!(left[0].name(b"Subtype"), Some(&b"Square"[..]));
|
||||
assert!(left[0].get(b"NM").is_none());
|
||||
delete_annotation(&mut doc, 0, square).unwrap();
|
||||
let left: Vec<Option<String>> = list(&doc, 0).iter().map(|d| d.get(b"NM").map(|_| text(d, b"NM"))).collect();
|
||||
assert_eq!(left, [None, Some("member".into())], "the primary and its replies are gone; the fixture's Square and the subordinate stay");
|
||||
let summary = summaries(&doc);
|
||||
let member = summary.iter().find(|s| s.name.as_deref() == Some("member")).unwrap();
|
||||
assert!(member.parent.is_none() && !member.is_group_member(), "left without its primary, it is a comment of its own: {member:?}");
|
||||
}
|
||||
|
||||
/// A subordinate whose `/IRT` chain reaches its primary within the 64 annotations the listing
|
||||
/// follows deletes the whole group; one whose chain runs past them is refused, and the file
|
||||
/// keeps every member, rather than losing only the part of the group below the cut
|
||||
/// (ISO 32000-2 §12.5.6.2).
|
||||
/// A subordinate deep in a chain of subordinates, within the 64 annotations the listing follows
|
||||
/// up a chain or past them, is deleted alone: the chain above it stays.
|
||||
#[test]
|
||||
fn a_group_chain_past_the_depth_limit_is_not_deleted_in_part() {
|
||||
for (subordinates, deletes) in [(63, true), (64, false), (65, false)] {
|
||||
fn a_subordinate_deep_in_a_chain_is_deleted_alone() {
|
||||
for subordinates in [63, 64, 65] {
|
||||
let mut doc = fixture();
|
||||
let top = add_annotation(&mut doc, &new(0, Shape::Rectangle { rect: [100.0, 100.0, 200.0, 200.0] }), &meta("top")).unwrap();
|
||||
let (_, mut up) = annot_ref(&mut doc, 0, top).unwrap();
|
||||
@@ -1002,23 +1007,15 @@ fn a_group_chain_past_the_depth_limit_is_not_deleted_in_part() {
|
||||
up = r;
|
||||
}
|
||||
let before = list(&doc, 0).len();
|
||||
let result = delete_annotation(&mut doc, 0, last);
|
||||
let left = list(&doc, 0).len();
|
||||
if deletes {
|
||||
result.unwrap();
|
||||
assert_eq!(left, 1, "{subordinates}: the group is gone, the fixture's Square stays");
|
||||
} else {
|
||||
assert!(matches!(result, Err(AnnotError::Invalid(_))), "{subordinates}: {result:?}");
|
||||
assert_eq!(left, before, "{subordinates}: nothing is deleted");
|
||||
}
|
||||
delete_annotation(&mut doc, 0, last).unwrap();
|
||||
assert_eq!(list(&doc, 0).len(), before - 1, "{subordinates}: the subordinate is gone, the rest of its group stays");
|
||||
}
|
||||
}
|
||||
|
||||
/// Deleting an unlocked subordinate takes every member of its group, so one locked member,
|
||||
/// even one that is neither the subordinate nor the primary, refuses the whole deletion
|
||||
/// (ISO 32000-2 §12.5.3, Table 167: Locked).
|
||||
/// A locked member doesn't hold back the deletion of another, the primary included: each goes
|
||||
/// alone (ISO 32000-2 §12.5.3, Table 167: Locked).
|
||||
#[test]
|
||||
fn a_locked_group_member_keeps_its_group() {
|
||||
fn a_locked_group_member_holds_back_no_other_deletion() {
|
||||
let mut doc = fixture();
|
||||
let top = add_annotation(&mut doc, &new(0, Shape::Rectangle { rect: [100.0, 100.0, 200.0, 200.0] }), &meta("top")).unwrap();
|
||||
let (_, tr) = annot_ref(&mut doc, 0, top).unwrap();
|
||||
@@ -1035,13 +1032,13 @@ fn a_locked_group_member_keeps_its_group() {
|
||||
}
|
||||
set_locked(&mut doc, 0, members[1], true).unwrap();
|
||||
let before = list(&doc, 0).len();
|
||||
let result = delete_annotation(&mut doc, 0, members[0]);
|
||||
assert!(matches!(result, Err(AnnotError::Invalid(_))), "{result:?}");
|
||||
assert_eq!(list(&doc, 0).len(), before, "nothing is deleted");
|
||||
|
||||
set_locked(&mut doc, 0, members[1], false).unwrap();
|
||||
delete_annotation(&mut doc, 0, members[0]).unwrap();
|
||||
assert_eq!(list(&doc, 0).len(), 1, "unlocked, the group goes; the fixture's Square stays");
|
||||
let mut alone = doc.clone();
|
||||
delete_annotation(&mut alone, 0, members[0]).unwrap();
|
||||
assert_eq!(list(&alone, 0).len(), before - 1, "the unlocked subordinate goes alone");
|
||||
delete_annotation(&mut doc, 0, top).unwrap();
|
||||
assert_eq!(list(&doc, 0).len(), before - 1, "the primary goes alone too");
|
||||
let locked = list(&doc, 0).iter().filter(|d| d.get(b"F").and_then(|f| f.as_int()).is_some_and(|f| f & 128 != 0)).count();
|
||||
assert_eq!(locked, 1, "the locked member stays");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -739,7 +739,7 @@ pub fn tools() -> Vec<ToolDef> {
|
||||
.ro().cmd("measure.snap").with(schema(json!({"doc":doc(),"page":{"type":"integer","minimum":1},"at":point(),"tolerance":{"type":"number","minimum":0,"maximum":10000},"endpoints":{"type":"boolean"},"midpoints":{"type":"boolean"},"intersections":{"type":"boolean"},"paths":{"type":"boolean"}}), &["doc","page","at"])),
|
||||
t("measure_export", "Export measurements as CSV", "Atomically write saved measurement values, labels, authors and scale ratios as spreadsheet-safe CSV. Returns how many unsupported measurements were left out.")
|
||||
.cmd("measure.export").with(schema(json!({"doc":doc(),"out":path_arg()}), &["doc","out"])),
|
||||
t("comment_list", "List comments", "Every comment (annotation other than links, form widgets and pop-ups) with its page, index, id, type, author, text, date, rectangle, colour, review status, checkmark and replies (each with its own status and checkmark). Annotations grouped with a comment (/RT /Group) are listed once, as that comment with its attributes, and named in its `group`.")
|
||||
t("comment_list", "List comments", "Every comment (annotation other than links, form widgets and pop-ups) with its page, index, id, type, author, text, date, rectangle, colour, review status, checkmark and replies (each with its own status and checkmark). Annotations grouped with a comment (/RT /Group) are listed once, as that comment with its attributes, and named in its `group`. comment_delete deletes just the member named, as Acrobat does (Replace Text's strikeout and caret go together).")
|
||||
.ro()
|
||||
.with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1, "description": "Only this page." } }), &["doc"])),
|
||||
t(
|
||||
|
||||
@@ -2382,11 +2382,17 @@ fn a_group_lists_as_its_primary() {
|
||||
check(&json!({ "count": 1, "comments": [primary] }), "edited");
|
||||
assert_eq!(after["count"], 2, "{after}");
|
||||
|
||||
// Deleting a member deletes the group (one undo step), as deleting the primary does.
|
||||
// Deleting a member deletes just it, as Acrobat does: the subordinate, or the primary, which
|
||||
// leaves the subordinate a comment of its own. One undo step each.
|
||||
ok(&mut a, "comment_delete", json!({ "doc": doc, "id": "circ-sub" }));
|
||||
let left = ok(&mut a, "comment_list", json!({ "doc": doc }));
|
||||
assert_eq!(left["count"], 1, "only the new note is left: {left}");
|
||||
assert!(!left.to_string().contains("sq-primary") && !left.to_string().contains("circ-sub"), "{left}");
|
||||
assert_eq!(left["count"], 2, "the primary and the new note are left: {left}");
|
||||
assert!(left.to_string().contains("sq-primary") && !left.to_string().contains("circ-sub"), "{left}");
|
||||
ok(&mut a, "edit_undo", json!({ "doc": doc }));
|
||||
ok(&mut a, "comment_delete", json!({ "doc": doc, "id": "sq-primary" }));
|
||||
let left = ok(&mut a, "comment_list", json!({ "doc": doc }));
|
||||
let ids: Vec<&str> = left["comments"].as_array().unwrap().iter().filter_map(|c| c["id"].as_str()).collect();
|
||||
assert!(ids.contains(&"circ-sub") && !ids.contains(&"sq-primary") && ids.len() == 2, "the new note and the Circle, on its own: {left}");
|
||||
ok(&mut a, "edit_undo", json!({ "doc": doc }));
|
||||
let back = ok(&mut a, "comment_list", json!({ "doc": doc }));
|
||||
let primary = back["comments"].as_array().unwrap().iter().find(|c| c["id"] == "sq-primary").cloned().unwrap();
|
||||
@@ -2426,12 +2432,18 @@ fn looping_groups_end_and_chained_groups_reach_their_primary() {
|
||||
let group: Vec<&str> = top["group"].as_array().unwrap().iter().filter_map(|m| m["id"].as_str()).collect();
|
||||
assert_eq!(group, ["mid", "low"], "{list}");
|
||||
assert_eq!(top["replies"], json!([]), "{list}");
|
||||
// Deleting a looping member ends too, and takes its loop with it.
|
||||
// Deleting a looping member ends too and takes just it, as any group member goes alone; so
|
||||
// does the lowest subordinate of the chain.
|
||||
ok(&mut a, "comment_delete", json!({ "doc": doc, "id": "a" }));
|
||||
ok(&mut a, "comment_delete", json!({ "doc": doc, "id": "self" }));
|
||||
ok(&mut a, "comment_delete", json!({ "doc": doc, "id": "low" }));
|
||||
let info = ok(&mut a, "doc_info", json!({ "doc": doc }));
|
||||
assert_eq!(ok(&mut a, "comment_list", json!({ "doc": doc }))["count"], 0, "{info}");
|
||||
let list = ok(&mut a, "comment_list", json!({ "doc": doc }));
|
||||
let mut ids: Vec<&str> = list["comments"].as_array().unwrap().iter().filter_map(|c| c["id"].as_str()).collect();
|
||||
ids.sort_unstable();
|
||||
assert_eq!(ids, ["b", "top"], "{list}");
|
||||
let top = list["comments"].as_array().unwrap().iter().find(|c| c["id"] == "top").cloned().unwrap_or_default();
|
||||
let group: Vec<&str> = top["group"].as_array().into_iter().flatten().filter_map(|m| m["id"].as_str()).collect();
|
||||
assert_eq!(group, ["mid"], "{list}");
|
||||
}
|
||||
|
||||
// Contributor-original PDF: a Replace Text group as earlier PdfCraft builds wrote it, the
|
||||
@@ -2495,6 +2507,15 @@ fn replace_text_lists_its_replacement() {
|
||||
assert_eq!(list["count"], 2, "{path}: {list}");
|
||||
let c = list["comments"].as_array().unwrap().iter().find(|c| c["type"] == primary).cloned().unwrap();
|
||||
assert_eq!(c["contents"], text, "{path} after an edit: {list}");
|
||||
// Replace Text is one comment: deleting either half deletes both, as Acrobat does, in
|
||||
// either orientation.
|
||||
for id in [c["id"].clone(), c["group"][0]["id"].clone()] {
|
||||
ok(&mut b, "comment_delete", json!({ "doc": doc, "id": id }));
|
||||
let left = ok(&mut b, "comment_list", json!({ "doc": doc }));
|
||||
assert_eq!(left["count"], 1, "{path}: deleting {id} leaves only the note: {left}");
|
||||
assert!(!left.to_string().contains("StrikeOut") && !left.to_string().contains("Caret"), "{path}: {left}");
|
||||
ok(&mut b, "edit_undo", json!({ "doc": doc }));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -752,7 +752,7 @@ pub fn clear_marks(doc: &mut Document, pages: Option<&[usize]>) -> Result<usize,
|
||||
let doomed = pdfcraft_annot::with_replies_and_popups(doc, &listed, targets.iter().copied());
|
||||
for p in pdfcraft_model::pages(doc) {
|
||||
let annots = annots_of(doc, &p.dict);
|
||||
let kept: Vec<Object> = annots.iter().filter(|a| !pdfcraft_annot::goes_with(a, &doomed)).cloned().collect();
|
||||
let kept: Vec<Object> = annots.iter().filter(|a| !pdfcraft_annot::goes_with(doc, a, &doomed)).cloned().collect();
|
||||
// As deleting a comment does: a page whose marks were all it had loses `/Annots` again,
|
||||
// and an indirect array stays the one object the page points to.
|
||||
if kept.len() != annots.len() {
|
||||
|
||||
@@ -33,6 +33,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>,
|
||||
cv.context_at = Some((cx.page, cx.to_user(p)));
|
||||
// On a group subordinate, its group (the primary), so the menu offers the comment's actions.
|
||||
cv.selected = cx.click_hit(p).map(|a| (cx.page, a.index));
|
||||
cv.member = cx.picked(p, cv.selected);
|
||||
}
|
||||
let gestures = Gestures { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page };
|
||||
match cx.tool {
|
||||
@@ -264,6 +265,7 @@ fn select_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mu
|
||||
match owner {
|
||||
Some((a, _)) => {
|
||||
cv.selected = Some((cx.page, a.index));
|
||||
cv.member = pointer.and_then(|p| cx.picked(p, cv.selected));
|
||||
cv.reveal = true;
|
||||
consumed = true;
|
||||
}
|
||||
@@ -599,7 +601,7 @@ pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool
|
||||
}
|
||||
}
|
||||
if allowed
|
||||
&& let Some((page, index)) = cv.selected
|
||||
&& let Some((page, index)) = cv.target()
|
||||
&& ctx.input(|i| i.key_pressed(egui::Key::Delete) || i.key_pressed(egui::Key::Backspace))
|
||||
{
|
||||
cv.selected = None;
|
||||
|
||||
@@ -92,10 +92,12 @@ pub(crate) fn context_menu(
|
||||
ui.close();
|
||||
}
|
||||
ui.separator();
|
||||
// The engine's own check: a locked comment, or one with a locked reply, stays.
|
||||
if ui.add_enabled(allowed && deletable(page, index), egui::Button::new(tl!("Delete"))).clicked() {
|
||||
// The engine's own check: a locked comment, or one with a locked reply, stays. On a group
|
||||
// subordinate, Delete takes just it, as Acrobat's does.
|
||||
let (dpage, dindex) = view.comments.target().unwrap_or((page, index));
|
||||
if ui.add_enabled(allowed && deletable(dpage, dindex), egui::Button::new(tl!("Delete"))).clicked() {
|
||||
view.comments.selected = None;
|
||||
action = Some(CanvasAction::Edit(Box::new(Edit::DeleteAnnotation { page, index })));
|
||||
action = Some(CanvasAction::Edit(Box::new(Edit::DeleteAnnotation { page: dpage, index: dindex })));
|
||||
ui.close();
|
||||
}
|
||||
if ui.add_enabled(allowed, egui::Button::new(tl!("Properties…"))).clicked() {
|
||||
|
||||
@@ -126,6 +126,10 @@ impl CommentView {
|
||||
pub struct CommentView {
|
||||
/// The selected comment: (page, index in `/Annots`).
|
||||
pub selected: Option<(usize, usize)>,
|
||||
/// The group subordinate a click on the page picked, with the selection (its group's primary)
|
||||
/// it was picked under: Delete acts on it while that group stays selected, as Acrobat deletes
|
||||
/// the member clicked ([`Self::target`]).
|
||||
pub(crate) member: Option<((usize, usize), (usize, usize))>,
|
||||
pub gesture: Option<Gesture>,
|
||||
/// Drawings just released: shown from their points until the page raster that includes the
|
||||
/// committed annotation arrives, so the stroke never vanishes in between (#1858).
|
||||
@@ -186,6 +190,15 @@ pub(crate) enum EditorFocus {
|
||||
}
|
||||
|
||||
impl CommentView {
|
||||
/// What Delete acts on: the group member clicked, while its group is still the selection,
|
||||
/// else the selected comment.
|
||||
pub(crate) fn target(&self) -> Option<(usize, usize)> {
|
||||
match self.member {
|
||||
Some((member, group)) if Some(group) == self.selected => Some(member),
|
||||
_ => self.selected,
|
||||
}
|
||||
}
|
||||
|
||||
/// Takes back the last point of a polygon, cloud or connected lines being drawn; the first
|
||||
/// point's removal cancels the drawing. Returns whether there was such a drawing.
|
||||
pub fn undo_point(&mut self) -> bool {
|
||||
@@ -410,8 +423,20 @@ impl PageCx<'_> {
|
||||
/// The comment a click at `p` selects: the topmost one under it or, on a group subordinate,
|
||||
/// the group's primary, which stands for the group.
|
||||
pub(super) fn click_hit(&self, p: Pos2) -> Option<&Annotation> {
|
||||
self.hit(p)
|
||||
.or_else(|| self.group_members().into_iter().rev().find(|(m, _)| self.screen_rects(m).any(|r| r.expand(3.0).contains(p))).map(|(_, p)| p))
|
||||
self.hit(p).or_else(|| self.member_at(p).map(|(_, p)| p))
|
||||
}
|
||||
|
||||
/// The group subordinate under `p`, where no comment is, with its group's primary.
|
||||
fn member_at(&self, p: Pos2) -> Option<(&Annotation, &Annotation)> {
|
||||
self.group_members().into_iter().rev().find(|(m, _)| self.screen_rects(m).any(|r| r.expand(3.0).contains(p)))
|
||||
}
|
||||
|
||||
/// The group member a click at `p` picked when it selects `selected`, its group's primary
|
||||
/// ([`CommentView::member`]): the subordinate under the pointer.
|
||||
pub(super) fn picked(&self, p: Pos2, selected: Option<(usize, usize)>) -> Option<((usize, usize), (usize, usize))> {
|
||||
let (m, primary) = self.hit(p).is_none().then(|| self.member_at(p)).flatten()?;
|
||||
let group = (primary.page, primary.index);
|
||||
(Some(group) == selected).then_some(((self.page, m.index), group))
|
||||
}
|
||||
|
||||
pub(crate) fn rect_to_user(&self, r: Rect) -> [f64; 4] {
|
||||
|
||||
@@ -3079,3 +3079,31 @@ fn a_group_is_one_comment_in_the_panel_and_on_the_page() {
|
||||
let composer = h.state().views[0].comments.composer.as_ref().map(|c| c.text.clone());
|
||||
assert_eq!(composer.as_deref(), Some("Primary"), "a double-click on the Circle edits its group's text");
|
||||
}
|
||||
|
||||
/// As in Acrobat desktop, Delete deletes the group member clicked and keeps the rest of its
|
||||
/// group, although ISO 32000-2 §12.5.6.2 lists deletion among the operations that act on the
|
||||
/// entire group. The Circle goes with its reply and the Square stays; the Square goes with its
|
||||
/// reply and the Circle stays, a comment of its own.
|
||||
#[test]
|
||||
fn delete_deletes_the_group_member_clicked() {
|
||||
let shapes = |h: &Harness<'static, PdfCraftApp>| -> Vec<(String, bool)> {
|
||||
let mut out: Vec<(String, bool)> = comments(h).iter().filter(|a| a.subtype != "Text").map(|a| (a.subtype.clone(), a.is_comment())).collect();
|
||||
out.sort();
|
||||
out
|
||||
};
|
||||
let mut h = harness_with_bytes(GROUP_FIXTURE.to_vec(), |_| {});
|
||||
click_pt(&mut h, (220.0, 150.0));
|
||||
assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the group is selected");
|
||||
h.key_press(egui::Key::Delete);
|
||||
h.run_steps(3);
|
||||
assert_eq!(shapes(&h), [("Square".to_string(), true)], "the Circle goes, the Square stays");
|
||||
let texts: Vec<String> = comments(&h).iter().filter_map(|a| a.contents.clone()).collect();
|
||||
assert!(!texts.iter().any(|c| c == "Reply to the subordinate") && texts.iter().any(|c| c == "Plain reply"), "{texts:?}");
|
||||
h.state_mut().undo();
|
||||
h.run_steps(3);
|
||||
|
||||
click_pt(&mut h, (60.0, 50.0));
|
||||
h.key_press(egui::Key::Delete);
|
||||
h.run_steps(3);
|
||||
assert_eq!(shapes(&h), [("Circle".to_string(), true)], "the Square goes, the Circle stays as a comment of its own");
|
||||
}
|
||||
@@ -3596,7 +3596,7 @@ 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", "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", "crates/automation/tests/it/automation.rs::replies_thread_by_reference_when_ids_repeat_across_pages", "crates/ui-egui/tests/it/comments.rs::a_group_is_one_comment_in_the_panel_and_on_the_page", "crates/automation/tests/it/automation.rs::a_group_lists_as_its_primary", "crates/automation/tests/it/automation.rs::a_group_member_on_another_page_is_a_comment_of_its_own", "crates/automation/tests/it/automation.rs::looping_groups_end_and_chained_groups_reach_their_primary", "crates/render/src/inspect/tests.rs::group_primaries_follow_irt_to_the_first_non_subordinate"]
|
||||
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. Annotation groups (`/RT /Group`, ISO 32000-2 §12.5.6.2) list as one comment with the primary's author, text and colour (the subordinate's own are ignored), a reply to a subordinate joins the primary's thread, `comment_list` names the members in `group`, clicking a subordinate on the page selects the group, and deleting any member deletes its group on the page (one undo step), as ISO 32000-2 §12.5.6.2 has it and PDF-XChange and Foxit do (Acrobat desktop deletes only the member clicked). A group is one page's: a subordinate whose primary is on another page is a comment of its own, listed, selected and deleted on its own, and it stays when the primary is deleted, as Acrobat (desktop and iPad) and PDF-XChange read it. Missing: moving or restyling a member does not move or restyle the group, and a subordinate has no hover tooltip"
|
||||
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. Annotation groups (`/RT /Group`, ISO 32000-2 §12.5.6.2) list as one comment with the primary's author, text and colour (the subordinate's own are ignored), a reply to a subordinate joins the primary's thread, `comment_list` names the members in `group`, clicking a subordinate on the page selects the group, and Delete deletes the member clicked alone, the primary included, as Acrobat desktop does (one undo step; a subordinate left without its primary is a comment of its own; Replace Text's strikeout and caret delete together). ISO 32000-2 §12.5.6.2 has deletion act on the whole group, as PDF-XChange and Foxit do on one page; Acrobat's is followed as the more practical. A group is one page's: a subordinate whose primary is on another page is a comment of its own, listed, selected, moved and deleted on its own, as Acrobat (desktop and iPad), PDF-XChange and Foxit read it. Missing: restyling a member does not restyle its group as Acrobat does, a subordinate can't be dragged, resized or reshaped on its own, and it has no hover tooltip"
|
||||
|
||||
[[feature]]
|
||||
id = "comment.appearance-generation"
|
||||
|
||||
Reference in new issue
Block a user