From d1e1357775b18550a8c32f11e4dc144e8166ad4f Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:05:22 +0800 Subject: [PATCH 01/19] M5: comment pop-up windows on the page, with connector lines A double-click on a comment opens its pop-up window as in Acrobat: author, date, the text edited in place, the replies; the title bar drags it, x closes it, each an undo step saved in /Popup (/Open, /Rect) and a note's own /Open. A connector line in the comment's colour joins the comment to its open window on roll-over or selection. comment_popup does the same headlessly; comment_list reports the pop-up. Three undo labels in 22 catalogues (Vietnamese included). Ported onto main's split modules: set_popup lives in annot's modify.rs and PopupInfo in info.rs; the pop-up windows are their own module, ui-egui tools/comments/popups.rs, listed with the page modules that read the pointer and set the cursor themselves; the hover tip filter is in comments::describe. PageCx's pointer and redaction look, the tests/it evidence paths and the Annotation test fixtures follow main (formerly fcd33e131, c68e5d902, 0a7fb530b, 8af2c338e, 345c1573d), and labels are logical text now that egui shapes bidi itself. Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- ROADMAP.md | 1 + crates/annot/README.md | 4 +- crates/annot/src/info.rs | 24 +++ crates/annot/src/lib.rs | 4 +- crates/annot/src/modify.rs | 73 ++++++- crates/annot/src/tests.rs | 45 ++++ crates/automation/src/comments.rs | 13 ++ crates/automation/src/dispatch.rs | 1 + crates/automation/src/tools.rs | 5 + crates/automation/tests/it/automation.rs | 26 +++ crates/engine/src/comments.rs | 1 + crates/engine/src/edit.rs | 8 + crates/engine/src/edit/labels.rs | 6 + crates/engine/src/edit/run.rs | 2 + crates/engine/src/history.rs | 1 + crates/render/src/inspect.rs | 2 +- crates/render/src/inspect/annotations.rs | 10 +- crates/render/src/inspect/model.rs | 11 + crates/render/src/inspect/tests.rs | 13 ++ crates/render/src/lib.rs | 2 +- crates/ui-egui/src/canvas.rs | 5 +- .../ui-egui/src/canvas/page_pointer/tests.rs | 13 +- crates/ui-egui/src/canvas/xform.rs | 5 + crates/ui-egui/src/editing.rs | 1 + crates/ui-egui/src/i18n/ar.tsv | 3 + crates/ui-egui/src/i18n/bg.tsv | 3 + crates/ui-egui/src/i18n/cs.tsv | 3 + crates/ui-egui/src/i18n/de.tsv | 3 + crates/ui-egui/src/i18n/el.tsv | 3 + crates/ui-egui/src/i18n/es.tsv | 3 + crates/ui-egui/src/i18n/fr.tsv | 3 + crates/ui-egui/src/i18n/hu.tsv | 3 + crates/ui-egui/src/i18n/it.tsv | 3 + crates/ui-egui/src/i18n/ja.tsv | 3 + crates/ui-egui/src/i18n/kk.tsv | 3 + crates/ui-egui/src/i18n/lt.tsv | 3 + crates/ui-egui/src/i18n/pl.tsv | 3 + crates/ui-egui/src/i18n/pt-br.tsv | 3 + crates/ui-egui/src/i18n/ru.tsv | 3 + crates/ui-egui/src/i18n/sl.tsv | 3 + crates/ui-egui/src/i18n/te.tsv | 3 + crates/ui-egui/src/i18n/tr.tsv | 3 + crates/ui-egui/src/i18n/uk.tsv | 3 + crates/ui-egui/src/i18n/vi.tsv | 5 + crates/ui-egui/src/i18n/zh-hans.tsv | 3 + crates/ui-egui/src/i18n/zh-hant.tsv | 3 + crates/ui-egui/src/tools/comments.rs | 2 + crates/ui-egui/src/tools/comments/input.rs | 13 +- crates/ui-egui/src/tools/comments/paint.rs | 2 + crates/ui-egui/src/tools/comments/popups.rs | 196 ++++++++++++++++++ crates/ui-egui/src/tools/comments/state.rs | 9 + crates/ui-egui/src/tools/comments/tests.rs | 4 + crates/ui-egui/tests/it/comments.rs | 65 ++++++ parity/acrobat-features.toml | 14 +- 54 files changed, 629 insertions(+), 20 deletions(-) create mode 100644 crates/ui-egui/src/tools/comments/popups.rs diff --git a/ROADMAP.md b/ROADMAP.md index 14419f9857..0854b65f96 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -432,6 +432,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-15 (M3, #1248):** A fresh install follows the desktop's appearance: the theme preference defaults to System (dark on a dark desktop, Light where the desktop says nothing); saved settings and `--theme` still win. Dark page rendering is Reading colours, and the page reading themes are #1732. - **2026-10-15 (M3, #1767):** Restore last view settings when reopening documents, as Acrobat's preference does: a file opened again shows the page, zoom and page display it was last shown with (remembered for the last 100 files when a tab closes or settings are saved), over the file's own initial view; on by default, Preferences ▸ Documents and view turns it off. +- **2026-10-14 (M5, comment pop-ups):** A double-click on a comment opens its pop-up window on the page, as in Acrobat: author, date, the text edited in place, the replies; the title bar drags it and × closes it, each an undo step saved in the file's `/Popup` so other viewers agree. A connector line in the comment's colour joins the comment to its open window on roll-over or selection. `comment_popup` does the same headlessly and `comment_list` reports the pop-up. - **2026-10-13 (M3, native-opening candidate):** Chosen tools survive preparation, conversion and password retries; startup intake waits for pending preparation. Regression tests cover completion, cancellation and document identity. Estimates unchanged. - **2026-10-13 (community, M3, #531 touchpad momentum):** On Wayland a two-finger swipe keeps gliding after the fingers lift and slows to a stop, measured from the gesture's last 0.1 s and decaying with a 0.35 s time constant through the #759 1:1 path. Any wheel input, click, key, zoom or edge ends it (fingers resting on the touchpad don't yet: the vendored winit binds no Wayland hold gestures); single-page paging, macOS (which sends its own momentum), the web, and Animation effects Off never glide. Unit tests for the physics and two headless view tests. Estimates unchanged. - **2026-10-13 (M3, native-open admission):** Later prepared replies stay queued while a modal or native picker owns the foreground. Picker completion, cancellation and abandonment wake retained opens. Regressions cover recipient ownership, exactly-once admission, source/history preservation and one-shot repaint requests. Estimates unchanged. diff --git a/crates/annot/README.md b/crates/annot/README.md index 999baec20a..48f137b07c 100644 --- a/crates/annot/README.md +++ b/crates/annot/README.md @@ -21,7 +21,9 @@ Comments (annotations), ISO 32000-2 §12.5. Layer L3, depends on `pdfcraft-cos`, comment another program drew with a font that has those characters, whose appearance is kept rather than redrawn with `?` (new text for it is taken). - **Edits:** reply (`/IRT`), set review status (a `/State` reply, as Acrobat's "Set status" - does), change text, move, resize, restyle, delete (with pop-up and replies). + does), change text, move, resize, restyle, delete (with pop-up and replies), and open, close + or move the pop-up window (`set_popup`: `/Popup` `/Open` and `/Rect`, created beside a comment + that has none; a note's own `/Open` follows). `summaries` report each comment's pop-up. ## API sketch diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index b56e0de20d..bb4cc238a0 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -38,6 +38,29 @@ pub struct Summary { /// A line's two ends (`/L`), or the corners of connected lines, a polygon or a cloud /// (`/Vertices`), in user space; empty for other comments. pub points: Vec<[f32; 2]>, + /// The comment's pop-up window (`/Popup`), when it has one. + pub popup: Option, +} + +/// A comment's pop-up window (ISO 32000-2 §12.5.6.14): where it sits on the page and whether it +/// is shown open. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct PopupInfo { + /// `/Rect` of the `/Popup` annotation, in user space. + pub rect: [f32; 4], + /// `/Open` of the `/Popup` annotation. + pub open: bool, +} + +/// The pop-up `annot` names, as [`PopupInfo`]: `None` when it has none or it is unreadable. +fn popup_info(doc: &Document, annot: &Dict) -> Option { + let r = annot.reference(b"Popup")?; + let p = doc.get(r); + let p = p.as_dict()?; + let rect: Vec = doc.resolve(p.get(b"Rect")?).as_array()?.iter().filter_map(|v| doc.resolve(v).as_f64().map(|x| x as f32)).collect(); + let [x0, y0, x1, y1] = rect[..] else { return None }; + let open = p.get(b"Open").map(|o| doc.resolve(o)).is_some_and(|o| matches!(&*o, Object::Bool(true))); + Some(PopupInfo { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open }) } pub(crate) fn text_value(doc: &Document, d: &Dict, key: &[u8]) -> Option { @@ -145,6 +168,7 @@ pub fn summaries(doc: &Document) -> Vec { fill_sign: is_fill_sign(doc, d), no_rotate: drawn_upright(doc, d), points, + popup: popup_info(doc, d), }); } } diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 7bd0894c66..017ee3c802 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -29,7 +29,7 @@ mod types; use create::num_array; pub use create::{add_annotation, callout_attach, orient_image_stamp}; pub use draw::{AppearanceCache, set_appearance, with_missing_appearances, with_missing_appearances_cached}; -pub use info::{Props, Summary, is_comment_subtype, props, set_info, summaries}; +pub use info::{PopupInfo, Props, Summary, is_comment_subtype, props, set_info, summaries}; #[cfg(test)] use lookup::{ANNOTS_ENTRIES_COPIED, PageWalk, annot_dict, annot_ref}; use lookup::{annots, page_ref}; @@ -39,7 +39,7 @@ pub use lookup::{ pub(crate) use modify::n; pub use modify::{ add_reply, add_text_replacement, deletable, delete_annotation, erase_ink, fillable, goes_with, lift_annotation, move_annotation, set_contents, - set_fill, set_font_size, set_locked, set_marked, set_points, set_rect, set_review_state, set_style, with_replies_and_popups, + set_fill, set_font_size, set_locked, set_marked, set_points, set_popup, set_rect, set_review_state, set_style, with_replies_and_popups, }; pub use types::{ AttachIcon, FillMark, LineEnding, Markup, Meta, NOTE_SIZE, NewAnnotation, NoteIcon, OverlayFont, OverlayLook, ReviewState, Rgb, Shape, diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index 2e3ae0b7d0..126897e509 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -2,8 +2,8 @@ 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::{line_endings_of, text_value}; -use super::lookup::{annot_dict, annot_ref, annotation_flags, annots, page_ref, set_annots}; +use super::info::{is_comment_subtype, line_endings_of, text_value}; +use super::lookup::{annot_dict, annot_ref, annotation_flags, annots, is_fill_sign, 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, StyleChange, TextReplacement, @@ -286,6 +286,75 @@ pub(crate) fn unlocked(doc: &Document, r: ObjRef) -> Result<(), AnnotError> { if flags(doc, r) & FLAG_LOCKED != 0 { Err(AnnotError::Invalid("the comment is locked".into())) } else { Ok(()) } } +/// Open, close or move a comment's pop-up window (Acrobat: double-click a comment to open it, +/// drag its title bar to move it). `rect` is the pop-up's new `/Rect` in user space; `open` its +/// `/Open`. A comment without a `/Popup` gets one, placed to the right of the comment as new +/// notes get theirs. A text box, a Fill & Sign mark or a reply has no pop-up of its own (a reply +/// shows in its parent's). The lock does not apply: the window's place and state are not the +/// comment. The modification date is left alone: nothing of the comment changed. +pub fn set_popup(doc: &mut Document, page: usize, index: usize, rect: Option<[f64; 4]>, open: Option, meta: &Meta) -> Result<(), AnnotError> { + let (p, r) = annot_ref(doc, page, index)?; + let d = annot_dict(doc, r); + let subtype = String::from_utf8_lossy(d.name(b"Subtype").unwrap_or_default()).into_owned(); + if !is_comment_subtype(&subtype) || subtype == "FreeText" || is_fill_sign(doc, &d) || d.contains(b"IRT") { + return Err(AnnotError::Invalid(format!("{subtype} comments have no pop-up of their own"))); + } + let rect = match rect { + Some(rect) => { + let rect = normalize(rect); + if !finite(&rect) || rect[2] - rect[0] < 1.0 || rect[3] - rect[1] < 1.0 { + return Err(AnnotError::Invalid("invalid pop-up rectangle".into())); + } + Some(rect) + } + None => None, + }; + let _ = meta; + match d.reference(b"Popup").filter(|pr| doc.get(*pr).as_dict().is_some()) { + Some(pr) => doc.update_dict(pr, |p| { + if let Some(rect) = rect { + p.set(b"Rect".to_vec(), num_array(&rect)); + } + if let Some(open) = open { + p.set(b"Open".to_vec(), Object::Bool(open)); + } + })?, + None => { + let own: Vec = d + .get(b"Rect") + .map(|o| doc.resolve(o)) + .and_then(|o| o.as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(|v| doc.resolve(v).as_f64()) + .collect(); + let beside = match own[..] { + [_, _, x1, y1] if finite(&own) => [x1 + 10.0, y1 - 120.0, x1 + 210.0, y1], + _ => [0.0, 0.0, 200.0, 120.0], + }; + let mut pd = Dict::new(); + pd.set(b"Type".to_vec(), Object::name("Annot")); + pd.set(b"Subtype".to_vec(), Object::name("Popup")); + pd.set(b"Rect".to_vec(), num_array(&rect.unwrap_or(beside))); + pd.set(b"Parent".to_vec(), Object::Ref(r)); + pd.set(b"Open".to_vec(), Object::Bool(open.unwrap_or(false))); + pd.set(b"F".to_vec(), Object::Int(FLAG_PRINT | FLAG_NO_ZOOM | FLAG_NO_ROTATE)); + let pref = doc.add(Object::Dict(pd)); + doc.update_dict(r, |d| d.set(b"Popup".to_vec(), Object::Ref(pref)))?; + let mut list = annots(doc, p); + list.push(Object::Ref(pref)); + set_annots(doc, p, list)?; + } + } + // A note's own `/Open` says the same thing to viewers that read it there (§12.5.6.4). + if let Some(open) = open + && (subtype == "Text" || d.contains(b"Open")) + { + doc.update_dict(r, |d| d.set(b"Open".to_vec(), Object::Bool(open)))?; + } + Ok(()) +} + /// Lock or unlock a comment (the Locked flag). A locked comment can't be moved, resized, /// restyled or deleted; its text and replies stay editable, as in Acrobat. pub fn set_locked(doc: &mut Document, page: usize, index: usize, locked: bool) -> Result<(), AnnotError> { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index a3dc14e262..19973781c8 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2194,3 +2194,48 @@ fn degenerate_and_absurd_leader_and_caption_values_do_not_panic() { assert!(long.contains(&format!("({}) Tj", "w".repeat(256))), "cut to 256"); assert!(!long.contains(&"w".repeat(257))); } + +/// A pop-up is opened, closed and moved through `set_popup` (#comment.popups): a note's own +/// pop-up follows, a comment without one gets one beside it, and the comment itself (its text, +/// its date) is left alone. Summaries report the pop-up. Text boxes, Fill & Sign marks and +/// replies have none of their own; a bad rectangle is refused. +#[test] +fn pop_ups_open_close_and_move_and_are_reported() { + let mut doc = fixture(); + let note = add_annotation(&mut doc, &new(0, Shape::Note { at: [20.0, 280.0], icon: NoteIcon::Comment }), &meta("n1")).unwrap(); + let before = summaries(&doc).into_iter().find(|s| s.index == note).unwrap(); + assert_eq!(before.popup.map(|p| p.open), Some(false), "new notes have a closed pop-up"); + set_popup(&mut doc, 0, note, None, Some(true), &meta("n1")).unwrap(); + let after = summaries(&doc).into_iter().find(|s| s.index == note).unwrap(); + assert_eq!(after.popup.map(|p| p.open), Some(true)); + assert_eq!(after.popup.map(|p| p.rect), before.popup.map(|p| p.rect), "opening does not move it"); + assert_eq!((after.modified, after.contents.as_deref()), (before.modified, Some("hello")), "the comment is untouched"); + let (_, r) = annot_ref(&mut doc, 0, note).unwrap(); + assert_eq!(annot_dict(&doc, r).get(b"Open"), Some(&Object::Bool(true)), "the note's own /Open follows"); + set_popup(&mut doc, 0, note, Some([150.0, 100.0, 290.0, 200.0]), Some(false), &meta("n1")).unwrap(); + let moved = summaries(&doc).into_iter().find(|s| s.index == note).unwrap(); + assert_eq!(moved.popup, Some(PopupInfo { rect: [150.0, 100.0, 290.0, 200.0], open: false })); + assert_eq!(annot_dict(&doc, r).get(b"Open"), Some(&Object::Bool(false))); + + // A highlight is created without a pop-up: the first open gives it one, beside it. + let mark = add_annotation( + &mut doc, + &new(0, Shape::TextMarkup { kind: Markup::Highlight, quads: vec![[20.0, 200.0, 120.0, 200.0, 20.0, 190.0, 120.0, 190.0]] }), + &meta("h1"), + ) + .unwrap(); + assert_eq!(summaries(&doc).into_iter().find(|s| s.index == mark).unwrap().popup, None); + set_popup(&mut doc, 0, mark, None, Some(true), &meta("h1")).unwrap(); + let given = summaries(&doc).into_iter().find(|s| s.index == mark).unwrap().popup.expect("a pop-up now"); + assert!(given.open); + assert!((given.rect[0] - 130.0).abs() < 0.01 && (given.rect[2] - 330.0).abs() < 0.01, "beside the mark: {:?}", given.rect); + + // None of their own. + let text_box = add_annotation(&mut doc, &new(0, Shape::TextBox { rect: [10.0, 10.0, 110.0, 40.0], font_size: 12.0 }), &meta("t1")).unwrap(); + assert!(matches!(set_popup(&mut doc, 0, text_box, None, Some(true), &meta("t1")), Err(AnnotError::Invalid(_)))); + let reply = add_reply(&mut doc, 0, note, "Agreed", "Ada", &meta("r1")).unwrap(); + assert!(matches!(set_popup(&mut doc, 0, reply, None, Some(true), &meta("r1")), Err(AnnotError::Invalid(_)))); + assert!(matches!(set_popup(&mut doc, 0, note, Some([0.0, 0.0, 0.5, 0.5]), None, &meta("n1")), Err(AnnotError::Invalid(_)))); + assert!(matches!(set_popup(&mut doc, 0, note, Some([f64::NAN, 0.0, 10.0, 10.0]), None, &meta("n1")), Err(AnnotError::Invalid(_)))); + assert!(matches!(set_popup(&mut doc, 0, 99, None, Some(true), &meta("n1")), Err(AnnotError::NoSuchAnnotation { .. }))); +} diff --git a/crates/automation/src/comments.rs b/crates/automation/src/comments.rs index f87fd0b58e..9f7fd4a86f 100644 --- a/crates/automation/src/comments.rs +++ b/crates/automation/src/comments.rs @@ -188,6 +188,8 @@ impl Automation { "modified": c.modified, "rect": crate::view_rect(&doc.info, c.page, c.shown_rect(doc.info.pages.get(c.page).map_or(0, |p| p.rotation))), "color": c.color.map(hex), + // Unchanged on a page the viewer could not read, as `rect` is. + "popup": c.popup.map(|w| json!({ "rect": crate::view_rect(&doc.info, c.page, w.rect), "open": w.open })), }); // A line's ends, or the corners of connected lines, a polygon or a cloud, as // comment_edit's `points` takes them. @@ -611,6 +613,17 @@ impl Automation { self.apply(a, Edit::MarkAnnotation { page, index, marked, author }) } + pub(crate) fn comment_popup(&mut self, a: &Args) -> Result { + let (page, index) = self.comment_target(a)?; + let info = self.doc(a)?.info.pages[page].clone(); + let open = a.opt_bool("open")?; + let rect = a.nums::<4>("rect")?.map(|r| rect_to_user(&info, r)); + if open.is_none() && rect.is_none() { + return Err(ToolError::InvalidArgs("nothing to change: pass open and/or rect".into())); + } + self.apply(a, Edit::SetAnnotationPopup { page, index, rect, open }) + } + pub(crate) fn comment_lock(&mut self, a: &Args) -> Result { let (page, index) = self.comment_target(a)?; let locked = a.opt_bool("locked")?.unwrap_or(true); diff --git a/crates/automation/src/dispatch.rs b/crates/automation/src/dispatch.rs index 36e0d00454..4cd6e17643 100644 --- a/crates/automation/src/dispatch.rs +++ b/crates/automation/src/dispatch.rs @@ -805,6 +805,7 @@ impl Automation { "sign_trust" => self.sign_trust(&a)?, "comment_mark" => self.comment_mark(&a)?, "comment_lock" => self.comment_lock(&a)?, + "comment_popup" => self.comment_popup(&a)?, "comments_hide" => self.comments_hide(&a)?, "layer_set" => self.layer_set(&a)?, "view_reading_colors" => self.view_reading_colors(&a)?, diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index c6c200cece..fe4f699d1a 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -923,6 +923,11 @@ pub fn tools() -> Vec { t("comment_image_preview", "Preview image signature layers", "Return a PNG of the page without the selected image signature/initials (layer=background, default), or its embedded image with alpha (layer=image). Includes the displayed rectangle, document rotation, image_rotation (how far clockwise the image is shown turned) and annotation opacity. Cache these layers for live placement/resizing; commit once with comment_edit. Does not change the document or undo history.") .ro().with(schema(comment_ref(json!({ "layer": { "type": "string", "enum": ["background", "image"], "default": "background" }, "dpi": { "type": "number", "minimum": 1, "maximum": 600, "default": 96 } })), &["doc"])), t("comment_delete", "Delete a comment", "Delete a comment with its pop-up and replies, or one reply (its id, or page and index, from comment_list) with its pop-up and the replies to it; the comment and its other replies stay. Undoable.").destructive().with(schema(comment_ref(json!({})), &["doc"])), + t("comment_popup", "Open, close or move a comment's pop-up", "Open or close a comment's pop-up window (open), or move it (rect, in page view coordinates as comment_list reports). A comment without a pop-up gets one beside it. Text boxes, Fill & Sign marks and replies have none of their own. Undoable.") + .with(schema(comment_ref(json!({ + "open": { "type": "boolean" }, + "rect": { "type": "array", "items": { "type": "number" }, "minItems": 4, "maxItems": 4 }, + })), &["doc"])), t( "doc_protect", "Protect with passwords", diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6a5765b911..ee868206b3 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -10190,3 +10190,29 @@ fn simple_rule_scan_drawn(content: &[u8]) -> Vec { } write_full(&doc, &SaveOptions::default()).unwrap() } + +/// comment_popup opens, closes and moves a comment's pop-up, comment_list reports it, and each +/// change is one undo step with its own label. +#[test] +fn comment_popups_are_opened_moved_and_listed() { + let dir = workdir("popup"); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); + ok(&mut a, "comment_add", json!({ "doc": doc, "page": 1, "type": "note", "at": [30.0, 40.0], "contents": "Look here" })); + let listed = ok(&mut a, "comment_list", json!({ "doc": doc })); + let note = &listed["comments"][0]; + assert_eq!(note["popup"]["open"], json!(false), "{listed}"); + let id = note["id"].as_str().unwrap().to_string(); + let opened = ok(&mut a, "comment_popup", json!({ "doc": doc, "id": id, "open": true })); + assert_eq!(opened["undo"], json!("Open comment pop-up"), "{opened}"); + let listed = ok(&mut a, "comment_list", json!({ "doc": doc })); + assert_eq!(listed["comments"][0]["popup"]["open"], json!(true)); + let moved = ok(&mut a, "comment_popup", json!({ "doc": doc, "id": id, "rect": [60.0, 60.0, 180.0, 140.0] })); + assert_eq!(moved["undo"], json!("Move comment pop-up")); + let listed = ok(&mut a, "comment_list", json!({ "doc": doc })); + assert_eq!(listed["comments"][0]["popup"]["rect"], json!([60.0, 60.0, 180.0, 140.0]), "{listed}"); + let closed = ok(&mut a, "comment_popup", json!({ "doc": doc, "id": id, "open": false })); + assert_eq!(closed["undo"], json!("Close comment pop-up")); + assert!(matches!(err(&mut a, "comment_popup", json!({ "doc": doc, "id": id })), ToolError::InvalidArgs(_))); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/engine/src/comments.rs b/crates/engine/src/comments.rs index eee799ed2f..43ab3b6b83 100644 --- a/crates/engine/src/comments.rs +++ b/crates/engine/src/comments.rs @@ -24,6 +24,7 @@ pub(crate) fn comment_list(doc: &pdfcraft_cos::Document) -> Vec, }, + /// Open, close or move a comment's pop-up window (`rect` in user space; a comment without a + /// pop-up gets one). + SetAnnotationPopup { + page: usize, + index: usize, + rect: Option<[f64; 4]>, + open: Option, + }, /// Resize a rectangle, oval or text box. ResizeAnnotation { page: usize, diff --git a/crates/engine/src/edit/labels.rs b/crates/engine/src/edit/labels.rs index c3339453af..fbfc33d2f2 100644 --- a/crates/engine/src/edit/labels.rs +++ b/crates/engine/src/edit/labels.rs @@ -84,6 +84,9 @@ pub const EDIT_LABELS: &[&str] = &[ "Unlock comment", "Move comment", "Reshape comment", + "Open comment pop-up", + "Close comment pop-up", + "Move comment pop-up", "Resize comment", "Change comment properties", "Fill in {name}", @@ -217,6 +220,9 @@ impl Edit { Edit::LockAnnotation { .. } => Label::of("Unlock comment"), Edit::MoveAnnotation { .. } => Label::of("Move comment"), Edit::ReshapeAnnotation { .. } => Label::of("Reshape comment"), + Edit::SetAnnotationPopup { rect: Some(_), .. } => Label::of("Move comment pop-up"), + Edit::SetAnnotationPopup { open: Some(true), .. } => Label::of("Open comment pop-up"), + Edit::SetAnnotationPopup { .. } => Label::of("Close comment pop-up"), Edit::ResizeAnnotation { .. } => Label::of("Resize comment"), Edit::StyleAnnotation { .. } | Edit::FillAnnotation { .. } | Edit::FontSizeAnnotation { .. } | Edit::SetAnnotationInfo { .. } => { Label::of("Change comment properties") diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index c6b1a7438b..aa55a90105 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -58,6 +58,7 @@ pub(crate) fn check_permission(edit: &Edit, p: &pdfcraft_cos::Permissions) -> Re | Edit::LockAnnotation { .. } | Edit::MoveAnnotation { .. } | Edit::ReshapeAnnotation { .. } + | Edit::SetAnnotationPopup { .. } | Edit::ResizeAnnotation { .. } | Edit::StyleAnnotation { .. } | Edit::FillAnnotation { .. } @@ -384,6 +385,7 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E Edit::LockAnnotation { page, index, locked } => pdfcraft_annot::set_locked(doc, *page, *index, *locked)?, Edit::MoveAnnotation { page, index, dx, dy } => pdfcraft_annot::move_annotation(doc, *page, *index, *dx, *dy, &cx.meta())?, Edit::ReshapeAnnotation { page, index, points } => pdfcraft_annot::set_points(doc, *page, *index, points, &cx.meta())?, + Edit::SetAnnotationPopup { page, index, rect, open } => pdfcraft_annot::set_popup(doc, *page, *index, *rect, *open, &cx.meta())?, Edit::ResizeAnnotation { page, index, rect } => pdfcraft_annot::set_rect(doc, *page, *index, *rect, &cx.meta())?, Edit::StyleAnnotation { page, index, color, opacity, width, endings } => { pdfcraft_annot::set_style( diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index fafa65b4c2..fc813fbe19 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -64,6 +64,7 @@ pub(crate) fn scope_of(edit: &Edit) -> Scope { | Edit::EraseInk { .. } | Edit::MoveAnnotation { .. } | Edit::ReshapeAnnotation { .. } + | Edit::SetAnnotationPopup { .. } | Edit::ResizeAnnotation { .. } | Edit::StyleAnnotation { .. } | Edit::FillAnnotation { .. } diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 0cf287b3f1..25940ed032 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -37,7 +37,7 @@ pub use core::{check_pages, inspect, pretty_date}; use core::{inspect_structure, spend}; pub use model::{ Annotation, Attachment, AttachmentSource, DestView, Field, FieldKind, FontInfo, Layer, LayerOp, Link, LinkTarget, OutlineItem, PageInfo, - PageTransition, RemoteDestination, TransitionStyle, Xfa, link_replies, thread_tops, + PageTransition, Popup, RemoteDestination, TransitionStyle, Xfa, link_replies, thread_tops, }; #[cfg(test)] use navigation::{OUTLINE_DEPTH, OUTLINE_WINDOW}; diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 603aa88110..fcb9dd77bb 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -1,5 +1,5 @@ use super::core::{limited_pdf_string, pretty_date, spend}; -use super::model::{Annotation, AttachmentSource, DestView, Field, FieldKind, LayerOp, Link, LinkTarget, RemoteDestination, link_replies}; +use super::model::{Annotation, AttachmentSource, DestView, Field, FieldKind, LayerOp, Link, LinkTarget, Popup, RemoteDestination, link_replies}; use super::navigation::text_string; use super::{Dictionary, DocInfo, Entry, HashMap, HashSet, Inspector, Object, ObjectId}; @@ -158,6 +158,13 @@ impl<'a> Inspector<'a> { } _ => Vec::new(), }; + let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).map(|p| { + let [x0, y0, x1, y1] = rect4(self.resolve(p.get(b"Rect").unwrap_or(&Object::Null))); + Popup { + rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], + open: p.get(b"Open").ok().and_then(|o| self.resolve(o).as_bool().ok()).unwrap_or(false), + } + }); let annotation = Annotation { page, subtype, @@ -180,6 +187,7 @@ impl<'a> Inspector<'a> { fill_sign, no_rotate: d.get(b"F").is_ok_and(|f| draw_flags(self.resolve(f)) & 16 != 0), points, + popup, }; kept = kept.saturating_add(retained(&annotation)); if kept > MAX_ANNOTATION_BYTES { diff --git a/crates/render/src/inspect/model.rs b/crates/render/src/inspect/model.rs index 490897b60f..d966e23132 100644 --- a/crates/render/src/inspect/model.rs +++ b/crates/render/src/inspect/model.rs @@ -341,6 +341,17 @@ pub struct Annotation { /// The points that shape a line (its two ends, `/L`) or connected lines, a polygon or a /// cloud (their corners, `/Vertices`), in user space; empty for other comments. pub points: Vec<[f32; 2]>, + /// The comment's pop-up window (`/Popup`): where it sits and whether it is open. + pub popup: Option, +} + +/// A comment's pop-up window (ISO 32000-2 §12.5.6.14). +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Popup { + /// `/Rect` of the `/Popup` annotation, in user space. + pub rect: [f32; 4], + /// `/Open`: shown open. + pub open: bool, } impl Annotation { diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index a5bfec474d..79b938719f 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -304,6 +304,7 @@ fn no_rotate_comments_are_shown_upright_from_their_corner_and_back() { fill_sign: false, no_rotate: true, points: Vec::new(), + popup: None, }; let rect = note.rect.map(f64::from); for rotation in [0u16, 90, 180, 270, 45] { @@ -1850,6 +1851,18 @@ fn carriage_returns_in_comment_text_become_line_breaks() { assert_eq!(info.annotations[0].contents.as_deref(), Some("one\n\ntwo\nthree")); } +/// A comment's pop-up (`/Popup`) is read with its place and state; one without reads none. +#[test] +fn a_comments_pop_up_is_read_with_its_place_and_state() { + let bytes = b"%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] /Annots [4 0 R 5 0 R 6 0 R] >> endobj\n4 0 obj << /Type /Annot /Subtype /Text /Rect [20 260 40 280] /Contents (Hi) /Popup 5 0 R /Open true >> endobj\n5 0 obj << /Type /Annot /Subtype /Popup /Rect [190 160 50 280] /Parent 4 0 R /Open true >> endobj\n6 0 obj << /Type /Annot /Subtype /Square /Rect [10 10 60 60] >> endobj\ntrailer << /Root 1 0 R >>\n%%EOF\n"; + let info = inspect(Arc::new(bytes.to_vec()), None).expect("opens"); + let note = info.annotations.iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup, Some(Popup { rect: [50.0, 160.0, 190.0, 280.0], open: true }), "normalised, and open"); + let square = info.annotations.iter().find(|a| a.subtype == "Square").expect("the square"); + assert_eq!(square.popup, None); + assert!(info.annotations.iter().all(|a| a.subtype != "Popup"), "the pop-up is not a comment of its own"); +} + #[test] fn dates_are_prettified() { assert_eq!(pretty_date("D:20260930104512-04'00'"), "2026-09-30 10:45"); diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 6f9a3ac319..7e437fef27 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -19,7 +19,7 @@ pub mod transition; pub use inspect::{ Annotation, Attachment, AttachmentSource, DestView, DocInfo, Field, FieldKind, FontInfo, Layer, LayerOp, Link, LinkTarget, OutlineItem, PageInfo, - PageTransition, RemoteDestination, TransitionStyle, Xfa, attachment_data, check_pages, inspect, link_replies, pretty_date, thread_tops, + PageTransition, Popup, RemoteDestination, TransitionStyle, Xfa, attachment_data, check_pages, inspect, link_replies, pretty_date, thread_tops, }; pub use pixels::Pixels; pub use raster::{ diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 5625587a69..399783f41a 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2712,7 +2712,7 @@ fn track_scroll(ui: &egui::Ui, view: &mut DocView, out: &egui::containers::sc /// The hover tip, find bar, size hint and the in-place editors drawn over the pages. Whether the /// text editor was finished. fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, primary_down: bool, hover_text: Option<(Pos2, String)>) -> bool { - let PageEnv { added, author, avail, can_modify, doc, form, info, prefs, t, today, tool, units, unobstructed, .. } = *env; + let PageEnv { added, author, avail, can_modify, comments_hidden, doc, form, info, prefs, t, today, tool, units, unobstructed, .. } = *env; if let Some((pos, text)) = hover_text { // Click-through, as a tooltip is: the pages lose the pointer under anything that isn't. let tip = egui::Area::new(egui::Id::unique("canvas-hover")).order(egui::Order::Tooltip).interactable(false); @@ -2731,6 +2731,9 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim if let Some(e) = comments::composer(ui.ctx(), view, info, prefs) { view.pending_edit = Some(e); } + if let Some(e) = comments::popups(ui.ctx(), view, info, comments_hidden, can_modify) { + view.pending_edit = Some(e); + } if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), view, info) { view.pending_edit = Some(e); } diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index 2cbde2b995..e21c38a259 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -150,7 +150,7 @@ fn edges_are_primary_finite_in_order_and_capped() { } /// The page modules: the per-page chain in `canvas::document_area` and what it calls. -const PAGE_MODULES: [&str; 35] = [ +const PAGE_MODULES: [&str; 36] = [ "canvas.rs", "canvas/area.rs", "canvas/bars.rs", @@ -167,6 +167,7 @@ const PAGE_MODULES: [&str; 35] = [ "tools/comments/input.rs", "tools/comments/menu.rs", "tools/comments/paint.rs", + "tools/comments/popups.rs", "tools/comments/state.rs", "tools/forms_ui.rs", "tools/prepare.rs", @@ -216,7 +217,7 @@ const RAW_READS: [&str; 21] = [ /// Functions in the page modules that are not page input and read egui's pointer state /// themselves: (file, function, why). -const NOT_PAGE_INPUT: [(&str, &str, &str); 11] = [ +const NOT_PAGE_INPUT: [(&str, &str, &str); 12] = [ ("canvas/area.rs", "zoom_anchor", "a pinch zooms the whole view, and is read before the pages' widget exists"), ("canvas/area.rs", "drawing_edge_pointer", "scrolls the view at its edges, read before the pages' widget exists"), ("canvas/area.rs", "glide_interrupted", "a press anywhere stops the touchpad's glide"), @@ -224,6 +225,7 @@ const NOT_PAGE_INPUT: [(&str, &str, &str); 11] = [ ("canvas/bars.rs", "page_size_hint", "the page size hint sits in the view's corner, not on a page"), ("canvas/organize.rs", "organize_grid", "Organize Pages shows thumbnails, not pages"), ("tools/comments/input.rs", "keys", "keyboard handling after the pages: a drag cancelled with Escape ends once no button is down"), + ("tools/comments/popups.rs", "popups", "the comment pop-up windows are Areas of their own over the page"), ("tools/forms_ui.rs", "overlay", "the field editor is an Area of its own over the page"), ("tools/content_ui.rs", "editor", "the added-text editor is an Area of its own over the page"), ("tools/edit_text_ui.rs", "overlay", "the paragraph editor is an Area of its own over the page"), @@ -251,10 +253,15 @@ fn declares(line: &str) -> Option<&str> { const CURSOR_WRITES: [&str; 4] = ["set_cursor_icon(", ".cursor_icon = ", "on_hover_cursor(", "on_hover_and_drag_cursor("]; /// Functions in the page modules that set a cursor other than the pages': (file, function, why). -const NOT_PAGE_CURSOR: [(&str, &str, &str); 3] = [ +const NOT_PAGE_CURSOR: [(&str, &str, &str); 4] = [ ("tools/fill_sign/scan.rs", "scan_tab", "the signature dialog's Scan tab"), ("tools/edit_text_ui.rs", "overlay", "the paragraph editor is an Area of its own over the page, drawn after the pages' cursor is set"), ("tools/calc_order_ui.rs", "panel", "the Calculation order panel, whose rows are dragged to reorder them"), + ( + "tools/comments/popups.rs", + "popups", + "the comment pop-up windows are Areas of their own over the page; their title bars are dragged to move them", + ), ]; /// Whether `line` uses `name`, and not a longer name that ends in it (a field, after its `.`, is diff --git a/crates/ui-egui/src/canvas/xform.rs b/crates/ui-egui/src/canvas/xform.rs index 91a3739d62..6c41645f43 100644 --- a/crates/ui-egui/src/canvas/xform.rs +++ b/crates/ui-egui/src/canvas/xform.rs @@ -20,6 +20,11 @@ impl DocView { pub(crate) fn page_xform(&self, page: usize) -> Option { self.screen_xforms.iter().find(|(p, _)| *p == page).map(|(_, xf)| *xf) } + + /// Where the pages are on screen this frame, with their transforms. + pub(crate) fn page_xforms(&self) -> Vec<(usize, PageXform)> { + self.screen_xforms.iter().map(|(p, xf)| (*p, *xf)).collect() + } } /// `r` moved so its corner lies on a whole physical pixel, so that a raster drawn from there diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 8def569f58..b26e2aed42 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -809,6 +809,7 @@ fn comment_page(edit: &Edit) -> Option { | Edit::SetAnnotationStatus { page, .. } | Edit::MoveAnnotation { page, .. } | Edit::ReshapeAnnotation { page, .. } + | Edit::SetAnnotationPopup { page, .. } | Edit::ResizeAnnotation { page, .. } | Edit::StyleAnnotation { page, .. } | Edit::FillAnnotation { page, .. } diff --git a/crates/ui-egui/src/i18n/ar.tsv b/crates/ui-egui/src/i18n/ar.tsv index 6e06c424df..54801f7fec 100644 --- a/crates/ui-egui/src/i18n/ar.tsv +++ b/crates/ui-egui/src/i18n/ar.tsv @@ -1980,6 +1980,9 @@ Lock comment قفل التعليق Unlock comment إلغاء قفل التعليق Move comment نقل التعليق + Open comment pop-up فتح النافذة المنبثقة للتعليق + Close comment pop-up إغلاق النافذة المنبثقة للتعليق + Move comment pop-up نقل النافذة المنبثقة للتعليق Resize comment تغيير حجم التعليق Change comment properties تغيير خصائص التعليق Add field إضافة حقل diff --git a/crates/ui-egui/src/i18n/bg.tsv b/crates/ui-egui/src/i18n/bg.tsv index 8fe56514df..60b3457ca5 100644 --- a/crates/ui-egui/src/i18n/bg.tsv +++ b/crates/ui-egui/src/i18n/bg.tsv @@ -2069,6 +2069,9 @@ Microsoft Word (.docx) Microsoft Word (.docx) Move bookmark Премести отметката Move comment Премести коментара + Open comment pop-up Отваряне на изскачащия прозорец на коментара + Close comment pop-up Затваряне на изскачащия прозорец на коментара + Move comment pop-up Преместване на изскачащия прозорец на коментара Move image Премести изображението Move page Премести страницата Move pages Премести страниците diff --git a/crates/ui-egui/src/i18n/cs.tsv b/crates/ui-egui/src/i18n/cs.tsv index 8ecd80d30b..7b8f3731ce 100644 --- a/crates/ui-egui/src/i18n/cs.tsv +++ b/crates/ui-egui/src/i18n/cs.tsv @@ -2030,6 +2030,9 @@ key Space Mezerník Lock comment Zamknout komentář Unlock comment Odemknout komentář Move comment Přesunout komentář + Open comment pop-up Otevřít okno komentáře + Close comment pop-up Zavřít okno komentáře + Move comment pop-up Přesunout okno komentáře Resize comment Změnit velikost komentáře Change comment properties Změnit vlastnosti komentáře Add field Přidat pole diff --git a/crates/ui-egui/src/i18n/de.tsv b/crates/ui-egui/src/i18n/de.tsv index 2d6afea2c8..234e448f48 100644 --- a/crates/ui-egui/src/i18n/de.tsv +++ b/crates/ui-egui/src/i18n/de.tsv @@ -1916,6 +1916,9 @@ Lock comment Kommentar sperren Unlock comment Kommentar entsperren Move comment Kommentar verschieben + Open comment pop-up Kommentar-Pop-up öffnen + Close comment pop-up Kommentar-Pop-up schließen + Move comment pop-up Kommentar-Pop-up verschieben Resize comment Kommentargröße ändern Change comment properties Kommentareigenschaften ändern Add field Feld hinzufügen diff --git a/crates/ui-egui/src/i18n/el.tsv b/crates/ui-egui/src/i18n/el.tsv index 3c2f2e2050..85144efa79 100644 --- a/crates/ui-egui/src/i18n/el.tsv +++ b/crates/ui-egui/src/i18n/el.tsv @@ -1345,6 +1345,9 @@ Lock comment Κλείδωμα σχολίου Unlock comment Ξεκλείδωμα σχολίου Move comment Μετακίνηση σχολίου + Open comment pop-up Άνοιγμα αναδυόμενου παραθύρου σχολίου + Close comment pop-up Κλείσιμο αναδυόμενου παραθύρου σχολίου + Move comment pop-up Μετακίνηση αναδυόμενου παραθύρου σχολίου Resize comment Αλλαγή μεγέθους σχολίου Change comment properties Αλλαγή ιδιοτήτων σχολίου Add field Προσθήκη πεδίου diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index f5f1b4b6ff..ba9b3b64b7 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -1995,6 +1995,9 @@ action wizard Start Iniciar Lock comment Bloquear comentario Unlock comment Desbloquear comentario Move comment Mover comentario + Open comment pop-up Abrir la ventana emergente del comentario + Close comment pop-up Cerrar la ventana emergente del comentario + Move comment pop-up Mover la ventana emergente del comentario Resize comment Cambiar tamaño del comentario Change comment properties Cambiar propiedades del comentario Add field Añadir campo diff --git a/crates/ui-egui/src/i18n/fr.tsv b/crates/ui-egui/src/i18n/fr.tsv index f31b3741b6..6a75a5fb49 100644 --- a/crates/ui-egui/src/i18n/fr.tsv +++ b/crates/ui-egui/src/i18n/fr.tsv @@ -1911,6 +1911,9 @@ Lock comment Verrouiller le commentaire Unlock comment Déverrouiller le commentaire Move comment Déplacer le commentaire + Open comment pop-up Ouvrir la fenêtre du commentaire + Close comment pop-up Fermer la fenêtre du commentaire + Move comment pop-up Déplacer la fenêtre du commentaire Resize comment Redimensionner le commentaire Change comment properties Modifier les propriétés du commentaire Add field Ajouter un champ diff --git a/crates/ui-egui/src/i18n/hu.tsv b/crates/ui-egui/src/i18n/hu.tsv index e74944e33e..79275bf6f2 100644 --- a/crates/ui-egui/src/i18n/hu.tsv +++ b/crates/ui-egui/src/i18n/hu.tsv @@ -1901,6 +1901,9 @@ Lock comment Megjegyzés zárolása Unlock comment Megjegyzés feloldása Move comment Megjegyzés áthelyezése + Open comment pop-up Megjegyzés előugró ablakának megnyitása + Close comment pop-up Megjegyzés előugró ablakának bezárása + Move comment pop-up Megjegyzés előugró ablakának áthelyezése Resize comment Megjegyzés méretezése Change comment properties Megjegyzés tulajdonságainak módosítása Add field Mező hozzáadása diff --git a/crates/ui-egui/src/i18n/it.tsv b/crates/ui-egui/src/i18n/it.tsv index 82973756fa..d97720d519 100644 --- a/crates/ui-egui/src/i18n/it.tsv +++ b/crates/ui-egui/src/i18n/it.tsv @@ -1986,6 +1986,9 @@ Lock comment Blocca commento Unlock comment Sblocca commento Move comment Sposta commento + Open comment pop-up Apri il pop-up del commento + Close comment pop-up Chiudi il pop-up del commento + Move comment pop-up Sposta il pop-up del commento Resize comment Ridimensiona commento Change comment properties Modifica proprietà commento Add field Aggiungi campo diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index ef055d436f..d4ae187d24 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -1993,6 +1993,9 @@ Lock comment 注釈をロック Unlock comment 注釈のロックを解除 Move comment 注釈を移動 + Open comment pop-up 注釈のポップアップを開く + Close comment pop-up 注釈のポップアップを閉じる + Move comment pop-up 注釈のポップアップを移動 Resize comment 注釈のサイズを変更 Change comment properties 注釈のプロパティを変更 Add field フィールドを追加 diff --git a/crates/ui-egui/src/i18n/kk.tsv b/crates/ui-egui/src/i18n/kk.tsv index 47061d3a25..74b0bc50e7 100644 --- a/crates/ui-egui/src/i18n/kk.tsv +++ b/crates/ui-egui/src/i18n/kk.tsv @@ -1902,6 +1902,9 @@ Lock comment Түсініктемені құлыптау Unlock comment Түсініктеменің құлпын ашу Move comment Түсініктемені жылжыту + Open comment pop-up Пікірдің қалқымалы терезесін ашу + Close comment pop-up Пікірдің қалқымалы терезесін жабу + Move comment pop-up Пікірдің қалқымалы терезесін жылжыту Resize comment Түсініктеме өлшемін өзгерту Change comment properties Түсініктеме сипаттарын өзгерту Add field Өріс қосу diff --git a/crates/ui-egui/src/i18n/lt.tsv b/crates/ui-egui/src/i18n/lt.tsv index 9385c27907..334a9aacfa 100644 --- a/crates/ui-egui/src/i18n/lt.tsv +++ b/crates/ui-egui/src/i18n/lt.tsv @@ -2209,6 +2209,9 @@ comment menu Edit Redaguoti Mouse Up Atleidus pelės mygtuką Move bookmark Perkelti žymelę Move comment Perkelti komentarą + Open comment pop-up Atidaryti komentaro iškylantįjį langą + Close comment pop-up Uždaryti komentaro iškylantįjį langą + Move comment pop-up Perkelti komentaro iškylantįjį langą Move image Perkelti paveikslėlį Move page Perkelti puslapį Move pages Perkelti puslapius diff --git a/crates/ui-egui/src/i18n/pl.tsv b/crates/ui-egui/src/i18n/pl.tsv index e333b1075e..46443462f5 100644 --- a/crates/ui-egui/src/i18n/pl.tsv +++ b/crates/ui-egui/src/i18n/pl.tsv @@ -1974,6 +1974,9 @@ Lock comment Zablokuj komentarz Unlock comment Odblokuj komentarz Move comment Przenieś komentarz + Open comment pop-up Otwórz okienko komentarza + Close comment pop-up Zamknij okienko komentarza + Move comment pop-up Przenieś okienko komentarza Resize comment Zmień rozmiar komentarza Change comment properties Zmień właściwości komentarza Add field Dodaj pole diff --git a/crates/ui-egui/src/i18n/pt-br.tsv b/crates/ui-egui/src/i18n/pt-br.tsv index cd6d8f3e2f..54d290711e 100644 --- a/crates/ui-egui/src/i18n/pt-br.tsv +++ b/crates/ui-egui/src/i18n/pt-br.tsv @@ -1942,6 +1942,9 @@ Microsoft Word (.docx) Microsoft Word (.docx) Move bookmark Mover marcador Move comment Mover comentário + Open comment pop-up Abrir o pop-up do comentário + Close comment pop-up Fechar o pop-up do comentário + Move comment pop-up Mover o pop-up do comentário Move image Mover imagem Move page Mover página Move pages Mover páginas diff --git a/crates/ui-egui/src/i18n/ru.tsv b/crates/ui-egui/src/i18n/ru.tsv index 047c01228e..618f0ef75c 100644 --- a/crates/ui-egui/src/i18n/ru.tsv +++ b/crates/ui-egui/src/i18n/ru.tsv @@ -1981,6 +1981,9 @@ Lock comment Заблокировать комментарий Unlock comment Разблокировать комментарий Move comment Переместить комментарий + Open comment pop-up Открыть всплывающее окно комментария + Close comment pop-up Закрыть всплывающее окно комментария + Move comment pop-up Переместить всплывающее окно комментария Resize comment Изменить размер комментария Change comment properties Изменить свойства комментария Add field Добавить поле diff --git a/crates/ui-egui/src/i18n/sl.tsv b/crates/ui-egui/src/i18n/sl.tsv index 63c3a3f77d..4679a8bd63 100644 --- a/crates/ui-egui/src/i18n/sl.tsv +++ b/crates/ui-egui/src/i18n/sl.tsv @@ -1153,6 +1153,9 @@ Mouse Up Miška navzgor Move bookmark Premakni zaznamek Move comment Premakni komentar + Open comment pop-up Odpri pojavno okno komentarja + Close comment pop-up Zapri pojavno okno komentarja + Move comment pop-up Premakni pojavno okno komentarja Move down Premakni navzdol Move earlier Premakni prej Move image Premakni sliko diff --git a/crates/ui-egui/src/i18n/te.tsv b/crates/ui-egui/src/i18n/te.tsv index 6186eb1713..3cc3403618 100644 --- a/crates/ui-egui/src/i18n/te.tsv +++ b/crates/ui-egui/src/i18n/te.tsv @@ -1900,6 +1900,9 @@ action wizard Start ప్రారంభించు Lock comment వ్యాఖ్యను లాక్ చేయి Unlock comment వ్యాఖ్యను అన్‌లాక్ చేయి Move comment వ్యాఖ్యను తరలించు + Open comment pop-up వ్యాఖ్య పాప్-అప్ తెరవండి + Close comment pop-up వ్యాఖ్య పాప్-అప్ మూసివేయండి + Move comment pop-up వ్యాఖ్య పాప్-అప్ తరలించండి Resize comment వ్యాఖ్య పరిమాణం మార్చు Change comment properties వ్యాఖ్య లక్షణాలను మార్చు Add field ఫీల్డ్ జోడించు diff --git a/crates/ui-egui/src/i18n/tr.tsv b/crates/ui-egui/src/i18n/tr.tsv index a8a993af80..d6bdb24a1f 100644 --- a/crates/ui-egui/src/i18n/tr.tsv +++ b/crates/ui-egui/src/i18n/tr.tsv @@ -1874,6 +1874,9 @@ Lock comment Yorumu kilitle Unlock comment Yorumun kilidini aç Move comment Yorumu taşı + Open comment pop-up Yorum açılır penceresini aç + Close comment pop-up Yorum açılır penceresini kapat + Move comment pop-up Yorum açılır penceresini taşı Resize comment Yorumu yeniden boyutlandır Change comment properties Yorum özelliklerini değiştir Add field Alan ekle diff --git a/crates/ui-egui/src/i18n/uk.tsv b/crates/ui-egui/src/i18n/uk.tsv index 566e673176..348359b93a 100644 --- a/crates/ui-egui/src/i18n/uk.tsv +++ b/crates/ui-egui/src/i18n/uk.tsv @@ -1978,6 +1978,9 @@ Lock comment Заблокувати коментар Unlock comment Розблокувати коментар Move comment Перемістити коментар + Open comment pop-up Відкрити спливаюче вікно коментаря + Close comment pop-up Закрити спливаюче вікно коментаря + Move comment pop-up Перемістити спливаюче вікно коментаря Resize comment Змінити розмір коментаря Change comment properties Змінити властивості коментаря Add field Додати поле diff --git a/crates/ui-egui/src/i18n/vi.tsv b/crates/ui-egui/src/i18n/vi.tsv index 41063cc0da..a4a016865e 100644 --- a/crates/ui-egui/src/i18n/vi.tsv +++ b/crates/ui-egui/src/i18n/vi.tsv @@ -2846,3 +2846,8 @@ key Space Space Options for this reply Tùy chọn cho câu trả lời này Export value Giá trị xuất Show Page Edge Shadows Hiển thị bóng viền trang + +# AI-assisted additions for comment pop-up windows (2026-10-16); not reviewed by a native speaker. + Open comment pop-up Mở cửa sổ bật lên của bình luận + Close comment pop-up Đóng cửa sổ bật lên của bình luận + Move comment pop-up Di chuyển cửa sổ bật lên của bình luận diff --git a/crates/ui-egui/src/i18n/zh-hans.tsv b/crates/ui-egui/src/i18n/zh-hans.tsv index 6089c942fd..c0645a0c6d 100644 --- a/crates/ui-egui/src/i18n/zh-hans.tsv +++ b/crates/ui-egui/src/i18n/zh-hans.tsv @@ -2079,6 +2079,9 @@ Microsoft Word (.docx) Microsoft Word（.docx） Move bookmark 移动书签 Move comment 移动注释 + Open comment pop-up 打开注释弹出窗口 + Close comment pop-up 关闭注释弹出窗口 + Move comment pop-up 移动注释弹出窗口 Move image 移动图像 Move page 移动页面 Move pages 移动页面 diff --git a/crates/ui-egui/src/i18n/zh-hant.tsv b/crates/ui-egui/src/i18n/zh-hant.tsv index 292e399579..6455101d38 100644 --- a/crates/ui-egui/src/i18n/zh-hant.tsv +++ b/crates/ui-egui/src/i18n/zh-hant.tsv @@ -1980,6 +1980,9 @@ Lock comment 鎖定註解 Unlock comment 解鎖註解 Move comment 移動註解 + Open comment pop-up 開啟註解彈出視窗 + Close comment pop-up 關閉註解彈出視窗 + Move comment pop-up 移動註解彈出視窗 Resize comment 調整註解大小 Change comment properties 變更註解內容 Add field 新增欄位 diff --git a/crates/ui-egui/src/tools/comments.rs b/crates/ui-egui/src/tools/comments.rs index e9956e5f95..fc0236bba7 100644 --- a/crates/ui-egui/src/tools/comments.rs +++ b/crates/ui-egui/src/tools/comments.rs @@ -26,6 +26,7 @@ mod geometry; mod input; mod menu; mod paint; +mod popups; mod state; pub(crate) use composer::{composer, submit_key}; @@ -45,6 +46,7 @@ pub(crate) use menu::{attachment_items, context_menu, quick_bar_controls}; pub(crate) use paint::paint_page; #[cfg(test)] use paint::{changing_points, drawn_shape, edited_in_view}; +pub(crate) use popups::popups; #[cfg(test)] use state::after_delete; use state::login_name; diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index 5e74fdf56e..1a44f656da 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -3,6 +3,7 @@ use super::geometry::{ resized, snapped_caret, }; use super::paint::{drawn_shape, edit_anchor, erase, new_comment}; +use super::popups::has_popup; use super::state::{Composer, ComposerKind, Gesture, PageCx}; use super::{Annotation, CommentPrefs, CommentTool, DocInfo, DocView, Edit, Pos2, QuickTool, Rect, Shape, vec2}; use crate::page_cursor::{Layer, Subject}; @@ -149,6 +150,8 @@ pub(crate) fn describe(cx: &PageCx<'_>, view: &mut DocView) { // tip is that of the one drawn on top, among those with something to say (#3620). let top = cx .comments() + // An open pop-up already shows the comment: no tooltip over it. + .filter(|a| !a.popup.is_some_and(|w| w.open)) .filter(|a| cx.screen_rects(a).any(|r| r.contains(p))) .filter_map(|a| hover_tip(a, cx.prefs).map(|text| (a.index, text))) .max_by_key(|(index, _)| *index); @@ -263,12 +266,20 @@ fn select_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mu None => cv.selected = None, } } - if resp.double_clicked() + if (resp.double_clicked() || resp.triple_clicked()) && on_page && cx.allowed && let Some((a, _)) = owner { cv.selected = Some((cx.page, a.index)); + // A double-click opens the comment's pop-up window, as in Acrobat; a text box, whose + // text is the comment itself, is edited in place. + if has_popup(a) { + if !a.popup.is_some_and(|p| p.open) { + view.pending_edit = Some(Edit::SetAnnotationPopup { page: cx.page, index: a.index, rect: None, open: Some(true) }); + } + return true; + } cv.composer = Some(Composer { page: cx.page, at: edit_anchor(cx.info, a), diff --git a/crates/ui-egui/src/tools/comments/paint.rs b/crates/ui-egui/src/tools/comments/paint.rs index bfcd67982b..77d6315255 100644 --- a/crates/ui-egui/src/tools/comments/paint.rs +++ b/crates/ui-egui/src/tools/comments/paint.rs @@ -1,4 +1,5 @@ use super::geometry::{HANDLES, grabs_points, handle_pos, is_markup, reshapable, reshaped, resizable}; +use super::popups::paint_connectors; use super::state::{Gesture, PageCx, SETTLE_SECS}; use super::{ Annotation, Color32, CommentTool, CornerRadius, DocInfo, DocView, Edit, LineEnding, NewAnnotation, Pos2, QuickTool, Rect, SELECT_BLUE, Shape, @@ -57,6 +58,7 @@ pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_> } } } + paint_connectors(painter, cx, view, pointer); paint_gesture(painter, cx, view); } diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs new file mode 100644 index 0000000000..12fef6ca5f --- /dev/null +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -0,0 +1,196 @@ +use super::state::PageCx; +use super::{ + Annotation, Color32, CommentPrefs, CornerRadius, DocInfo, DocView, Edit, PagePointer, PageXform, Pos2, QuickTool, Rect, Stroke, Tokens, pos2, + theme, +}; + +/// A comment with a pop-up window of its own (not a text box, a Fill & Sign mark or a reply). +pub(super) fn has_popup(a: &Annotation) -> bool { + a.subtype != "FreeText" && !a.fill_sign && a.in_reply_to.is_none() +} + +/// The line from a comment to its open pop-up, in the comment's colour, while the pointer is on +/// either or the comment is selected (Acrobat's "lines connecting comment markups to their +/// pop-ups on mouse rollover"). It runs from the point of the comment nearest the window to the +/// point of the window nearest the comment. +pub(super) fn paint_connectors(painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView, pointer: Option) { + let cv = &view.comments; + for ((page, index), window) in &cv.popup_rects { + if *page != cx.page { + continue; + } + let Some(a) = cx.get(*index) else { continue }; + if !a.popup.is_some_and(|p| p.open) { + continue; + } + let rects: Vec = cx.screen_rects(a).collect(); + let on_mark = pointer.is_some_and(|p| rects.iter().any(|r| r.expand(3.0).contains(p))); + if !(on_mark || cv.selected == Some((*page, *index)) || cv.popup_hovered == Some((*page, *index))) { + continue; + } + let Some(mark) = rects.first() else { continue }; + let from = nearest_on(*mark, window.center()); + let to = nearest_on(*window, from); + if from.distance(to) < 4.0 { + continue; + } + let color = a + .color + .map_or(Color32::from_rgb(0xDC, 0x26, 0x26), |c| Color32::from_rgb((c[0] * 255.0) as u8, (c[1] * 255.0) as u8, (c[2] * 255.0) as u8)); + painter.line_segment([from, to], Stroke::new(1.5, color)); + } +} + +/// The point of `r`'s boundary nearest `p` (`p` inside `r` is pushed to the nearer vertical edge). +fn nearest_on(r: Rect, p: Pos2) -> Pos2 { + let x = p.x.clamp(r.left(), r.right()); + let y = p.y.clamp(r.top(), r.bottom()); + if r.left() < x && x < r.right() && r.top() < y && y < r.bottom() { + pos2(if (p.x - r.left()).abs() <= (p.x - r.right()).abs() { r.left() } else { r.right() }, y) + } else { + pos2(x, y) + } +} + +/// The open pop-up windows of the comments on screen: author, date and the comment's text, +/// editable in place; the title bar drags the window (its new place is saved with the comment); +/// × closes it. Returns the edits to apply (one, or a batch when a click both saves typed text +/// and closes). Hidden comments show none. +pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hidden: bool, allowed: bool) -> Option { + let t = Tokens::get(ctx); + let pointer = ctx.input(|i| i.pointer.hover_pos()); + let mut edits: Vec = Vec::new(); + let mut rects = Vec::new(); + let mut hovered = None; + let xforms: Vec<(usize, PageXform)> = view.page_xforms(); + for (page, xf) in xforms { + let shown: Vec = + info.annotations.iter().filter(|a| !hidden && a.page == page && has_popup(a) && a.popup.is_some_and(|p| p.open)).cloned().collect(); + for a in shown { + let Some(p) = a.popup else { continue }; + let key = (page, a.index); + let placed = xf.user_rect(info, page, p.rect); + // The window keeps a readable size whatever the zoom: its place follows the page, + // its width stays between what a note needs and what fits. + let width = placed.width().clamp(220.0, 420.0); + let dragging = view.comments.popup_drag.filter(|(dp, di, _)| (*dp, *di) == key).map(|(_, _, at)| at); + let pos = dragging.unwrap_or(placed.left_top()); + let id = egui::Id::unique(("comment-popup", view.id.0, page, a.index)); + let color = a + .color + .map_or(Color32::from_rgb(0xFF, 0xD6, 0x5C), |c| Color32::from_rgb((c[0] * 255.0) as u8, (c[1] * 255.0) as u8, (c[2] * 255.0) as u8)); + let area = egui::Area::new(id).order(egui::Order::Foreground).fixed_pos(pos).show(ctx, |ui| { + egui::Frame::popup(ui.style()).inner_margin(egui::Margin::ZERO).corner_radius(CornerRadius::same(6)).show(ui, |ui| { + ui.set_width(width); + // Title bar: the comment's colour, author and date; drag to move, × to close. + let bar = egui::Frame::NONE.fill(color.gamma_multiply(0.35)).inner_margin(egui::Margin::symmetric(10, 6)).show(ui, |ui| { + ui.set_width(width - 20.0); + ui.horizontal(|ui| { + let who = a.author.clone().unwrap_or_else(|| crate::comments_panel::kind_label(&a).to_string()); + ui.label(egui::RichText::new(who.as_str()).font(theme::semibold(13.0)).color(t.text)); + if let Some(m) = &a.modified { + ui.label(egui::RichText::new(m).font(theme::regular(11.0)).color(t.text_faint)); + } + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if crate::icons::button(ui, "x", 18.0, false, tl!("Close")).clicked() { + edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: None, open: Some(false) }); + } + }); + }); + }); + // The bar drags the window, except where the × is: a region registered after + // the button would take its clicks. + let bar_rect = bar.response.rect; + let grip = ui.interact( + Rect::from_min_max(bar_rect.min, pos2(bar_rect.right() - 30.0, bar_rect.bottom())), + id.with("grip"), + egui::Sense::drag(), + ); + if grip.hovered() || grip.dragged() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + } + if grip.drag_started() { + view.comments.popup_drag = Some((page, a.index, pos)); + } + if grip.dragged() + && let Some((_, _, at)) = view.comments.popup_drag.as_mut() + { + *at += grip.drag_delta(); + } + if grip.drag_stopped() + && let Some((_, _, at)) = view.comments.popup_drag.take() + && allowed + { + // The same size on the page, at the new place. + let moved = Rect::from_min_size(at, placed.size()); + // Only converts the rectangle to user space: the pointer and the + // redaction look are unused. + let cx = PageCx { + page, + xf: &xf, + ptr: &PagePointer::default(), + info, + tool: QuickTool::Select, + prefs: &CommentPrefs::default(), + allowed, + hidden, + redact_mark: ([0.0; 3], 1.0), + }; + let rect = cx.rect_to_user(moved); + if at.distance(placed.left_top()) >= 2.0 { + edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: Some(rect), open: None }); + } + } + // Body: the text, typed in place; saved when the field loses focus. + egui::Frame::NONE.inner_margin(egui::Margin::symmetric(10, 8)).show(ui, |ui| { + ui.set_width(width - 20.0); + if allowed && !a.locked { + let typed = view.comments.popup_text.as_ref().filter(|(p, i, _)| (*p, *i) == key).map(|(_, _, s)| s.clone()); + let mut text = typed.unwrap_or_else(|| a.contents.clone().unwrap_or_default()); + let field = ui.add( + egui::TextEdit::multiline(&mut text) + .hint_text(tl!("Add a comment")) + .desired_rows(3) + .desired_width(f32::INFINITY) + .id_salt(("popup-text", page, a.index)), + ); + if field.changed() { + view.comments.popup_text = Some((page, a.index, text.clone())); + } + if field.lost_focus() + && let Some((_, _, s)) = view.comments.popup_text.take_if(|(p, i, _)| (*p, *i) == key) + && s.trim_end() != a.contents.as_deref().unwrap_or_default() + { + edits.push(Edit::SetAnnotationContents { page, index: a.index, text: s.trim_end().to_string() }); + } + } else { + ui.label(a.contents.as_deref().unwrap_or_default()); + } + // Replies, as the thread reads. + let replies: Vec<&Annotation> = + info.annotations.iter().filter(|r| r.page == page && r.in_reply_to.is_some() && r.in_reply_to == a.name).collect(); + for r in replies { + ui.add_space(4.0); + ui.horizontal_wrapped(|ui| { + ui.label(egui::RichText::new(r.author.as_deref().unwrap_or("?")).font(theme::semibold(12.0))); + ui.label(egui::RichText::new(r.contents.as_deref().unwrap_or_default()).font(theme::regular(12.0))); + }); + } + }); + }); + }); + let shown_at = area.response.rect; + if pointer.is_some_and(|p| shown_at.contains(p)) { + hovered = Some(key); + } + rects.push((key, shown_at)); + } + } + view.comments.popup_rects = rects; + view.comments.popup_hovered = hovered; + match edits.len() { + 0 => None, + 1 => edits.pop(), + _ => Some(Edit::Batch { label: "Edit comment".into(), edits }), + } +} diff --git a/crates/ui-egui/src/tools/comments/state.rs b/crates/ui-egui/src/tools/comments/state.rs index ab24352256..bc1564aad9 100644 --- a/crates/ui-egui/src/tools/comments/state.rs +++ b/crates/ui-egui/src/tools/comments/state.rs @@ -139,6 +139,15 @@ pub struct CommentView { pub editing: Option<(usize, usize, String)>, /// The panel should scroll the selected card into view. pub reveal: bool, + /// A pop-up window's title bar being dragged: the comment, and where the window's top-left + /// corner is now on screen. + pub popup_drag: Option<(usize, usize, Pos2)>, + /// Text being typed in a pop-up window: (page, index, text). + pub popup_text: Option<(usize, usize, String)>, + /// Where each open pop-up was drawn last frame, for the connector lines and hit-testing. + pub popup_rects: Vec<((usize, usize), Rect)>, + /// The pop-up the pointer was over last frame. + pub popup_hovered: Option<(usize, usize)>, /// The "Add a comment" box at the top of the Comments panel. pub add_box: String, /// The Comments panel's search query (`None`: search closed). diff --git a/crates/ui-egui/src/tools/comments/tests.rs b/crates/ui-egui/src/tools/comments/tests.rs index 91e796e43a..0eb2ba4456 100644 --- a/crates/ui-egui/src/tools/comments/tests.rs +++ b/crates/ui-egui/src/tools/comments/tests.rs @@ -42,6 +42,7 @@ fn an_upright_note_on_a_turned_page_is_in_view_where_it_is_drawn() { fill_sign: false, no_rotate, points: Vec::new(), + popup: None, }; let page = PageInfo { width: 140.0, height: 200.0, label: String::new(), crop: [0.0, 0.0, 200.0, 140.0], rotation: 90 }; let xf = PageXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(140.0, 200.0)), rot: 0, pw: 140.0, ph: 200.0 }; @@ -87,6 +88,7 @@ fn listed(page: usize, index: usize, contents: &str, parent: Option<(usize, usiz fill_sign: false, no_rotate: false, points: Vec::new(), + popup: None, } } @@ -176,6 +178,7 @@ fn annotation(subtype: &str, contents: Option<&str>, fill_sign: bool) -> Annotat fill_sign, no_rotate: false, points: Vec::new(), + popup: None, } } @@ -380,6 +383,7 @@ fn context_menu_colour_submenu_keeps_its_picker_open_and_a_swatch_closes_the_men intent: None, fill_sign: false, points: Vec::new(), + popup: None, no_rotate: false, }], ..Default::default() diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 664f61733e..6b479ec8c4 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3022,3 +3022,68 @@ fn an_upright_comment_on_a_turned_page_is_drawn_where_it_goes_before_release() { h.drop_at(from + delta); h.run_steps(3); } + +/// A double-click on a sticky note opens its pop-up window on the page, where its text is read +/// and edited; the title bar drags it (saved as the pop-up's place, one undo step); × closes it +/// (comment.popups, comment.connector-lines). +#[test] +fn a_double_click_opens_a_notes_pop_up_which_drags_and_closes() { + // Frames a sixtieth apart, so two clicks land within egui's double-click time. + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("text.pdf", None, TEXT_FIXTURE.to_vec()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app.set_option("author", "Tester").unwrap(); + app.set_option("quick", "note").unwrap(); + app + }); + h.run_steps(4); + crate::support::settle(&mut h); + click_pt(&mut h, (60.0, 160.0)); + field(&h, "Add a comment").type_text("Please check"); + h.run_steps(2); + h.get_by_label("Post").click(); + h.run_steps(3); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.map(|p| p.open), Some(false)); + assert!(h.state().views[0].comments.popup_rects.is_empty(), "closed pop-ups are not shown"); + // Well after the Post click, so egui does not count the next clicks with it. + h.run_steps(45); + + // Double-click the icon. + let icon = at(&h, (note.rect[0] + note.rect[2]) / 2.0, (note.rect[1] + note.rect[3]) / 2.0); + h.hover_at(icon); + h.run_steps(1); + for _ in 0..2 { + h.drag_at(icon); + h.run_steps(1); + h.drop_at(icon); + h.run_steps(1); + } + h.run_steps(4); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.map(|p| p.open), Some(true), "the pop-up opened"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Open comment pop-up")); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is on screen"); + h.get_by_label("Tester"); + + // Drag the title bar 80 px right and 40 px down: the pop-up's place on the page moves. + let before = note.popup.unwrap().rect; + let bar = pos2(window.left() + 30.0, window.top() + 12.0); + drag(&mut h, bar, bar + egui::vec2(80.0, 40.0)); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + let after = note.popup.unwrap().rect; + // 80 × 40 screen pixels is about 20 × 10 pt at this zoom. + assert!(after[0] > before[0] + 10.0 && after[1] < before[1] - 5.0, "moved right and down on the page: {before:?} -> {after:?}"); + assert!(((after[2] - after[0]) - (before[2] - before[0])).abs() < 0.5, "the same width"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Move comment pop-up")); + + // Close it. + h.get_by_label("Close").click(); + h.run_steps(4); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.map(|p| p.open), Some(false)); + 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")); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 18498399c9..2e2a141772 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4132,11 +4132,11 @@ title = "Open and reposition comment pop-ups" area = "E" tier = "P1" milestone = "M5" -status = "planned" +status = "shipped" commands = [] -tools = [] -evidence = [] -notes = "" +tools = ["comment_popup", "comment_list"] +evidence = ["crates/ui-egui/tests/it/comments.rs::a_double_click_opens_a_notes_pop_up_which_drags_and_closes", "crates/annot/src/tests.rs::pop_ups_open_close_and_move_and_are_reported", "crates/automation/tests/it/automation.rs::comment_popups_are_opened_moved_and_listed", "crates/render/src/inspect/tests.rs::a_comments_pop_up_is_read_with_its_place_and_state"] +notes = "double-click a comment to open its pop-up window on the page (its /Popup /Open and /Rect, so other viewers show it the same); author, date, the text typed in place and the replies; the title bar drags it, × closes it; each is an undo step. Not yet: Acrobat's pop-up options menu (reply, status, delete from the window), replying from the window, resizing the window, Preferences ▸ Commenting pop-up behaviours (open on roll-over, hide when the Comments list opens)" [[feature]] id = "comment.connector-lines" @@ -4144,11 +4144,11 @@ title = "Connector lines between markups and pop-ups" area = "E" tier = "P2" milestone = "M5" -status = "planned" +status = "shipped" commands = [] tools = [] -evidence = [] -notes = "" +evidence = ["crates/ui-egui/tests/it/comments.rs::a_double_click_opens_a_notes_pop_up_which_drags_and_closes"] +notes = "a line in the comment's colour from the comment to its open pop-up while the pointer is on either or the comment is selected (Acrobat's roll-over lines). Not yet: the preference to turn them off" [[feature]] id = "comment.review-history" From 7a916ae7039a50a3e8c99881edcd594f146b2a00 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:07:46 +0800 Subject: [PATCH 02/19] M6: pop-ups: a /Popup that isn't the comment's own is replaced, edits follow comment permission, selected markup shows its connector Review follow-ups on #2137. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/annot/src/modify.rs | 7 +++- crates/annot/src/tests.rs | 11 ++++++ crates/ui-egui/src/canvas.rs | 5 +-- crates/ui-egui/src/tools/comments/paint.rs | 2 ++ crates/ui-egui/src/tools/comments/popups.rs | 3 +- crates/ui-egui/tests/it/comments.rs | 40 +++++++++++++++++++++ 6 files changed, 64 insertions(+), 4 deletions(-) diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index 126897e509..d594689e7e 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -310,7 +310,12 @@ pub fn set_popup(doc: &mut Document, page: usize, index: usize, rect: Option<[f6 None => None, }; let _ = meta; - match d.reference(b"Popup").filter(|pr| doc.get(*pr).as_dict().is_some()) { + // Only a pop-up that is this comment's: `/Popup` naming another annotation (or a pop-up + // whose `/Parent` is another comment) would move or open that one, so it gets its own. + let own_popup = |pr: &ObjRef| { + doc.get(*pr).as_dict().is_some_and(|pd| pd.name(b"Subtype") == Some(b"Popup") && pd.reference(b"Parent").is_none_or(|parent| parent == r)) + }; + match d.reference(b"Popup").filter(own_popup) { Some(pr) => doc.update_dict(pr, |p| { if let Some(rect) = rect { p.set(b"Rect".to_vec(), num_array(&rect)); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 19973781c8..10e8069681 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2230,6 +2230,17 @@ fn pop_ups_open_close_and_move_and_are_reported() { assert!(given.open); assert!((given.rect[0] - 130.0).abs() < 0.01 && (given.rect[2] - 330.0).abs() < 0.01, "beside the mark: {:?}", given.rect); + // #2137 review: a /Popup naming another annotation (here the note) isn't this comment's: the + // note is left alone and the mark gets a pop-up of its own. + let (_, mr) = annot_ref(&mut doc, 0, mark).unwrap(); + doc.update_dict(mr, |m| m.set(b"Popup".to_vec(), Object::Ref(r))).unwrap(); + let note_before = annot_dict(&doc, r); + set_popup(&mut doc, 0, mark, Some([10.0, 10.0, 110.0, 60.0]), Some(true), &meta("h1")).unwrap(); + assert_eq!(annot_dict(&doc, r), note_before, "the note isn't moved or opened"); + let own = annot_dict(&doc, mr).reference(b"Popup").expect("a pop-up of its own"); + assert_ne!(own, r); + assert_eq!(doc.get(own).as_dict().and_then(|p| p.reference(b"Parent")), Some(mr)); + // None of their own. let text_box = add_annotation(&mut doc, &new(0, Shape::TextBox { rect: [10.0, 10.0, 110.0, 40.0], font_size: 12.0 }), &meta("t1")).unwrap(); assert!(matches!(set_popup(&mut doc, 0, text_box, None, Some(true), &meta("t1")), Err(AnnotError::Invalid(_)))); diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 399783f41a..05a977d9cc 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2712,7 +2712,8 @@ fn track_scroll(ui: &egui::Ui, view: &mut DocView, out: &egui::containers::sc /// The hover tip, find bar, size hint and the in-place editors drawn over the pages. Whether the /// text editor was finished. fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, primary_down: bool, hover_text: Option<(Pos2, String)>) -> bool { - let PageEnv { added, author, avail, can_modify, comments_hidden, doc, form, info, prefs, t, today, tool, units, unobstructed, .. } = *env; + let PageEnv { added, allowed, author, avail, can_modify, comments_hidden, doc, form, info, prefs, t, today, tool, units, unobstructed, .. } = + *env; if let Some((pos, text)) = hover_text { // Click-through, as a tooltip is: the pages lose the pointer under anything that isn't. let tip = egui::Area::new(egui::Id::unique("canvas-hover")).order(egui::Order::Tooltip).interactable(false); @@ -2731,7 +2732,7 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim if let Some(e) = comments::composer(ui.ctx(), view, info, prefs) { view.pending_edit = Some(e); } - if let Some(e) = comments::popups(ui.ctx(), view, info, comments_hidden, can_modify) { + if let Some(e) = comments::popups(ui.ctx(), view, info, comments_hidden, allowed) { view.pending_edit = Some(e); } if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), view, info) { diff --git a/crates/ui-egui/src/tools/comments/paint.rs b/crates/ui-egui/src/tools/comments/paint.rs index 77d6315255..14870dd2f8 100644 --- a/crates/ui-egui/src/tools/comments/paint.rs +++ b/crates/ui-egui/src/tools/comments/paint.rs @@ -39,6 +39,8 @@ pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_> for r in cx.screen_rects(a) { painter.rect_stroke(r.expand(2.0), CornerRadius::ZERO, Stroke::new(1.0, SELECT_BLUE), egui::StrokeKind::Middle); } + // Selected text markup is linked to its open pop-up too. + paint_connectors(painter, cx, view, pointer); return paint_gesture(painter, cx, view); } if view.signature_drag.contains(page, index) && matches!(cv.gesture, Some(Gesture::Move { page: p, index: i, .. }) if (p, i) == (page, index)) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index 12fef6ca5f..ae35c3b69d 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -92,7 +92,8 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hi ui.label(egui::RichText::new(m).font(theme::regular(11.0)).color(t.text_faint)); } ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - if crate::icons::button(ui, "x", 18.0, false, tl!("Close")).clicked() { + // Closing changes the comment's /Open, which needs comment permission. + if ui.add_enabled_ui(allowed, |ui| crate::icons::button(ui, "x", 18.0, false, tl!("Close"))).inner.clicked() { edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: None, open: Some(false) }); } }); diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 6b479ec8c4..62eb98fe49 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3023,6 +3023,46 @@ fn an_upright_comment_on_a_turned_page_is_drawn_where_it_goes_before_release() { h.run_steps(3); } +/// #2137 review: a pop-up is edited under comment permission, not change permission: in a +/// document whose security allows comments but not changes, an open pop-up still takes typing +/// and closes. +#[test] +fn a_pop_up_is_editable_where_comments_are_allowed_but_changes_are_not() { + let plain = 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] >> endobj +4 0 obj << /Type /Annot /Subtype /Text /Rect [20 150 40 170] /T (Ada) /Popup 5 0 R >> endobj +5 0 obj << /Type /Annot /Subtype /Popup /Parent 4 0 R /Rect [60 60 260 160] /Open true >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + let mut doc = pdfcraft_cos::Document::open(std::sync::Arc::new(plain.to_vec())).unwrap(); + doc.set_encryption(&pdfcraft_cos::NewEncryption { + algorithm: pdfcraft_cos::Algorithm::Aes256, + user_password: "", + owner_password: "owner", + // Everything but bit 4, modifying the document. + permissions: -1 ^ 8, + encrypt_metadata: true, + seed: [4; 32], + }) + .unwrap(); + let bytes = pdfcraft_cos::write_full(&doc, &Default::default()).unwrap(); + let mut h = harness(move |app| { + app.open_bytes("restricted.pdf", None, bytes).expect("opens"); + // Only the restricted document, so it is the first view. + app.close_tab(0); + }); + h.run_steps(6); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the open pop-up shows"); + field(&h, "Add a comment"); + let close = h.query_all_by_label("Close").find(|n| window.contains(n.rect().center())).expect("the pop-up's ×"); + close.click(); + h.run_steps(4); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.map(|p| p.open), Some(false), "closed"); +} + /// A double-click on a sticky note opens its pop-up window on the page, where its text is read /// and edited; the title bar drags it (saved as the pop-up's place, one undo step); × closes it /// (comment.popups, comment.connector-lines). From a7fd7f27ae790c0342ec45127d3184ba7cfad087 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:09:04 +0800 Subject: [PATCH 03/19] M4: a pop-up counts as a comment's only when it is a pop-up of that comment (#2137) Review on #2137. set_popup accepted a pop-up whose /Parent was present but not a reference, and the comment list (pdfcraft_annot::summaries and the inspector) reported whatever /Popup named: another annotation's rectangle and open state. One check, own_popup, now decides for both: /Subtype /Popup, and /Parent absent or a reference to the comment (ISO 32000-2:2020 12.5.6.14, Table 186: "Optional; shall be an indirect reference"). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/annot/src/info.rs | 23 ++++++++++--- crates/annot/src/modify.rs | 7 ++-- crates/annot/src/tests.rs | 41 ++++++++++++++++++++++++ crates/render/src/inspect/annotations.rs | 9 +++++- crates/render/src/inspect/tests.rs | 14 ++++++++ 5 files changed, 84 insertions(+), 10 deletions(-) diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index bb4cc238a0..c4b385935e 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -52,9 +52,24 @@ pub struct PopupInfo { pub open: bool, } -/// The pop-up `annot` names, as [`PopupInfo`]: `None` when it has none or it is unreadable. -fn popup_info(doc: &Document, annot: &Dict) -> Option { - let r = annot.reference(b"Popup")?; +/// The pop-up the comment `annot` (the object `own`, when it is one of its own) names in +/// `/Popup`, when it is the comment's: a pop-up annotation whose `/Parent`, if it has one, is a +/// reference to the comment. A `/Popup` naming another annotation, or a pop-up of another +/// comment, isn't. ISO 32000-2:2020 §12.5.6.14, Table 186: `/Parent` is "(Optional; shall be +/// an indirect reference)". +pub(crate) fn own_popup(doc: &Document, own: Option, annot: &Dict) -> Option { + let pr = annot.reference(b"Popup")?; + let is_own = doc.get(pr).as_dict().is_some_and(|pd| { + pd.name(b"Subtype") == Some(b"Popup") && pd.get(b"Parent").is_none_or(|parent| parent.as_ref().is_some_and(|parent| Some(parent) == own)) + }); + is_own.then_some(pr) +} + +/// The comment's own pop-up (see [`own_popup`]), as [`PopupInfo`]: `None` when it has none or it +/// is unreadable. +fn popup_info(doc: &Document, own: Option, annot: &Dict) -> Option { + // Another annotation's rectangle and open state aren't this comment's (review on #2137). + let r = own_popup(doc, own, annot)?; let p = doc.get(r); let p = p.as_dict()?; let rect: Vec = doc.resolve(p.get(b"Rect")?).as_array()?.iter().filter_map(|v| doc.resolve(v).as_f64().map(|x| x as f32)).collect(); @@ -168,7 +183,7 @@ pub fn summaries(doc: &Document) -> Vec { fill_sign: is_fill_sign(doc, d), no_rotate: drawn_upright(doc, d), points, - popup: popup_info(doc, d), + popup: popup_info(doc, entry.as_ref(), d), }); } } diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index d594689e7e..8e5a297ab3 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -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::{is_comment_subtype, line_endings_of, text_value}; +use super::info::{is_comment_subtype, line_endings_of, own_popup, text_value}; use super::lookup::{annot_dict, annot_ref, annotation_flags, annots, is_fill_sign, 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, @@ -312,10 +312,7 @@ pub fn set_popup(doc: &mut Document, page: usize, index: usize, rect: Option<[f6 let _ = meta; // Only a pop-up that is this comment's: `/Popup` naming another annotation (or a pop-up // whose `/Parent` is another comment) would move or open that one, so it gets its own. - let own_popup = |pr: &ObjRef| { - doc.get(*pr).as_dict().is_some_and(|pd| pd.name(b"Subtype") == Some(b"Popup") && pd.reference(b"Parent").is_none_or(|parent| parent == r)) - }; - match d.reference(b"Popup").filter(own_popup) { + match own_popup(doc, Some(r), &d) { Some(pr) => doc.update_dict(pr, |p| { if let Some(rect) = rect { p.set(b"Rect".to_vec(), num_array(&rect)); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 10e8069681..c8e71c68d6 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2240,6 +2240,12 @@ fn pop_ups_open_close_and_move_and_are_reported() { let own = annot_dict(&doc, mr).reference(b"Popup").expect("a pop-up of its own"); assert_ne!(own, r); assert_eq!(doc.get(own).as_dict().and_then(|p| p.reference(b"Parent")), Some(mr)); + // Its own pop-up, with /Parent naming it, is the one moved next time. + set_popup(&mut doc, 0, mark, Some([12.0, 10.0, 112.0, 60.0]), None, &meta("h1")).unwrap(); + assert_eq!(annot_dict(&doc, mr).reference(b"Popup"), Some(own), "the same pop-up"); + + // #2137 review: nor is it reported as the comment's (see the helper). + foreign_pop_ups_are_not_the_marks(&mut doc, r, mark, mr); // None of their own. let text_box = add_annotation(&mut doc, &new(0, Shape::TextBox { rect: [10.0, 10.0, 110.0, 40.0], font_size: 12.0 }), &meta("t1")).unwrap(); @@ -2250,3 +2256,38 @@ fn pop_ups_open_close_and_move_and_are_reported() { assert!(matches!(set_popup(&mut doc, 0, note, Some([f64::NAN, 0.0, 10.0, 10.0]), None, &meta("n1")), Err(AnnotError::Invalid(_)))); assert!(matches!(set_popup(&mut doc, 0, 99, None, Some(true), &meta("n1")), Err(AnnotError::NoSuchAnnotation { .. }))); } + +/// #2137 review: a pop-up that is not the mark's own is neither reported as its pop-up nor +/// changed through it (part of `pop_ups_open_close_and_move_and_are_reported`). +fn foreign_pop_ups_are_not_the_marks(doc: &mut Document, note: ObjRef, mark: usize, mr: ObjRef) { + // #2137 review: nor is it reported as the comment's. A /Popup naming the note, or the note's + // own pop-up (its /Parent is the note), gives the mark no pop-up in the summaries. + let popup_of_mark = |doc: &Document| summaries(doc).into_iter().find(|s| s.index == mark).unwrap().popup; + let notes_popup = annot_dict(doc, note).reference(b"Popup").expect("the note's pop-up"); + for foreign in [note, notes_popup] { + doc.update_dict(mr, |m| m.set(b"Popup".to_vec(), Object::Ref(foreign))).unwrap(); + assert_eq!(popup_of_mark(doc), None, "{foreign:?} isn't the mark's"); + } + // A pop-up whose /Parent isn't a reference (it must be one) isn't the mark's either: it is + // neither reported nor changed, and the mark gets one of its own. + let mut bad = Dict::new(); + bad.set(b"Type".to_vec(), Object::name("Annot")); + bad.set(b"Subtype".to_vec(), Object::name("Popup")); + bad.set(b"Rect".to_vec(), Object::Array([0.0, 0.0, 50.0, 50.0].iter().map(|v| Object::Real(*v)).collect())); + bad.set(b"Parent".to_vec(), Object::Dict(Dict::new())); + let bad = doc.add(Object::Dict(bad)); + doc.update_dict(mr, |m| m.set(b"Popup".to_vec(), Object::Ref(bad))).unwrap(); + assert_eq!(popup_of_mark(doc), None, "a direct /Parent"); + let bad_before = doc.get(bad).as_dict().cloned(); + set_popup(doc, 0, mark, None, Some(true), &meta("h1")).unwrap(); + assert_eq!(doc.get(bad).as_dict().cloned(), bad_before, "left alone"); + assert_ne!(annot_dict(doc, mr).reference(b"Popup"), Some(bad)); + // A pop-up with no /Parent at all is taken as the comment's. + let mut orphan = Dict::new(); + orphan.set(b"Type".to_vec(), Object::name("Annot")); + orphan.set(b"Subtype".to_vec(), Object::name("Popup")); + orphan.set(b"Rect".to_vec(), Object::Array([0.0, 0.0, 50.0, 50.0].iter().map(|v| Object::Real(*v)).collect())); + let orphan = doc.add(Object::Dict(orphan)); + doc.update_dict(mr, |m| m.set(b"Popup".to_vec(), Object::Ref(orphan))).unwrap(); + assert_eq!(popup_of_mark(doc), Some(PopupInfo { rect: [0.0, 0.0, 50.0, 50.0], open: false }), "no /Parent"); +} diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index fcb9dd77bb..e16680119a 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -158,7 +158,14 @@ impl<'a> Inspector<'a> { } _ => Vec::new(), }; - let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).map(|p| { + // Only the comment's own pop-up: a pop-up annotation whose /Parent, if any, is a + // reference to this comment (keep in step with `pdfcraft_annot::summaries`). + let own = a.as_reference().ok(); + let is_own = |p: &&Dictionary| { + self.name(p, b"Subtype").as_deref() == Some("Popup") + && p.get(b"Parent").map_or(true, |parent| parent.as_reference().ok().is_some_and(|id| Some(id) == own)) + }; + let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).map(|p| { let [x0, y0, x1, y1] = rect4(self.resolve(p.get(b"Rect").unwrap_or(&Object::Null))); Popup { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 79b938719f..5258c4f682 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1861,6 +1861,20 @@ fn a_comments_pop_up_is_read_with_its_place_and_state() { let square = info.annotations.iter().find(|a| a.subtype == "Square").expect("the square"); assert_eq!(square.popup, None); assert!(info.annotations.iter().all(|a| a.subtype != "Popup"), "the pop-up is not a comment of its own"); + + // #2137 review: a /Popup naming another annotation, a pop-up of another comment, or one + // whose /Parent isn't a reference isn't this comment's pop-up; one without /Parent is. + for (target, extra, own) in [ + ("6 0 R", "", false), + ("7 0 R", "7 0 obj << /Type /Annot /Subtype /Popup /Rect [0 0 50 50] /Parent 6 0 R >> endobj\n", false), + ("7 0 R", "7 0 obj << /Type /Annot /Subtype /Popup /Rect [0 0 50 50] /Parent << >> >> endobj\n", false), + ("7 0 R", "7 0 obj << /Type /Annot /Subtype /Popup /Rect [0 0 50 50] >> endobj\n", true), + ] { + let text = String::from_utf8_lossy(bytes).replace("/Popup 5 0 R", &format!("/Popup {target}")).replace("trailer", &format!("{extra}trailer")); + let info = inspect(Arc::new(text.into_bytes()), None).expect("opens"); + let note = info.annotations.iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.is_some(), own, "/Popup {target} {extra}"); + } } #[test] From 479390cd50880c65aafbbed8977ca38c344d68e8 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:10:59 +0800 Subject: [PATCH 04/19] M4: a pop-up lists a comment's text replies, not its status or checkmark replies (#2137) The pop-up's reply list kept every annotation in reply to the comment, so a review status (/StateModel /Review) and the private checkmark reply Mark writes (/StateModel /Marked) showed as replies with their generated text. It now lists replies without a /State, as the Comments panel does (comments_panel.rs). Raised by CodeRabbit on #2137. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R3niW7kVeT8mi7TpWc8eAJ --- crates/ui-egui/src/tools/comments/popups.rs | 11 +++++--- crates/ui-egui/tests/it/comments.rs | 28 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index ae35c3b69d..494b6350d3 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -167,9 +167,14 @@ 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. - let replies: Vec<&Annotation> = - info.annotations.iter().filter(|r| r.page == page && r.in_reply_to.is_some() && r.in_reply_to == a.name).collect(); + // 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. + 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()) + .collect(); for r in replies { ui.add_space(4.0); ui.horizontal_wrapped(|ui| { diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 62eb98fe49..dc0f29fcbc 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3063,6 +3063,34 @@ trailer << /Root 1 0 R >> assert_eq!(note.popup.map(|p| p.open), Some(false), "closed"); } +/// #2137 review: a pop-up lists the comment's text replies only, as the Comments panel does. A +/// review status (`/StateModel /Review`) and a checkmark (`/StateModel /Marked`, the private +/// reply Mark writes) are replies too, but not ones to read in the thread. +#[test] +fn a_pop_up_lists_text_replies_but_not_status_or_checkmark_replies() { + 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] /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] >> endobj +4 0 obj << /Type /Annot /Subtype /Text /Rect [20 150 40 170] /T (Ada) /NM (note-1) /Contents (Please check) /Popup 5 0 R >> endobj +5 0 obj << /Type /Annot /Subtype /Popup /Parent 4 0 R /Rect [60 40 260 160] /Open true >> endobj +6 0 obj << /Type /Annot /Subtype /Text /Rect [20 150 40 170] /F 32 /T (Bob) /NM (reply-1) /IRT 4 0 R /Contents (Agreed with this) >> endobj +7 0 obj << /Type /Annot /Subtype /Text /Rect [20 150 40 170] /F 32 /T (Bob) /NM (reply-2) /IRT 4 0 R /State /Accepted /StateModel /Review /Contents (Accepted set by Bob) >> endobj +8 0 obj << /Type /Annot /Subtype /Text /Rect [20 150 40 170] /F 32 /T (Ada) /NM (reply-3) /IRT 4 0 R /State /Marked /StateModel /Marked /Contents (Marked set by Ada) >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + let mut h = harness(move |app| { + app.open_bytes("thread.pdf", None, pdf.to_vec()).expect("opens"); + app.close_tab(0); + }); + h.run_steps(6); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the open pop-up shows"); + let in_window = |label: &str| h.query_all_by_label(label).any(|n| window.contains(n.rect().center())); + assert!(in_window("Agreed with this"), "the text reply"); + assert!(!in_window("Accepted set by Bob"), "not the review status"); + assert!(!in_window("Marked set by Ada"), "not the checkmark"); +} + /// A double-click on a sticky note opens its pop-up window on the page, where its text is read /// and edited; the title bar drags it (saved as the pop-up's place, one undo step); × closes it /// (comment.popups, comment.connector-lines). From 34871b61a55c7a90424ffb677a0e9132eb2c7f37 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:12:33 +0800 Subject: [PATCH 05/19] M5: Catalan, Finnish and Romanian for the pop-up undo labels (#2137) These catalogues were added on main after this branch began. AI-assisted, marked as not reviewed by a native speaker. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/i18n/ca.tsv | 5 +++++ crates/ui-egui/src/i18n/fi.tsv | 5 +++++ crates/ui-egui/src/i18n/ro.tsv | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/crates/ui-egui/src/i18n/ca.tsv b/crates/ui-egui/src/i18n/ca.tsv index 83750ba124..764d3b5bcb 100644 --- a/crates/ui-egui/src/i18n/ca.tsv +++ b/crates/ui-egui/src/i18n/ca.tsv @@ -2838,3 +2838,8 @@ theme Appearance Aspecte # Comments panel: each reply's own "…" menu (AI-assisted, not separately native-reviewed) Options for this reply Opcions d'aquesta resposta + +# AI-assisted additions for comment pop-up windows (#2137); not reviewed by a native speaker. + Open comment pop-up Obre la finestra emergent del comentari + Close comment pop-up Tanca la finestra emergent del comentari + Move comment pop-up Mou la finestra emergent del comentari diff --git a/crates/ui-egui/src/i18n/fi.tsv b/crates/ui-egui/src/i18n/fi.tsv index 80d2a0f859..f23b478c2e 100644 --- a/crates/ui-egui/src/i18n/fi.tsv +++ b/crates/ui-egui/src/i18n/fi.tsv @@ -2847,3 +2847,8 @@ theme Appearance Teema # Comments panel: each reply's own "…" menu (AI-assisted, not separately native-reviewed) Options for this reply Tämän vastauksen asetukset + +# AI-assisted additions for comment pop-up windows (#2137); not reviewed by a native speaker. + Open comment pop-up Avaa kommentin ponnahdusikkuna + Close comment pop-up Sulje kommentin ponnahdusikkuna + Move comment pop-up Siirrä kommentin ponnahdusikkunaa diff --git a/crates/ui-egui/src/i18n/ro.tsv b/crates/ui-egui/src/i18n/ro.tsv index 6b23515b8e..183effb6b0 100644 --- a/crates/ui-egui/src/i18n/ro.tsv +++ b/crates/ui-egui/src/i18n/ro.tsv @@ -2838,3 +2838,8 @@ theme Appearance Aspect # Comments panel: each reply's own "…" menu (AI-assisted, not separately native-reviewed) Options for this reply Opțiuni pentru acest răspuns + +# AI-assisted additions for comment pop-up windows (#2137); not reviewed by a native speaker. + Open comment pop-up Deschidere fereastră pop-up a comentariului + Close comment pop-up Închidere fereastră pop-up a comentariului + Move comment pop-up Mutare fereastră pop-up a comentariului From 45f5a3c591f2d937d3b9b6a4c560e7b9d35db069 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:14:44 +0800 Subject: [PATCH 06/19] 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 Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC --- crates/ui-egui/src/tools/comments/popups.rs | 18 +++++++---- crates/ui-egui/tests/it/comments.rs | 33 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) 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"); + } +} From 65eb3257c5443d828191d2f64480668ff2e7468f Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:15:42 +0800 Subject: [PATCH 07/19] M5: a pop-up is its comment's when /Annots lists the comment through a reference to it (#2137) Review on #2137: when /Annots holds 5 0 R and object 5 is only a reference to the comment 6 0 R, the pop-up's /Parent names 6 0 R, but the ownership check compared it with 5 0 R. The comment's pop-up was then missing from both the on-open listing (render inspect) and the summaries after an edit (annot). Both now compare the references they resolve to. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/annot/src/info.rs | 7 +++++-- crates/annot/src/tests.rs | 9 +++++++++ crates/render/src/inspect/annotations.rs | 5 +++-- crates/render/src/inspect/tests.rs | 7 +++++++ 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index c4b385935e..6a90541437 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -60,7 +60,10 @@ pub struct PopupInfo { pub(crate) fn own_popup(doc: &Document, own: Option, annot: &Dict) -> Option { let pr = annot.reference(b"Popup")?; let is_own = doc.get(pr).as_dict().is_some_and(|pd| { - pd.name(b"Subtype") == Some(b"Popup") && pd.get(b"Parent").is_none_or(|parent| parent.as_ref().is_some_and(|parent| Some(parent) == own)) + pd.name(b"Subtype") == Some(b"Popup") + && pd + .get(b"Parent") + .is_none_or(|parent| parent.as_ref().is_some_and(|parent| own.is_some_and(|own| final_ref(doc, parent) == final_ref(doc, own)))) }); is_own.then_some(pr) } @@ -183,7 +186,7 @@ pub fn summaries(doc: &Document) -> Vec { fill_sign: is_fill_sign(doc, d), no_rotate: drawn_upright(doc, d), points, - popup: popup_info(doc, entry.as_ref(), d), + popup: popup_info(doc, entry.as_ref().map(|r| final_ref(doc, r)), d), }); } } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index c8e71c68d6..e20f001a65 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2255,6 +2255,15 @@ fn pop_ups_open_close_and_move_and_are_reported() { assert!(matches!(set_popup(&mut doc, 0, note, Some([0.0, 0.0, 0.5, 0.5]), None, &meta("n1")), Err(AnnotError::Invalid(_)))); assert!(matches!(set_popup(&mut doc, 0, note, Some([f64::NAN, 0.0, 10.0, 10.0]), None, &meta("n1")), Err(AnnotError::Invalid(_)))); assert!(matches!(set_popup(&mut doc, 0, 99, None, Some(true), &meta("n1")), Err(AnnotError::NoSuchAnnotation { .. }))); + + // #2137 review: /Annots may list the note through an object that is only a reference to it. + // Its pop-up's /Parent names the note itself, so the pop-up is still the note's. + let p = page_ref(&doc, 0).unwrap(); + let via = doc.add(Object::Ref(r)); + let list: Vec = annots(&doc, p).into_iter().map(|o| if o.as_ref() == Some(r) { Object::Ref(via) } else { o }).collect(); + set_annots(&mut doc, p, list).unwrap(); + let listed = summaries(&doc).into_iter().find(|s| s.index == note).unwrap(); + assert_eq!(listed.popup, Some(PopupInfo { rect: [150.0, 100.0, 290.0, 200.0], open: false }), "listed through {via:?}"); } /// #2137 review: a pop-up that is not the mark's own is neither reported as its pop-up nor diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index e16680119a..71e95024ab 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -160,10 +160,11 @@ impl<'a> Inspector<'a> { }; // Only the comment's own pop-up: a pop-up annotation whose /Parent, if any, is a // reference to this comment (keep in step with `pdfcraft_annot::summaries`). - let own = a.as_reference().ok(); + let own = self.resolved_id(a); let is_own = |p: &&Dictionary| { self.name(p, b"Subtype").as_deref() == Some("Popup") - && p.get(b"Parent").map_or(true, |parent| parent.as_reference().ok().is_some_and(|id| Some(id) == own)) + && p.get(b"Parent") + .map_or(true, |parent| parent.as_reference().is_ok() && self.resolved_id(parent).is_some_and(|id| Some(id) == own)) }; let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).map(|p| { let [x0, y0, x1, y1] = rect4(self.resolve(p.get(b"Rect").unwrap_or(&Object::Null))); diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 5258c4f682..94794b698b 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1875,6 +1875,13 @@ fn a_comments_pop_up_is_read_with_its_place_and_state() { let note = info.annotations.iter().find(|a| a.subtype == "Text").expect("the note"); assert_eq!(note.popup.is_some(), own, "/Popup {target} {extra}"); } + + // #2137 review: /Annots may list the note through an object that is only a reference to + // it; its pop-up's /Parent names the note itself, and is still its own. + let text = String::from_utf8_lossy(bytes).replace("/Annots [4 0 R", "/Annots [8 0 R").replace("trailer", "8 0 obj 4 0 R endobj\ntrailer"); + let info = inspect(Arc::new(text.into_bytes()), None).expect("opens"); + let note = info.annotations.iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup, Some(Popup { rect: [50.0, 160.0, 190.0, 280.0], open: true }), "listed through 8 0 R"); } #[test] From 6aea7318f3b8378f0d20d9af4a97e403888fc61b Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:17:05 +0800 Subject: [PATCH 08/19] M5: comment.connector-lines cites a test that reads the painted line (#2137) Review on #2137: the cited test opened, moved and closed a pop-up but never looked at the connector line. The new test reads the frame's shapes: a line in the comment's colour from the comment's edge to the window's, nearest points at each end, shown with the pointer on the comment or the window or with the comment selected, and not otherwise. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/tests/it/comments.rs | 67 ++++++++++++++++++++++++++++- parity/acrobat-features.toml | 2 +- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 6e96ffe260..bd74a70faf 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3093,7 +3093,7 @@ trailer << /Root 1 0 R >> /// A double-click on a sticky note opens its pop-up window on the page, where its text is read /// and edited; the title bar drags it (saved as the pop-up's place, one undo step); × closes it -/// (comment.popups, comment.connector-lines). +/// (comment.popups). #[test] fn a_double_click_opens_a_notes_pop_up_which_drags_and_closes() { // Frames a sixtieth apart, so two clicks land within egui's double-click time. @@ -3188,3 +3188,68 @@ fn a_pop_up_lists_its_replies_by_reference_when_names_repeat() { assert!(shown.iter().all(|c| second.contains(*c)), "{text:?} is in the second note's window"); } } + +// Contributor-original PDF: one blue note with its pop-up open beside it. +const BLUE_NOTE_WITH_OPEN_POP_UP: &[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] >> endobj +4 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /C [0 0 1] /T (Alpha) /Contents (A note) /Popup 5 0 R /Open true >> endobj +5 0 obj << /Type /Annot /Subtype /Popup /Rect [120 20 260 120] /Parent 4 0 R /Open true >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// The line segments of colour `color` the last frame painted. +fn line_segments(h: &Harness<'static, PdfCraftApp>, color: egui::Color32) -> Vec<[Pos2; 2]> { + fn walk(shape: &egui::Shape, color: egui::Color32, out: &mut Vec<[Pos2; 2]>) { + match shape { + egui::Shape::LineSegment { points, stroke } if stroke.color == color => out.push(*points), + egui::Shape::Vec(shapes) => shapes.iter().for_each(|s| walk(s, color, out)), + _ => {} + } + } + let mut out = Vec::new(); + for clipped in &h.output().shapes { + walk(&clipped.shape, color, &mut out); + } + out +} + +/// A comment's connector line (comment.connector-lines): in the comment's colour, from the +/// edge of the comment nearest its open pop-up to the edge of the window nearest that point, +/// shown while the pointer is on the comment or on the window or the comment is selected, and +/// not otherwise. +#[test] +fn a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_either() { + let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |_| {}); + let blue = egui::Color32::from_rgb(0, 0, 255); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + let mark = egui::Rect::from_two_pos(at(&h, 20.0, 180.0), at(&h, 40.0, 160.0)); + let on_edge = |r: egui::Rect, p: Pos2| { + r.expand(1.0).contains(p) && [p.x - r.left(), r.right() - p.x, p.y - r.top(), r.bottom() - p.y].iter().any(|d| d.abs() < 1.0) + }; + + // Away from both: no line. + h.hover_at(at(&h, 280.0, 190.0)); + h.run_steps(2); + assert!(line_segments(&h, blue).is_empty(), "no line with the pointer elsewhere"); + + // On the note or on its window: one line, from the note's edge to the window's. + for (what, pointer) in [("the note", mark.center()), ("the window", window.center())] { + h.hover_at(pointer); + h.run_steps(2); + let lines = line_segments(&h, blue); + assert_eq!(lines.len(), 1, "one line with the pointer on {what}: {lines:?}"); + let [from, to] = lines[0]; + assert!(on_edge(mark, from), "starts on the note's edge {mark:?}: {from:?}"); + assert!(on_edge(window, to), "ends on the window's edge {window:?}: {to:?}"); + // The nearest points: the note's lower right corner, the window's upper left one. + assert!(from.distance(mark.right_bottom()) < 1.0 && to.distance(window.left_top()) < 1.0, "{from:?} -> {to:?}"); + } + + // Selected, the note keeps its line with the pointer elsewhere. + h.state_mut().views[0].comments.selected = Some((0, 0)); + h.hover_at(at(&h, 280.0, 190.0)); + h.run_steps(2); + assert_eq!(line_segments(&h, blue).len(), 1, "a selected comment shows its line"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 2e2a141772..a120d1ad17 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4147,7 +4147,7 @@ milestone = "M5" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/it/comments.rs::a_double_click_opens_a_notes_pop_up_which_drags_and_closes"] +evidence = ["crates/ui-egui/tests/it/comments.rs::a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_either"] notes = "a line in the comment's colour from the comment to its open pop-up while the pointer is on either or the comment is selected (Acrobat's roll-over lines). Not yet: the preference to turn them off" [[feature]] From 002b7bf4ff3cc1eb46f281645014cbd2bb3caeae Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 09:17:14 +0800 Subject: [PATCH 09/19] M5: opening, closing or moving a pop-up keeps the page's raster (#2137) Review on #2137 (nitpick): a SetAnnotationPopup edit marked its page stale, so its tiles were rendered again, its text cache dropped and its selection cleared, though nothing in the raster changes: the window is drawn over the page and a /Popup annotation paints nothing. Edits that also change the comment (a batch) still invalidate as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/editing.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index b26e2aed42..f1c9ba7949 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -146,8 +146,12 @@ impl PdfCraftApp { view.move_background.committed(&edit, doc.edit_generation()); view.text_background.committed(&edit); if let Some(page) = comment_page(&edit).or_else(|| content_page(&edit)) { - // Comment and content edits change one page: keep every other raster. - view.page_changed(page); + // Comment and content edits change one page: keep every other raster. Opening, + // closing or moving a pop-up changes none: the window is drawn over the page, and + // a /Popup annotation paints nothing in the raster (ISO 32000-2 §12.5.6.14). + if !matches!(edit, Edit::SetAnnotationPopup { .. }) { + view.page_changed(page); + } } else if let Some(form_before) = form_before && doc.info.pages.len() == page_count_before && doc.xfa.is_some() == had_xfa From 1b9930e6123e9ac25be99c992d8c9c6c6d9fa314 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:15:29 +0800 Subject: [PATCH 10/19] M5: a comment's connector is a faint wedge to its pop-up (comment.connector-lines) The connector from a comment to its open pop-up window was a one-pixel line. It is now a faint wedge in the comment's colour: a point on the comment's edge nearest the window, widening to a base on the window's side that faces the comment, as the common readers draw it. Shown, as before, while the pointer is on the comment or the window, or the comment is selected. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/tools/comments/popups.rs | 49 +++++++++++++++--- crates/ui-egui/tests/it/comments.rs | 56 +++++++++++++-------- parity/acrobat-features.toml | 2 +- 3 files changed, 79 insertions(+), 28 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index e71139a775..db34e1098e 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -1,7 +1,6 @@ use super::state::PageCx; use super::{ - Annotation, Color32, CommentPrefs, CornerRadius, DocInfo, DocView, Edit, PagePointer, PageXform, Pos2, QuickTool, Rect, Stroke, Tokens, pos2, - theme, + Annotation, Color32, CommentPrefs, CornerRadius, DocInfo, DocView, Edit, PagePointer, PageXform, Pos2, QuickTool, Rect, Tokens, pos2, theme, }; /// A comment with a pop-up window of its own: not a text box, a Fill & Sign mark, a reply or a @@ -10,10 +9,11 @@ pub(super) fn has_popup(a: &Annotation) -> bool { 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 -/// either or the comment is selected (Acrobat's "lines connecting comment markups to their -/// pop-ups on mouse rollover"). It runs from the point of the comment nearest the window to the -/// point of the window nearest the comment. +/// The connector from a comment to its open pop-up, in the comment's colour, while the pointer is +/// on either or the comment is selected (Acrobat's "lines connecting comment markups to their +/// pop-ups on mouse rollover"). It is a faint wedge: a point on the comment where it is +/// nearest the window, widening to a base on the window's side that faces +/// the comment ([`wedge_base`]). pub(super) fn paint_connectors(painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView, pointer: Option) { let cv = &view.comments; for ((page, index), window) in &cv.popup_rects { @@ -38,7 +38,42 @@ pub(super) fn paint_connectors(painter: &egui::Painter, cx: &PageCx<'_>, view: & let color = a .color .map_or(Color32::from_rgb(0xDC, 0x26, 0x26), |c| Color32::from_rgb((c[0] * 255.0) as u8, (c[1] * 255.0) as u8, (c[2] * 255.0) as u8)); - painter.line_segment([from, to], Stroke::new(1.5, color)); + let faded = Color32::from_rgba_unmultiplied(color.r(), color.g(), color.b(), WEDGE_ALPHA); + let [b0, b1] = wedge_base(*window, from, to); + let mut wedge = egui::Mesh::default(); + for p in [from, b0, b1] { + wedge.colored_vertex(p, faded); + } + wedge.add_triangle(0, 1, 2); + painter.add(egui::Shape::mesh(wedge)); + } +} + +/// How opaque a connector's wedge is (of 255). +const WEDGE_ALPHA: u8 = 90; + +/// The two ends of a connector's base on `window`: centred on `to` (the point of the window +/// nearest the comment) along the side that faces `from`, as wide as a small part of the window +/// and kept on that side. At a corner, the side is the one the comment lies further beyond. +fn wedge_base(window: Rect, from: Pos2, to: Pos2) -> [Pos2; 2] { + let half = (window.width().min(window.height()) * 0.08).clamp(5.0, 18.0); + // `half` along [lo, hi] around `c`, moved in to stay on the side (a side shorter than the + // base gives all of it). + let spread = |c: f32, lo: f32, hi: f32| { + let h = half.min((hi - lo).max(0.0) / 2.0); + let c = c.clamp(lo + h, hi - h); + (c - h, c + h) + }; + let on_horizontal = (to.y - window.top()).abs() < 0.5 || (to.y - window.bottom()).abs() < 0.5; + let on_vertical = (to.x - window.left()).abs() < 0.5 || (to.x - window.right()).abs() < 0.5; + let beyond_x = (from.x - to.x).abs() / window.width().max(1.0); + let beyond_y = (from.y - to.y).abs() / window.height().max(1.0); + if on_horizontal && !(on_vertical && beyond_x > beyond_y) { + let (a, b) = spread(to.x, window.left(), window.right()); + [pos2(a, to.y), pos2(b, to.y)] + } else { + let (a, b) = spread(to.y, window.top(), window.bottom()); + [pos2(to.x, a), pos2(to.x, b)] } } diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index bd74a70faf..d6db68403b 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3199,11 +3199,20 @@ const BLUE_NOTE_WITH_OPEN_POP_UP: &[u8] = b"%PDF-1.7 trailer << /Root 1 0 R >> %%EOF"; -/// The line segments of colour `color` the last frame painted. -fn line_segments(h: &Harness<'static, PdfCraftApp>, color: egui::Color32) -> Vec<[Pos2; 2]> { - fn walk(shape: &egui::Shape, color: egui::Color32, out: &mut Vec<[Pos2; 2]>) { +/// What the connectors of comments in colour `color` painted in the last frame: each wedge +/// (its point, the two ends of its base, its colour: `color`, faded). +fn connectors(h: &Harness<'static, PdfCraftApp>, color: egui::Color32) -> Vec<(Pos2, [Pos2; 2], egui::Color32)> { + fn walk(shape: &egui::Shape, color: egui::Color32, out: &mut Vec<(Pos2, [Pos2; 2], egui::Color32)>) { match shape { - egui::Shape::LineSegment { points, stroke } if stroke.color == color => out.push(*points), + egui::Shape::Mesh(m) => { + if let [tip, b0, b1] = m.vertices.as_slice() + && tip.color.a() > 0 + && tip.color.a() < 255 + && tip.color.to_opaque() == color + { + out.push((tip.pos, [b0.pos, b1.pos], tip.color)); + } + } egui::Shape::Vec(shapes) => shapes.iter().for_each(|s| walk(s, color, out)), _ => {} } @@ -3215,10 +3224,11 @@ fn line_segments(h: &Harness<'static, PdfCraftApp>, color: egui::Color32) -> Vec out } -/// A comment's connector line (comment.connector-lines): in the comment's colour, from the -/// edge of the comment nearest its open pop-up to the edge of the window nearest that point, -/// shown while the pointer is on the comment or on the window or the comment is selected, and -/// not otherwise. +/// A comment's connector (comment.connector-lines): a faint wedge in the comment's colour from +/// the edge of the comment nearest its open pop-up to a base on the window's edge around the +/// point nearest the comment; shown +/// while the pointer is on the comment or on the window or the comment is selected, and not +/// otherwise. #[test] fn a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_either() { let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |_| {}); @@ -3229,27 +3239,33 @@ fn a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_e r.expand(1.0).contains(p) && [p.x - r.left(), r.right() - p.x, p.y - r.top(), r.bottom() - p.y].iter().any(|d| d.abs() < 1.0) }; - // Away from both: no line. + // Away from both: no connector. h.hover_at(at(&h, 280.0, 190.0)); h.run_steps(2); - assert!(line_segments(&h, blue).is_empty(), "no line with the pointer elsewhere"); + assert!(connectors(&h, blue).is_empty(), "no connector with the pointer elsewhere"); - // On the note or on its window: one line, from the note's edge to the window's. + // On the note or on its window: one wedge, from the note's edge to the window's. for (what, pointer) in [("the note", mark.center()), ("the window", window.center())] { h.hover_at(pointer); h.run_steps(2); - let lines = line_segments(&h, blue); - assert_eq!(lines.len(), 1, "one line with the pointer on {what}: {lines:?}"); - let [from, to] = lines[0]; - assert!(on_edge(mark, from), "starts on the note's edge {mark:?}: {from:?}"); - assert!(on_edge(window, to), "ends on the window's edge {window:?}: {to:?}"); - // The nearest points: the note's lower right corner, the window's upper left one. - assert!(from.distance(mark.right_bottom()) < 1.0 && to.distance(window.left_top()) < 1.0, "{from:?} -> {to:?}"); + let found = connectors(&h, blue); + assert_eq!(found.len(), 1, "one connector with the pointer on {what}: {found:?}"); + let (tip, [b0, b1], base) = found[0]; + assert!(on_edge(mark, tip), "its point is on the note's edge {mark:?}: {tip:?}"); + // The nearest point of the note: its lower right corner. + assert!(tip.distance(mark.right_bottom()) < 1.0, "{tip:?}"); + assert!(on_edge(window, b0) && on_edge(window, b1), "its base is on the window's edge {window:?}: {b0:?} {b1:?}"); + // The base lies along one side, around the window's upper left corner, and is wider than a line. + assert!((b0.x - b1.x).abs() < 0.5 || (b0.y - b1.y).abs() < 0.5, "along one side: {b0:?} {b1:?}"); + assert!(b0.distance(b1) >= 10.0, "a base, not a line: {b0:?} {b1:?}"); + assert!(b0.distance(window.left_top()).min(b1.distance(window.left_top())) < 1.0, "at the corner: {b0:?} {b1:?}"); + // The wedge is faint, in the comment's hue. + assert!(base.a() < 255 && base.a() > 0 && base.to_opaque() == blue, "the wedge is faint blue: {base:?}"); } - // Selected, the note keeps its line with the pointer elsewhere. + // Selected, the note keeps its connector with the pointer elsewhere. h.state_mut().views[0].comments.selected = Some((0, 0)); h.hover_at(at(&h, 280.0, 190.0)); h.run_steps(2); - assert_eq!(line_segments(&h, blue).len(), 1, "a selected comment shows its line"); + assert_eq!(connectors(&h, blue).len(), 1, "a selected comment shows its connector"); } diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index a120d1ad17..3c856087fb 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4148,7 +4148,7 @@ status = "shipped" commands = [] tools = [] evidence = ["crates/ui-egui/tests/it/comments.rs::a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_either"] -notes = "a line in the comment's colour from the comment to its open pop-up while the pointer is on either or the comment is selected (Acrobat's roll-over lines). Not yet: the preference to turn them off" +notes = "a faint wedge in the comment's colour from the comment's edge nearest its open pop-up, widening to the window's side that faces it, while the pointer is on either or the comment is selected (Acrobat's roll-over lines). Not yet: the preference to turn them off" [[feature]] id = "comment.review-history" From b1c2c4cafe37213d79c9357c1d08d3cd3154da96 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:15:42 +0800 Subject: [PATCH 11/19] M5: comment pop-up windows laid out as reply threads, fixed in size, scrolling and resizable (comment.popups) The pop-up window now reads as the common readers' do: - a one-line title bar with the comment's kind and author, its date and x, in the comment's colour; the body in a lighter tint, with fixed ink so it reads in either app theme; - the comment's text, and each reply's, in a box typed in place; - the replies as panels in thread order, each inside the panel of the reply it answers (/IRT), nesting at most eight deep; - a Reply button under the comment and each reply, opening a box that answers that comment or reply (Enter starts a new line; Post or Ctrl/Cmd+Enter posts; Cancel closes it). The window keeps the size of its pop-up's /Rect in points on screen as the page zooms, instead of growing with the zoom; the text and replies scroll inside it, with a scroll bar shown whenever there is more. Dragging its lower right corner resizes it; the new size is saved as the pop-up's /Rect, the window's upper left corner kept in place on screen (on a page turned a quarter too), in one undo step. A /Rect with non-finite numbers gives the least size. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- ROADMAP.md | 2 +- crates/ui-egui/src/canvas.rs | 2 +- crates/ui-egui/src/tools/comments/popups.rs | 468 ++++++++++++++++---- crates/ui-egui/src/tools/comments/state.rs | 4 + crates/ui-egui/tests/it/comments.rs | 178 +++++++- parity/acrobat-features.toml | 4 +- 6 files changed, 564 insertions(+), 94 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 0854b65f96..a7f274ea62 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -432,7 +432,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-15 (M3, #1248):** A fresh install follows the desktop's appearance: the theme preference defaults to System (dark on a dark desktop, Light where the desktop says nothing); saved settings and `--theme` still win. Dark page rendering is Reading colours, and the page reading themes are #1732. - **2026-10-15 (M3, #1767):** Restore last view settings when reopening documents, as Acrobat's preference does: a file opened again shows the page, zoom and page display it was last shown with (remembered for the last 100 files when a tab closes or settings are saved), over the file's own initial view; on by default, Preferences ▸ Documents and view turns it off. -- **2026-10-14 (M5, comment pop-ups):** A double-click on a comment opens its pop-up window on the page, as in Acrobat: author, date, the text edited in place, the replies; the title bar drags it and × closes it, each an undo step saved in the file's `/Popup` so other viewers agree. A connector line in the comment's colour joins the comment to its open window on roll-over or selection. `comment_popup` does the same headlessly and `comment_list` reports the pop-up. +- **2026-10-14 (M5, comment pop-ups; reworked 2026-11-01):** A double-click on a comment opens its pop-up window on the page: a title bar with the comment's kind, author and date; its text and its replies' typed in place, the replies as panels each inside the one it answers, a Reply button under each; the window keeps its `/Rect` size on screen as the page zooms, scrolls its thread, drags by its title bar, resizes by its corner and closes with ×, each an undo step saved in the file's `/Popup` so other viewers agree. A faint wedge in the comment's colour joins the comment to its open window on roll-over or selection. `comment_popup` does the same headlessly and `comment_list` reports the pop-up. - **2026-10-13 (M3, native-opening candidate):** Chosen tools survive preparation, conversion and password retries; startup intake waits for pending preparation. Regression tests cover completion, cancellation and document identity. Estimates unchanged. - **2026-10-13 (community, M3, #531 touchpad momentum):** On Wayland a two-finger swipe keeps gliding after the fingers lift and slows to a stop, measured from the gesture's last 0.1 s and decaying with a 0.35 s time constant through the #759 1:1 path. Any wheel input, click, key, zoom or edge ends it (fingers resting on the touchpad don't yet: the vendored winit binds no Wayland hold gestures); single-page paging, macOS (which sends its own momentum), the web, and Animation effects Off never glide. Unit tests for the physics and two headless view tests. Estimates unchanged. - **2026-10-13 (M3, native-open admission):** Later prepared replies stay queued while a modal or native picker owns the foreground. Picker completion, cancellation and abandonment wake retained opens. Regressions cover recipient ownership, exactly-once admission, source/history preservation and one-shot repaint requests. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 05a977d9cc..7d9187270b 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2732,7 +2732,7 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim if let Some(e) = comments::composer(ui.ctx(), view, info, prefs) { view.pending_edit = Some(e); } - if let Some(e) = comments::popups(ui.ctx(), view, info, comments_hidden, allowed) { + if let Some(e) = comments::popups(ui.ctx(), view, info, prefs, comments_hidden, allowed) { view.pending_edit = Some(e); } if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), view, info) { diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index db34e1098e..fc902708af 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -1,7 +1,5 @@ use super::state::PageCx; -use super::{ - Annotation, Color32, CommentPrefs, CornerRadius, DocInfo, DocView, Edit, PagePointer, PageXform, Pos2, QuickTool, Rect, Tokens, pos2, theme, -}; +use super::{Annotation, Color32, CommentPrefs, CornerRadius, DocInfo, DocView, Edit, PagePointer, PageXform, Pos2, QuickTool, Rect, pos2, theme}; /// 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`]). @@ -88,12 +86,257 @@ fn nearest_on(r: Rect, p: Pos2) -> Pos2 { } } -/// The open pop-up windows of the comments on screen: author, date and the comment's text, -/// editable in place; the title bar drags the window (its new place is saved with the comment); -/// × closes it. Returns the edits to apply (one, or a batch when a click both saves typed text -/// and closes). Hidden comments show none. -pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hidden: bool, allowed: bool) -> Option { - let t = Tokens::get(ctx); +/// A pop-up window's size on screen, in points, whatever the zoom (the readers keep theirs the +/// same as the page zooms), kept within these bounds whatever size the file gives it. +const MIN_WIDTH: f32 = 200.0; +const MAX_WIDTH: f32 = 800.0; +const MIN_HEIGHT: f32 = 120.0; +const MAX_HEIGHT: f32 = 800.0; +/// The side of the grip at a window's lower right corner that resizes it. +const RESIZE_GRIP: f32 = 10.0; +/// How far a reply's panel sits in from the panel around it. +const NEST: i8 = 6; +/// Past this depth a reply's panel stops nesting and sits under the one before it. +const MAX_NESTING: usize = 8; + +/// `replies` (all filed under comment `top`) in thread order, each with its depth: a reply to +/// `top` is 1, and each reply comes right after the one it answers, followed by its own replies. +/// One whose parent isn't among them (a status reply, say) answers `top`. A cycle of `/IRT` +/// references shows each reply once. +fn thread<'a>(top: (usize, usize), replies: &[&'a Annotation]) -> Vec<(&'a Annotation, usize)> { + use std::collections::{HashMap, HashSet}; + let known: HashSet<(usize, usize)> = replies.iter().map(|r| (r.page, r.index)).collect(); + let mut answered: HashMap<(usize, usize), Vec<&'a Annotation>> = HashMap::new(); + for r in replies { + let parent = r.parent.filter(|p| known.contains(p) && *p != (r.page, r.index)).unwrap_or(top); + answered.entry(parent).or_default().push(r); + } + let mut out = Vec::with_capacity(replies.len()); + let mut seen = HashSet::new(); + // Depth first, answers in document order: the stack holds them reversed. + let below = |key: (usize, usize), depth: usize| answered.get(&key).into_iter().flatten().rev().map(move |r| (*r, depth)); + let mut stack: Vec<(&Annotation, usize)> = below(top, 1).collect(); + let mut next = 0; + loop { + while let Some((r, depth)) = stack.pop() { + if !seen.insert((r.page, r.index)) { + continue; + } + out.push((r, depth)); + stack.extend(below((r.page, r.index), depth.saturating_add(1))); + } + // Replies only a cycle reaches: shown from their first, as if they answered `top`. The + // search resumes where it stopped, so the whole of it is one pass. + while next < replies.len() && replies.get(next).is_some_and(|r| seen.contains(&(r.page, r.index))) { + next += 1; + } + match replies.get(next) { + Some(r) => stack.push((*r, 1)), + None => break, + } + } + out +} + +/// A thread as nested panels: each reply ([`thread`] order) and the positions of the replies +/// drawn inside its panel; then the positions of the replies to the comment itself. Nesting stops +/// at [`MAX_NESTING`], so drawing it recurses no deeper than that. +fn nest<'a>(order: &[(&'a Annotation, usize)]) -> (Vec<(&'a Annotation, Vec)>, Vec) { + let mut nodes: Vec<(&Annotation, Vec)> = Vec::with_capacity(order.len()); + let mut roots = Vec::new(); + // The panels open at each depth above the reply being placed. + let mut open: Vec = Vec::new(); + for (i, (r, depth)) in order.iter().enumerate() { + let depth = (*depth).clamp(1, MAX_NESTING); + open.truncate(depth - 1); + match open.last().and_then(|p| nodes.get_mut(*p)) { + Some((_, kids)) => kids.push(i), + None => roots.push(i), + } + nodes.push((r, Vec::new())); + open.push(i); + } + (nodes, roots) +} + +/// A pop-up window's colours: the comment's colour, lightened for the body and the title bar, +/// and ink that reads on them. Fixed, not from the app's theme: the window is the comment's, as +/// it looks on paper. +#[derive(Clone, Copy)] +struct Palette { + edge: Color32, + title: Color32, + title_ink: Color32, + body: Color32, + panel_edge: Color32, + ink: Color32, + faint: Color32, +} + +fn palette(color: Color32) -> Palette { + // `color` mixed into white, `f` of the way. + let mix = |f: f32| { + let c = |v: u8| (255.0 + (f32::from(v) - 255.0) * f).round().clamp(0.0, 255.0) as u8; + Color32::from_rgb(c(color.r()), c(color.g()), c(color.b())) + }; + let title = mix(0.8); + let light = |c: Color32| 0.299 * f32::from(c.r()) + 0.587 * f32::from(c.g()) + 0.114 * f32::from(c.b()) > 150.0; + Palette { + edge: color, + title, + title_ink: if light(title) { Color32::from_gray(20) } else { Color32::WHITE }, + body: mix(0.35), + panel_edge: mix(0.7), + ink: Color32::from_gray(20), + faint: Color32::from_gray(80), + } +} + +/// What a pop-up window's parts share as they draw. +struct Parts<'v> { + allowed: bool, + author: &'v str, + pal: Palette, + cv: &'v mut super::state::CommentView, + edits: &'v mut Vec, +} + +/// One comment's or reply's text in its box, typed in place (saved when the box loses focus); +/// read-only when comments can't be changed or it is locked. +fn text_box(ui: &mut egui::Ui, parts: &mut Parts<'_>, a: &Annotation, name: &str) { + let key = (a.page, a.index); + if parts.allowed && !a.locked { + let typed = parts.cv.popup_text.as_ref().filter(|(p, i, _)| (*p, *i) == key).map(|(_, _, s)| s.clone()); + let mut text = typed.unwrap_or_else(|| a.contents.clone().unwrap_or_default()); + let field = ui + .add( + egui::TextEdit::multiline(&mut text) + .desired_rows(1) + .desired_width(f32::INFINITY) + .font(theme::regular(12.5)) + .text_color(parts.pal.ink) + .background_color(Color32::WHITE) + .margin(egui::Margin::symmetric(4, 2)) + .id_salt(("popup-text", a.page, a.index)), + ) + .accessible_name(name); + if field.changed() { + parts.cv.popup_text = Some((a.page, a.index, text.clone())); + } + if field.lost_focus() + && let Some((_, _, s)) = parts.cv.popup_text.take_if(|(p, i, _)| (*p, *i) == key) + && s.trim_end() != a.contents.as_deref().unwrap_or_default() + { + parts.edits.push(Edit::SetAnnotationContents { page: a.page, index: a.index, text: s.trim_end().to_string() }); + } + } else { + egui::Frame::NONE.fill(Color32::WHITE).inner_margin(egui::Margin::symmetric(4, 2)).show(ui, |ui| { + ui.set_width(ui.available_width()); + ui.label(egui::RichText::new(a.contents.as_deref().unwrap_or_default()).font(theme::regular(12.5)).color(parts.pal.ink)); + }); + } +} + +/// The row under a comment's or reply's text: a Reply button at its end (the start is kept for +/// more of a reply's controls), and the box to type that reply in once it is pressed (Post, or +/// Ctrl/⌘+Enter, posts it; Enter starts a new line, as in the readers' pop-ups). +fn footer(ui: &mut egui::Ui, parts: &mut Parts<'_>, a: &Annotation) { + let key = (a.page, a.index); + // Small buttons, as the readers' footers have. + ui.spacing_mut().button_padding = egui::vec2(6.0, 1.0); + let mut opened = false; + if parts.allowed { + ui.horizontal(|ui| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let label = egui::RichText::new(tl!("Reply")).font(theme::regular(11.0)).color(parts.pal.ink); + let button = egui::Button::new(label) + .fill(Color32::WHITE.gamma_multiply(0.6)) + .stroke(egui::Stroke::new(1.0, parts.pal.panel_edge)) + .min_size(egui::vec2(0.0, 16.0)); + if ui.add(button).clicked() { + parts.cv.popup_reply = Some((a.page, a.index, String::new())); + opened = true; + } + }); + }); + } + let Some(mut reply) = parts.cv.popup_reply.as_ref().filter(|(p, i, _)| (*p, *i) == key).map(|(_, _, s)| s.clone()) else { return }; + let field = ui.add( + egui::TextEdit::multiline(&mut reply) + .desired_rows(1) + .desired_width(f32::INFINITY) + .hint_text(tl!("Add a reply")) + .font(theme::regular(12.5)) + .text_color(parts.pal.ink) + .background_color(Color32::WHITE) + .margin(egui::Margin::symmetric(4, 2)) + .id_salt(("popup-reply", a.page, a.index)), + ); + if opened { + field.request_focus(); + } + if field.changed() { + parts.cv.popup_reply = Some((a.page, a.index, reply.clone())); + } + let enter = super::submit_key(ui, &field, false); + let ok = !reply.trim().is_empty(); + let (post, cancel) = ui + .horizontal(|ui| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let small = |s: &str| egui::RichText::new(s).font(theme::regular(11.0)); + let post = ui.add_enabled(ok, egui::Button::new(small(tl!("Post"))).min_size(egui::vec2(0.0, 16.0))).clicked(); + let cancel = ui.add(egui::Button::new(small(tl!("Cancel"))).min_size(egui::vec2(0.0, 16.0))).clicked(); + (post, cancel) + }) + .inner + }) + .inner; + if (post || enter) && ok { + parts.cv.popup_reply = None; + parts.edits.push(Edit::ReplyToAnnotation { page: a.page, index: a.index, text: reply.trim().to_string(), author: parts.author.to_string() }); + } else if cancel { + parts.cv.popup_reply = None; + } +} + +/// A reply's panel: who wrote it and when, its text and footer, and the panels of its own +/// replies inside it. +fn reply_panel(ui: &mut egui::Ui, parts: &mut Parts<'_>, nodes: &[(&Annotation, Vec)], at: usize) { + let Some((r, kids)) = nodes.get(at) else { return }; + let pal = parts.pal; + egui::Frame::NONE.stroke(egui::Stroke::new(1.0, pal.panel_edge)).corner_radius(CornerRadius::same(3)).inner_margin(egui::Margin::same(4)).show( + ui, + |ui| { + ui.set_width(ui.available_width()); + ui.spacing_mut().item_spacing.y = 3.0; + ui.horizontal(|ui| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if let Some(m) = &r.modified { + ui.label(egui::RichText::new(m).font(theme::regular(10.5)).color(pal.faint)); + } + ui.with_layout(egui::Layout::left_to_right(egui::Align::Center), |ui| { + let who = r.author.as_deref().unwrap_or("?"); + ui.add(egui::Label::new(egui::RichText::new(who).font(theme::semibold(11.5)).color(pal.ink)).truncate()); + }); + }); + }); + text_box(ui, parts, r, r.author.as_deref().unwrap_or("?")); + footer(ui, parts, r); + for kid in kids { + egui::Frame::NONE.inner_margin(egui::Margin { left: NEST, ..Default::default() }).show(ui, |ui| reply_panel(ui, parts, nodes, *kid)); + } + }, + ); +} + +/// The open pop-up windows of the comments on screen, laid out as the readers' are: a title bar +/// with the comment's kind and author, its date and ×; the comment's text, editable in place; its +/// replies as panels, each inside the one it answers; and a Reply button under each, which opens +/// a box to answer it. The window keeps its size on screen as the page zooms, and its text and +/// replies scroll inside it. The title bar drags the window and its lower right corner resizes +/// it, both saved with the comment; × closes it. Returns the edits to apply (one, or a batch when +/// a click both saves typed text and closes). Hidden comments show none. +pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, prefs: &CommentPrefs, hidden: bool, allowed: bool) -> Option { let pointer = ctx.input(|i| i.pointer.hover_pos()); let mut edits: Vec = Vec::new(); let mut rects = Vec::new(); @@ -108,40 +351,74 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hi let Some(p) = a.popup else { continue }; let key = (page, a.index); let placed = xf.user_rect(info, page, p.rect); - // The window keeps a readable size whatever the zoom: its place follows the page, - // its width stays between what a note needs and what fits. - let width = placed.width().clamp(220.0, 420.0); + // Its place follows the page; its size is the pop-up's `/Rect` in points on screen, + // or the size its corner is being dragged to. + // A `/Rect` with numbers that aren't finite gives the least size. + let side = |a: f32, b: f32| Some((b - a).abs()).filter(|v| v.is_finite()).unwrap_or(0.0); + let saved = egui::vec2(side(p.rect[0], p.rect[2]), side(p.rect[1], p.rect[3])); + let clamp = |s: egui::Vec2| egui::vec2(s.x.clamp(MIN_WIDTH, MAX_WIDTH), s.y.clamp(MIN_HEIGHT, MAX_HEIGHT)); + let resizing = view.comments.popup_resize.filter(|(rp, ri, _)| (*rp, *ri) == key).map(|(_, _, size)| size); + let size = clamp(resizing.unwrap_or(saved)); let dragging = view.comments.popup_drag.filter(|(dp, di, _)| (*dp, *di) == key).map(|(_, _, at)| at); let pos = dragging.unwrap_or(placed.left_top()); let id = egui::Id::unique(("comment-popup", view.id.0, page, a.index)); let color = a .color .map_or(Color32::from_rgb(0xFF, 0xD6, 0x5C), |c| Color32::from_rgb((c[0] * 255.0) as u8, (c[1] * 255.0) as u8, (c[2] * 255.0) as u8)); + let pal = palette(color); + // A screen rectangle on this page in user space, to save as the pop-up's `/Rect`. Only + // converts: the pointer and the redaction look are unused. + let to_user = |r: Rect| { + let cx = PageCx { + page, + xf: &xf, + ptr: &PagePointer::default(), + info, + tool: QuickTool::Select, + prefs: &CommentPrefs::default(), + allowed, + hidden, + redact_mark: ([0.0; 3], 1.0), + }; + cx.rect_to_user(r) + }; let area = egui::Area::new(id).order(egui::Order::Foreground).fixed_pos(pos).show(ctx, |ui| { - egui::Frame::popup(ui.style()).inner_margin(egui::Margin::ZERO).corner_radius(CornerRadius::same(6)).show(ui, |ui| { - ui.set_width(width); - // Title bar: the comment's colour, author and date; drag to move, × to close. - let bar = egui::Frame::NONE.fill(color.gamma_multiply(0.35)).inner_margin(egui::Margin::symmetric(10, 6)).show(ui, |ui| { - ui.set_width(width - 20.0); + let frame = egui::Frame::popup(ui.style()).inner_margin(egui::Margin::ZERO).corner_radius(CornerRadius::same(3)).fill(pal.body); + let window = frame.stroke(egui::Stroke::new(1.0, pal.edge)).show(ui, |ui| { + ui.set_width(size.x); + // The whole size, however short its thread. + ui.set_min_height(size.y); + ui.spacing_mut().item_spacing.y = 3.0; + // Title bar: the comment's kind and author, its date and ×, on one line; it + // drags the window. + let bar = egui::Frame::NONE.fill(pal.title).inner_margin(egui::Margin::symmetric(6, 3)).show(ui, |ui| { + ui.set_width(size.x - 12.0); ui.horizontal(|ui| { - let who = a.author.clone().unwrap_or_else(|| crate::comments_panel::kind_label(&a).to_string()); - ui.label(egui::RichText::new(who.as_str()).font(theme::semibold(13.0)).color(t.text)); - if let Some(m) = &a.modified { - ui.label(egui::RichText::new(m).font(theme::regular(11.0)).color(t.text_faint)); - } ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { // Closing changes the comment's /Open, which needs comment permission. - if ui.add_enabled_ui(allowed, |ui| crate::icons::button(ui, "x", 18.0, false, tl!("Close"))).inner.clicked() { + if ui.add_enabled_ui(allowed, |ui| crate::icons::button(ui, "x", 14.0, false, tl!("Close"))).inner.clicked() { edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: None, open: Some(false) }); } - }); + if let Some(m) = &a.modified { + ui.label(egui::RichText::new(m).font(theme::regular(10.5)).color(pal.title_ink)); + } + // The kind and author take the rest of the line, cut short to fit. + ui.with_layout(egui::Layout::left_to_right(egui::Align::Center), |ui| { + let kind = tl!(crate::comments_panel::kind_label(&a)); + let title = match a.author.as_deref() { + Some(who) => format!("{kind} – {who}"), + None => kind.to_string(), + }; + ui.add(egui::Label::new(egui::RichText::new(title).font(theme::semibold(12.0)).color(pal.title_ink)).truncate()); + }); + }) }); }); // The bar drags the window, except where the × is: a region registered after // the button would take its clicks. let bar_rect = bar.response.rect; let grip = ui.interact( - Rect::from_min_max(bar_rect.min, pos2(bar_rect.right() - 30.0, bar_rect.bottom())), + Rect::from_min_max(bar_rect.min, pos2(bar_rect.right() - 24.0, bar_rect.bottom())), id.with("grip"), egui::Sense::drag(), ); @@ -159,72 +436,91 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, hi if grip.drag_stopped() && let Some((_, _, at)) = view.comments.popup_drag.take() && allowed + && at.distance(placed.left_top()) >= 2.0 { - // The same size on the page, at the new place. - let moved = Rect::from_min_size(at, placed.size()); - // Only converts the rectangle to user space: the pointer and the - // redaction look are unused. - let cx = PageCx { - page, - xf: &xf, - ptr: &PagePointer::default(), - info, - tool: QuickTool::Select, - prefs: &CommentPrefs::default(), - allowed, - hidden, - redact_mark: ([0.0; 3], 1.0), - }; - let rect = cx.rect_to_user(moved); - if at.distance(placed.left_top()) >= 2.0 { - edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: Some(rect), open: None }); - } + // The same size on the page, at the new place. Only converts the + // rectangle to user space: the pointer and the redaction look are unused. + let rect = to_user(Rect::from_min_size(at, placed.size())); + edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: Some(rect), open: None }); } - // Body: the text, typed in place; saved when the field loses focus. - egui::Frame::NONE.inner_margin(egui::Margin::symmetric(10, 8)).show(ui, |ui| { - ui.set_width(width - 20.0); - if allowed && !a.locked { - let typed = view.comments.popup_text.as_ref().filter(|(p, i, _)| (*p, *i) == key).map(|(_, _, s)| s.clone()); - let mut text = typed.unwrap_or_else(|| a.contents.clone().unwrap_or_default()); - let field = ui.add( - egui::TextEdit::multiline(&mut text) - .hint_text(tl!("Add a comment")) - .desired_rows(3) - .desired_width(f32::INFINITY) - .id_salt(("popup-text", page, a.index)), - ); - if field.changed() { - view.comments.popup_text = Some((page, a.index, text.clone())); + // Replies, as the thread reads: text replies only, filed under this + // comment by `/IRT` reference as in the Comments panel, each under the + // one it answers. 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.state.is_none() && r.parent.is_some_and(|parent| tops.get(&(r.page, r.index)).copied().unwrap_or(parent) == key) + }) + .collect(); + let order = thread(key, &replies); + let (nodes, roots) = nest(&order); + let mut parts = Parts { allowed, author: &prefs.author, pal, cv: &mut view.comments, edits: &mut edits }; + // The body keeps the window's height: the text and replies scroll in what the + // title bar leaves, with a bar that shows whenever there is more (egui's show + // only near the pointer), in the ink's colour. + egui::Frame::NONE.inner_margin(egui::Margin::same(4)).show(ui, |ui| { + ui.set_width(size.x - 8.0); + ui.spacing_mut().scroll = egui::style::ScrollStyle { + dormant_handle_opacity: 0.4, + dormant_background_opacity: 0.1, + foreground_color: true, + ..egui::style::ScrollStyle::solid() + }; + ui.visuals_mut().override_text_color = Some(pal.ink); + let body_height = (size.y - bar_rect.height() - 8.0).max(24.0); + let scroll = + egui::ScrollArea::vertical().id_salt(("popup-body", page, a.index)).max_height(body_height).auto_shrink([false, false]); + scroll.show(ui, |ui| { + ui.spacing_mut().item_spacing.y = 3.0; + text_box(ui, &mut parts, &a, tl!("Add a comment")); + footer(ui, &mut parts, &a); + for root in &roots { + reply_panel(ui, &mut parts, &nodes, *root); } - if field.lost_focus() - && let Some((_, _, s)) = view.comments.popup_text.take_if(|(p, i, _)| (*p, *i) == key) - && s.trim_end() != a.contents.as_deref().unwrap_or_default() - { - edits.push(Edit::SetAnnotationContents { page, index: a.index, text: s.trim_end().to_string() }); - } - } else { - ui.label(a.contents.as_deref().unwrap_or_default()); - } - // 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.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); - ui.horizontal_wrapped(|ui| { - ui.label(egui::RichText::new(r.author.as_deref().unwrap_or("?")).font(theme::semibold(12.0))); - ui.label(egui::RichText::new(r.contents.as_deref().unwrap_or_default()).font(theme::regular(12.0))); - }); - } + }); }); }); + // The lower right corner resizes the window; its new size is saved with the + // comment, its upper left corner where it was, when the drag ends. + let corner = window.response.rect.right_bottom(); + let grip = + ui.interact(Rect::from_min_max(corner - egui::vec2(RESIZE_GRIP, RESIZE_GRIP), corner), id.with("resize"), egui::Sense::drag()); + if grip.hovered() || grip.dragged() { + ui.ctx().set_cursor_icon(egui::CursorIcon::ResizeNwSe); + } + // Two short diagonal strokes mark the grip. + for inset in [3.0, 7.0] { + let stroke = egui::Stroke::new(1.0, pal.faint); + ui.painter().line_segment([corner - egui::vec2(3.0, inset), corner - egui::vec2(inset, 3.0)], stroke); + } + if grip.drag_started() { + view.comments.popup_resize = Some((page, a.index, size)); + } + if grip.dragged() + && let Some((_, _, s)) = view.comments.popup_resize.as_mut() + { + *s += grip.drag_delta(); + } + if grip.drag_stopped() + && let Some((_, _, s)) = view.comments.popup_resize.take() + && allowed + { + let s = clamp(s); + // A press on the grip that ends where it began changes nothing. + // The window's size in points is the `/Rect`'s, its upper left corner kept + // where it is on screen: the rectangle of that size on the page there. A page + // turned a quarter (its `/Rotate` and the view's) shows the `/Rect`'s width + // down the screen. + let zoom = xf.zoom(); + if (s - clamp(saved)).length() >= 2.0 && zoom.is_finite() && zoom > 0.0 { + let turn = info.pages.get(page).map_or(0, |pg| u32::from(pg.rotation)) + u32::from(xf.rot); + let on_page = if turn % 180 == 90 { egui::vec2(s.y, s.x) } else { s } * zoom; + let rect = to_user(Rect::from_min_size(placed.left_top(), on_page)); + edits.push(Edit::SetAnnotationPopup { page, index: a.index, rect: Some(rect), open: None }); + } + } }); let shown_at = area.response.rect; if pointer.is_some_and(|p| shown_at.contains(p)) { diff --git a/crates/ui-egui/src/tools/comments/state.rs b/crates/ui-egui/src/tools/comments/state.rs index bc1564aad9..32ef995f8f 100644 --- a/crates/ui-egui/src/tools/comments/state.rs +++ b/crates/ui-egui/src/tools/comments/state.rs @@ -142,8 +142,12 @@ pub struct CommentView { /// A pop-up window's title bar being dragged: the comment, and where the window's top-left /// corner is now on screen. pub popup_drag: Option<(usize, usize, Pos2)>, + /// A pop-up window's corner being dragged: the comment, and the window's size on screen now. + pub popup_resize: Option<(usize, usize, egui::Vec2)>, /// Text being typed in a pop-up window: (page, index, text). pub popup_text: Option<(usize, usize, String)>, + /// A reply being typed in a pop-up window: the comment or reply it answers (page, index), and its text. + pub popup_reply: Option<(usize, usize, String)>, /// Where each open pop-up was drawn last frame, for the connector lines and hit-testing. pub popup_rects: Vec<((usize, usize), Rect)>, /// The pop-up the pointer was over last frame. diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index d6db68403b..e2f6b0412b 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3085,7 +3085,7 @@ trailer << /Root 1 0 R >> }); h.run_steps(6); let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the open pop-up shows"); - let in_window = |label: &str| h.query_all_by_label(label).any(|n| window.contains(n.rect().center())); + let in_window = |text: &str| h.query_all_by(|n| shows_text(n, text)).any(|n| window.contains(n.rect().center())); assert!(in_window("Agreed with this"), "the text reply"); assert!(!in_window("Accepted set by Bob"), "not the review status"); assert!(!in_window("Marked set by Ada"), "not the checkmark"); @@ -3134,7 +3134,8 @@ fn a_double_click_opens_a_notes_pop_up_which_drags_and_closes() { assert_eq!(note.popup.map(|p| p.open), Some(true), "the pop-up opened"); assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Open comment pop-up")); let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is on screen"); - h.get_by_label("Tester"); + // Its title names the kind of comment and its author. + h.get_by_label("Note – Tester"); // Drag the title bar 80 px right and 40 px down: the pop-up's place on the page moves. let before = note.popup.unwrap().rect; @@ -3165,7 +3166,7 @@ const POPUPS_SHARING_A_NAME: &[u8] = b"%PDF-1.7 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 +7 0 obj << /Type /Annot /Subtype /Popup /Rect [180 -250 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 >> @@ -3183,7 +3184,7 @@ fn a_pop_up_lists_its_replies_by_reference_when_names_repeat() { 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(); + h.query_all_by(|n| shows_text(n, 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"); } @@ -3269,3 +3270,172 @@ fn a_connector_line_joins_a_comment_to_its_open_pop_up_while_the_pointer_is_on_e h.run_steps(2); assert_eq!(connectors(&h, blue).len(), 1, "a selected comment shows its connector"); } + +/// A note with an open pop-up and a thread of replies: "Second" and "Third" answer the note, +/// "Second, first answer" answers "Second" (`/IRT` to a reply). +const NOTE_WITH_A_THREAD: &[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] >> endobj +4 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /C [1 0.85 0.2] /T (Alpha) /Contents (First) /Popup 5 0 R /Open true >> endobj +5 0 obj << /Type /Annot /Subtype /Popup /Rect [60 -250 280 150] /Parent 4 0 R /Open true >> endobj +6 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /T (Beta) /Contents (Second) /IRT 4 0 R /F 30 >> endobj +7 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /T (Beta) /Contents (Second, first answer) /IRT 6 0 R /F 30 >> endobj +8 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /T (Beta) /Contents (Third) /IRT 4 0 R /F 30 >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// A pop-up window (comment.popup-windows) is laid out as the readers' are: a title bar with the +/// comment's kind and author; the comment's text; its replies as panels in thread order, an +/// answer to a reply inside that reply's panel; and a Reply button under each, which opens a box +/// that posts an answer to that comment or reply. +#[test] +fn a_pop_up_threads_its_replies_and_takes_a_new_one() { + let mut h = harness_with_bytes(NOTE_WITH_A_THREAD.to_vec(), |_| {}); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + h.get_by_label("Note – Alpha"); + // Each reply's text box, inside the window. + let rect = |h: &Harness<'static, PdfCraftApp>, text: &str| { + let found: Vec = h.query_all_by(|n| shows_text(n, text)).map(|n| n.rect()).filter(|r| window.contains_rect(*r)).collect(); + assert_eq!(found.len(), 1, "{text:?} once in the window: {found:?}"); + found[0] + }; + let (second, answer, third) = (rect(&h, "Second"), rect(&h, "Second, first answer"), rect(&h, "Third")); + assert!(second.bottom() < answer.top() && answer.bottom() < third.top(), "thread order: {second:?} {answer:?} {third:?}"); + assert!( + answer.left() > second.left() + 4.0 && answer.right() < second.right() - 4.0, + "the answer sits inside its reply's panel: {second:?} {answer:?}" + ); + assert!((third.left() - second.left()).abs() < 1.0, "replies to the note line up: {second:?} {third:?}"); + // The Reply buttons, top to bottom: the note's, then each reply's. + let replies = |h: &Harness<'static, PdfCraftApp>| { + let mut b: Vec = + h.query_all_by(|n| n.role() == Role::Button && n.label().as_deref() == Some("Reply")).map(|n| n.rect()).collect(); + b.sort_by(|x, y| x.top().total_cmp(&y.top())); + b + }; + assert_eq!(replies(&h).len(), 4, "one under the note and each reply"); + + // The note's Reply: Enter starts a new line in the box, as in the readers' pop-ups; Post posts. + let before = comments(&h).len(); + let at = replies(&h)[0].center(); + click(&mut h, at); + field(&h, "Add a reply").type_text("Fourth"); + h.run_steps(2); + h.key_press(egui::Key::Enter); + h.run_steps(2); + assert_eq!(comments(&h).len(), before, "Enter does not post"); + h.event(egui::Event::Text("line two".into())); + h.run_steps(2); + h.get_by_label("Post").click(); + h.run_steps(3); + let all = comments(&h); + let note = all.iter().find(|a| a.contents.as_deref() == Some("First")).expect("the note"); + let reply = all.iter().find(|a| a.contents.as_deref() == Some("Fourth\nline two")).expect("the reply was added, on two lines"); + assert_eq!(reply.parent, Some((note.page, note.index))); + assert_eq!(reply.author.as_deref(), Some("Tester")); + assert!(h.state().views[0].comments.popup_reply.is_none(), "the box closes"); + + // A reply's own Reply answers that reply; Ctrl+Enter posts too, as in the comment composer. + let second_key = all.iter().find(|a| a.contents.as_deref() == Some("Second")).map(|a| (a.page, a.index)).expect("the reply"); + let at = replies(&h)[1].center(); + click(&mut h, at); + field(&h, "Add a reply").type_text("Fifth"); + h.run_steps(2); + h.key_press_modifiers(egui::Modifiers::CTRL, egui::Key::Enter); + h.run_steps(3); + let fifth = comments(&h).into_iter().find(|a| a.contents.as_deref() == Some("Fifth")).expect("Ctrl+Enter posts"); + assert_eq!(fifth.parent, Some(second_key), "it answers the reply"); + + // Cancel closes the box without posting. + let before = comments(&h).len(); + let at = replies(&h)[0].center(); + click(&mut h, at); + field(&h, "Add a reply").type_text("Never mind"); + h.run_steps(2); + h.get_by_label("Cancel").click(); + h.run_steps(3); + assert_eq!(comments(&h).len(), before); + assert!(h.state().views[0].comments.popup_reply.is_none()); +} + +/// A note with an open pop-up and `replies` replies to it. +fn note_with_replies(replies: usize) -> Vec { + let annots: String = (0..replies + 2).map(|i| format!("{} 0 R ", i + 4)).collect(); + let mut pdf = format!( + "%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 [{annots}] >> endobj +4 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /T (Alpha) /Contents (First) /Popup 5 0 R /Open true >> endobj +5 0 obj << /Type /Annot /Subtype /Popup /Rect [60 10 280 150] /Parent 4 0 R /Open true >> endobj +" + ); + for i in 0..replies { + pdf.push_str(&format!( + "{} 0 obj << /Type /Annot /Subtype /Text /Rect [20 160 40 180] /T (Beta) /Contents (Reply {i}) /IRT 4 0 R /F 30 >> endobj\n", + i + 6 + )); + } + pdf.push_str("trailer << /Root 1 0 R >>\n%%EOF"); + pdf.into_bytes() +} + +/// A pop-up window keeps its height however long its thread grows: the text and replies scroll +/// inside it, as in the readers' pop-ups, rather than the window growing down the screen. +#[test] +fn a_long_thread_scrolls_inside_its_pop_up() { + let height = |replies: usize| { + let h = harness_with_bytes(note_with_replies(replies), |_| {}); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + window.height() + }; + let (short, long) = (height(1), height(25)); + assert!((short - long).abs() < 2.0, "the same height with 1 reply and with 25: {short} {long}"); + assert!(long < 600.0, "within the screen: {long}"); +} + +/// Dragging a pop-up window's lower right corner resizes it, and the new size is saved with the +/// comment (the pop-up's `/Rect`) at the same place, in one undo step; the window shows it. +#[test] +fn a_pop_ups_corner_resizes_it() { + let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |_| {}); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + let before = comments(&h).into_iter().find(|a| a.subtype == "Text").and_then(|a| a.popup).expect("its pop-up").rect; + let corner = window.right_bottom() - egui::vec2(3.0, 3.0); + drag(&mut h, corner, corner + egui::vec2(80.0, 60.0)); + h.run_steps(3); + let after = comments(&h).into_iter().find(|a| a.subtype == "Text").and_then(|a| a.popup).expect("its pop-up").rect; + // Wider and taller on the page, its upper left corner where it was. + assert!(after[2] - after[0] > before[2] - before[0] + 5.0, "wider: {before:?} -> {after:?}"); + assert!(after[3] - after[1] > before[3] - before[1] + 5.0, "taller: {before:?} -> {after:?}"); + assert!((after[0] - before[0]).abs() < 0.5 && (after[3] - before[3]).abs() < 0.5, "the same place: {before:?} -> {after:?}"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Move comment pop-up")); + let (_, resized) = h.state().views[0].comments.popup_rects.first().copied().expect("still open"); + // The window shows the saved size, in points on screen whatever the zoom. + assert!((resized.width() - (after[2] - after[0])).abs() < 3.0, "as wide as saved: {after:?} {resized:?}"); + assert!((resized.height() - (after[3] - after[1])).abs() < 3.0, "as tall as saved: {after:?} {resized:?}"); + assert!(resized.width() > window.width() + 60.0 && resized.height() > window.height() + 40.0, "bigger on screen: {window:?} -> {resized:?}"); + assert!((resized.left_top() - window.left_top()).length() < 1.0, "in the same place on screen"); +} + +/// A pop-up window's box holding `text` (a comment's or a reply's). +fn shows_text(n: &egui_kittest::kittest::AccessKitNode<'_>, text: &str) -> bool { + n.role() == Role::MultilineTextInput && n.value().as_deref() == Some(text) +} + +/// On a page turned a quarter (View ▸ Rotate View), a resized pop-up window stays where it is on +/// screen and shows the size it was dragged to. +#[test] +fn a_pop_up_resized_on_a_turned_page_stays_in_place() { + let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |app| app.views[0].rotation = 90); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + let corner = window.right_bottom() - egui::vec2(3.0, 3.0); + drag(&mut h, corner, corner + egui::vec2(80.0, 60.0)); + h.run_steps(3); + let (_, resized) = h.state().views[0].comments.popup_rects.first().copied().expect("still open"); + assert!((resized.left_top() - window.left_top()).length() < 1.0, "in the same place on screen: {window:?} -> {resized:?}"); + assert!((resized.width() - window.width() - 80.0).abs() < 3.0, "80 px wider: {window:?} -> {resized:?}"); + // Taller by the drag, less what its text had already stretched it. + assert!(resized.height() > window.height() + 50.0, "taller: {window:?} -> {resized:?}"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 3c856087fb..b87804573a 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4135,8 +4135,8 @@ milestone = "M5" status = "shipped" commands = [] tools = ["comment_popup", "comment_list"] -evidence = ["crates/ui-egui/tests/it/comments.rs::a_double_click_opens_a_notes_pop_up_which_drags_and_closes", "crates/annot/src/tests.rs::pop_ups_open_close_and_move_and_are_reported", "crates/automation/tests/it/automation.rs::comment_popups_are_opened_moved_and_listed", "crates/render/src/inspect/tests.rs::a_comments_pop_up_is_read_with_its_place_and_state"] -notes = "double-click a comment to open its pop-up window on the page (its /Popup /Open and /Rect, so other viewers show it the same); author, date, the text typed in place and the replies; the title bar drags it, × closes it; each is an undo step. Not yet: Acrobat's pop-up options menu (reply, status, delete from the window), replying from the window, resizing the window, Preferences ▸ Commenting pop-up behaviours (open on roll-over, hide when the Comments list opens)" +evidence = ["crates/ui-egui/tests/it/comments.rs::a_double_click_opens_a_notes_pop_up_which_drags_and_closes", "crates/ui-egui/tests/it/comments.rs::a_pop_up_threads_its_replies_and_takes_a_new_one", "crates/ui-egui/tests/it/comments.rs::a_long_thread_scrolls_inside_its_pop_up", "crates/ui-egui/tests/it/comments.rs::a_pop_ups_corner_resizes_it", "crates/ui-egui/tests/it/comments.rs::a_pop_up_resized_on_a_turned_page_stays_in_place", "crates/annot/src/tests.rs::pop_ups_open_close_and_move_and_are_reported", "crates/automation/tests/it/automation.rs::comment_popups_are_opened_moved_and_listed", "crates/render/src/inspect/tests.rs::a_comments_pop_up_is_read_with_its_place_and_state"] +notes = "double-click a comment to open its pop-up window on the page (its /Popup /Open and /Rect, so other viewers show it the same), at the size its /Rect gives in points whatever the zoom; a title bar with the comment's kind, author and date; its text typed in place; its replies as panels, each inside the one it answers (/IRT), their text typed in place too; a Reply button under the comment and under each reply opens a box that answers it (Enter starts a new line, Post or Ctrl+Enter posts); the text and replies scroll inside the window; the title bar drags it, its lower right corner resizes it, × closes it; each is an undo step. Not yet: the window's options menu (status, mark, lock, delete from the window), collapsing a reply's answers, resizing from the other corners and sides, Preferences ▸ Commenting pop-up behaviours (open on roll-over, hide when the Comments list opens)" [[feature]] id = "comment.connector-lines" From f508fdf053c4f3d66aeeb79bd6bb79668945edf7 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 10:22:55 +0800 Subject: [PATCH 12/19] M5: pop-up rectangles read alike, edits in one frame kept, pop-up drafts follow their comment (#2137 review) - The render path reads a pop-up as none unless its /Rect is four numbers, as pdfcraft_annot::summaries does; set_popup gives a comment's own pop-up without one the place a new pop-up gets, so opening it shows it again. - The composer's edit and a pop-up's, landing in one frame, are queued together in one batch instead of the pop-up's replacing the composer's. - A pop-up's unsaved text, reply draft, drag and resize follow their comment when a deletion moves it, are dropped on undo/redo, and are dropped after any other edit that leaves their (page, index) naming another comment (pages moved, inserted or deleted). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/annot/src/info.rs | 2 +- crates/annot/src/modify.rs | 48 ++++++++------- crates/annot/src/tests.rs | 20 +++++++ crates/render/src/inspect/annotations.rs | 12 ++-- crates/render/src/inspect/tests.rs | 12 ++++ crates/ui-egui/src/canvas.rs | 7 ++- crates/ui-egui/src/editing.rs | 13 ++++ crates/ui-egui/src/tools/comments/state.rs | 45 +++++++++++++- crates/ui-egui/tests/it/comments.rs | 70 ++++++++++++++++++++++ 9 files changed, 201 insertions(+), 28 deletions(-) diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index 6a90541437..18a395a848 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -70,7 +70,7 @@ pub(crate) fn own_popup(doc: &Document, own: Option, annot: &Dict) -> Op /// The comment's own pop-up (see [`own_popup`]), as [`PopupInfo`]: `None` when it has none or it /// is unreadable. -fn popup_info(doc: &Document, own: Option, annot: &Dict) -> Option { +pub(crate) fn popup_info(doc: &Document, own: Option, annot: &Dict) -> Option { // Another annotation's rectangle and open state aren't this comment's (review on #2137). let r = own_popup(doc, own, annot)?; let p = doc.get(r); diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index 8e5a297ab3..7a1071c306 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -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::{is_comment_subtype, line_endings_of, own_popup, text_value}; +use super::info::{is_comment_subtype, line_endings_of, own_popup, popup_info, text_value}; use super::lookup::{annot_dict, annot_ref, annotation_flags, annots, is_fill_sign, 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, @@ -310,30 +310,36 @@ pub fn set_popup(doc: &mut Document, page: usize, index: usize, rect: Option<[f6 None => None, }; let _ = meta; + // Where a new pop-up goes: to the right of the comment, as new notes' do. + let own: Vec = d + .get(b"Rect") + .map(|o| doc.resolve(o)) + .and_then(|o| o.as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(|v| doc.resolve(v).as_f64()) + .collect(); + let beside = match own[..] { + [_, _, x1, y1] if finite(&own) => [x1 + 10.0, y1 - 120.0, x1 + 210.0, y1], + _ => [0.0, 0.0, 200.0, 120.0], + }; // Only a pop-up that is this comment's: `/Popup` naming another annotation (or a pop-up // whose `/Parent` is another comment) would move or open that one, so it gets its own. match own_popup(doc, Some(r), &d) { - Some(pr) => doc.update_dict(pr, |p| { - if let Some(rect) = rect { - p.set(b"Rect".to_vec(), num_array(&rect)); - } - if let Some(open) = open { - p.set(b"Open".to_vec(), Object::Bool(open)); - } - })?, + Some(pr) => { + // One without a `/Rect` of four numbers reads as no pop-up (`summaries`), so it gets + // the new pop-up's place: opening it then shows it. + let rect = rect.or_else(|| popup_info(doc, Some(r), &d).is_none().then_some(beside)); + doc.update_dict(pr, |p| { + if let Some(rect) = rect { + p.set(b"Rect".to_vec(), num_array(&rect)); + } + if let Some(open) = open { + p.set(b"Open".to_vec(), Object::Bool(open)); + } + })? + } None => { - let own: Vec = d - .get(b"Rect") - .map(|o| doc.resolve(o)) - .and_then(|o| o.as_array().cloned()) - .unwrap_or_default() - .iter() - .filter_map(|v| doc.resolve(v).as_f64()) - .collect(); - let beside = match own[..] { - [_, _, x1, y1] if finite(&own) => [x1 + 10.0, y1 - 120.0, x1 + 210.0, y1], - _ => [0.0, 0.0, 200.0, 120.0], - }; let mut pd = Dict::new(); pd.set(b"Type".to_vec(), Object::name("Annot")); pd.set(b"Subtype".to_vec(), Object::name("Popup")); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index e20f001a65..4e45d1d777 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2300,3 +2300,23 @@ fn foreign_pop_ups_are_not_the_marks(doc: &mut Document, note: ObjRef, mark: usi doc.update_dict(mr, |m| m.set(b"Popup".to_vec(), Object::Ref(orphan))).unwrap(); assert_eq!(popup_of_mark(doc), Some(PopupInfo { rect: [0.0, 0.0, 50.0, 50.0], open: false }), "no /Parent"); } + +/// #2137 review: a comment's own pop-up without a `/Rect` of four numbers reads as none; opening +/// it gives it the place a new pop-up gets, so the window shows again. +#[test] +fn opening_a_pop_up_without_a_rect_gives_it_one() { + let mut doc = fixture(); + let note = add_annotation(&mut doc, &new(0, Shape::Note { at: [20.0, 280.0], icon: NoteIcon::Comment }), &meta("n1")).unwrap(); + let (_, r) = annot_ref(&mut doc, 0, note).unwrap(); + let pr = annot_dict(&doc, r).reference(b"Popup").expect("new notes have a pop-up"); + doc.update_dict(pr, |p| { + p.remove(b"Rect"); + }) + .unwrap(); + assert_eq!(summaries(&doc).into_iter().find(|s| s.index == note).unwrap().popup, None, "no /Rect reads as no pop-up"); + set_popup(&mut doc, 0, note, None, Some(true), &meta("n1")).unwrap(); + let popup = summaries(&doc).into_iter().find(|s| s.index == note).unwrap().popup.expect("the pop-up reads again"); + assert!(popup.open); + assert!(popup.rect[2] - popup.rect[0] >= 100.0 && popup.rect[3] - popup.rect[1] >= 100.0, "a window's size: {:?}", popup.rect); + assert_eq!(annot_dict(&doc, r).reference(b"Popup"), Some(pr), "the same pop-up, not a new one"); +} diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 71e95024ab..36e0556f65 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -166,12 +166,16 @@ impl<'a> Inspector<'a> { && p.get(b"Parent") .map_or(true, |parent| parent.as_reference().is_ok() && self.resolved_id(parent).is_some_and(|id| Some(id) == own)) }; - let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).map(|p| { - let [x0, y0, x1, y1] = rect4(self.resolve(p.get(b"Rect").unwrap_or(&Object::Null))); - Popup { + // A pop-up without a `/Rect` of four numbers is none, as `pdfcraft_annot::summaries` + // reads it: both lists of the comments agree. + let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).and_then(|p| { + let rect = self.resolve(p.get(b"Rect").ok()?).as_array().ok()?; + let nums: Vec = rect.iter().filter_map(|v| self.resolve(v).as_float().ok()).collect(); + let [x0, y0, x1, y1] = nums[..] else { return None }; + Some(Popup { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open: p.get(b"Open").ok().and_then(|o| self.resolve(o).as_bool().ok()).unwrap_or(false), - } + }) }); let annotation = Annotation { page, diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 94794b698b..62bc130290 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1904,3 +1904,15 @@ trailer << /Root 1 0 R >> assert!((p.width - 3874.96).abs() < 0.1, "width expected ~3874.96, got {}", p.width); assert!((p.height - 5493.54).abs() < 0.1, "height expected ~5493.54, got {}", p.height); } + +/// #2137 review: a pop-up without a `/Rect` of four numbers reads as no pop-up, as +/// `pdfcraft_annot::summaries` reads it, so the two lists of comments agree. +#[test] +fn a_pop_up_without_a_four_number_rect_is_none() { + let bytes = "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] /Annots [4 0 R 5 0 R] >> endobj\n4 0 obj << /Type /Annot /Subtype /Text /Rect [20 260 40 280] /Contents (Hi) /Popup 5 0 R >> endobj\n5 0 obj << /Type /Annot /Subtype /Popup RECT /Parent 4 0 R /Open true >> endobj\ntrailer << /Root 1 0 R >>\n%%EOF\n"; + for (rect, read) in [("", false), ("/Rect [0 0 50]", false), ("/Rect (box)", false), ("/Rect [0 0 50 /x]", false), ("/Rect [0 0 50 60]", true)] { + let info = inspect(Arc::new(bytes.replace("RECT", rect).into_bytes()), None).expect("opens"); + let note = info.annotations.iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.popup.is_some(), read, "{rect:?}"); + } +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 7d9187270b..5c5dd611f8 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2733,7 +2733,12 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim view.pending_edit = Some(e); } if let Some(e) = comments::popups(ui.ctx(), view, info, prefs, comments_hidden, allowed) { - view.pending_edit = Some(e); + // A click on the composer's Post also takes the focus from a pop-up's box, saving what + // was typed there: both edits land in this frame, so both are kept, in one batch. + view.pending_edit = Some(match view.pending_edit.take() { + Some(first) => Edit::Batch { label: "Edit comment".into(), edits: vec![first, e] }, + None => e, + }); } if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), view, info) { view.pending_edit = Some(e); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index f1c9ba7949..71b6beb030 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -123,6 +123,15 @@ impl PdfCraftApp { (Edit::DeleteAnnotation { page, .. }, Some(d)) => d.info.annotations.iter().filter(|a| a.page == *page).cloned().collect(), _ => Vec::new(), }; + // The comments the pop-ups hold unsaved text or a drag for, to see afterwards whether + // their places still name them (pages may move, come or go). + let popups_before: Vec = match (self.views.get(i), self.session.get(id)) { + (Some(v), Some(d)) => { + let keys = v.comments.popup_keys(); + d.info.annotations.iter().filter(|a| keys.contains(&(a.page, a.index))).cloned().collect() + } + _ => Vec::new(), + }; match self.session.apply(id, edit.clone()) { Ok(()) => { let Some(doc) = self.session.get(id) else { return true }; @@ -169,6 +178,10 @@ impl PdfCraftApp { if matches!(edit, Edit::AddAnnotation(_)) { view.comments.comment_added(); } + // A deletion moves the pop-ups' state with its comments (`annotation_deleted`). + if !matches!(edit, Edit::DeleteAnnotation { .. }) { + view.comments.popups_after_edit(&popups_before, &info.annotations); + } // Keep the pages the user acted on selected, where they now are. select_after_edit(view, edit, info, &annots_before, self.quick_tool); if std::mem::take(&mut view.comments.tool_done) && !self.comment_prefs.pinned { diff --git a/crates/ui-egui/src/tools/comments/state.rs b/crates/ui-egui/src/tools/comments/state.rs index 32ef995f8f..54a723396f 100644 --- a/crates/ui-egui/src/tools/comments/state.rs +++ b/crates/ui-egui/src/tools/comments/state.rs @@ -242,16 +242,59 @@ impl CommentView { if self.editing.is_none() { self.editor_focus = None; } + // What is typed in a pop-up, and a window being dragged or resized, follow theirs too. + let moved = |k: (usize, usize)| after_delete(before, after, gone, k); + self.popup_text = self.popup_text.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text))); + self.popup_reply = self.popup_reply.take().and_then(|(p, i, text)| moved((p, i)).map(|(p, i)| (p, i, text))); + self.popup_drag = self.popup_drag.and_then(|(p, i, at)| moved((p, i)).map(|(p, i)| (p, i, at))); + self.popup_resize = self.popup_resize.and_then(|(p, i, size)| moved((p, i)).map(|(p, i)| (p, i, size))); } /// Undo or Redo just ran. Either may have put annotations back or taken them away, on any /// page, so a (page, index) held here may now name another one. The selection and the inline /// editor close, its unsaved text with it, rather than Save writing that text over whatever - /// took its place. + /// took its place; so do the pop-ups' unsaved text and drags. pub(crate) fn history_stepped(&mut self) { self.selected = None; self.editing = None; self.editor_focus = None; + self.popup_text = None; + self.popup_reply = None; + self.popup_drag = None; + self.popup_resize = None; + } + + /// The comments the pop-ups hold unsaved text, a reply or a drag for. + pub(crate) fn popup_keys(&self) -> Vec<(usize, usize)> { + let text = self.popup_text.as_ref().map(|(p, i, _)| (*p, *i)); + let reply = self.popup_reply.as_ref().map(|(p, i, _)| (*p, *i)); + let drag = self.popup_drag.map(|(p, i, _)| (p, i)); + let resize = self.popup_resize.map(|(p, i, _)| (p, i)); + [text, reply, drag, resize].into_iter().flatten().collect() + } + + /// An edit other than a deletion just ran: `before` are the comments at [`Self::popup_keys`] + /// until then, `after` the document's annotations now. When pages moved, came or went, a pop-up's (page, index) + /// may now name another comment: what was typed in that pop-up and not saved, and a window + /// being dragged or resized, are dropped rather than written to whatever took its place. + pub(crate) fn popups_after_edit(&mut self, before: &[Annotation], after: &[Annotation]) { + // The same comment: its kind, author and `/NM`, at that place before and after. + let who = |list: &[Annotation], k: (usize, usize)| { + list.iter().find(|a| (a.page, a.index) == k).map(|a| (a.subtype.clone(), a.author.clone(), a.name.clone())) + }; + let kept = |p: usize, i: usize| who(before, (p, i)).is_some_and(|w| who(after, (p, i)) == Some(w)); + if self.popup_text.as_ref().is_some_and(|(p, i, _)| !kept(*p, *i)) { + self.popup_text = None; + } + if self.popup_reply.as_ref().is_some_and(|(p, i, _)| !kept(*p, *i)) { + self.popup_reply = None; + } + if self.popup_drag.is_some_and(|(p, i, _)| !kept(p, i)) { + self.popup_drag = None; + } + if self.popup_resize.is_some_and(|(p, i, _)| !kept(p, i)) { + self.popup_resize = None; + } } } diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index e2f6b0412b..64de66a640 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3439,3 +3439,73 @@ fn a_pop_up_resized_on_a_turned_page_stays_in_place() { // Taller by the drag, less what its text had already stretched it. assert!(resized.height() > window.height() + 50.0, "taller: {window:?} -> {resized:?}"); } + +/// #2137 review: the composer's Post, clicked while a pop-up's box holds unsaved text, also +/// saves that text as the box loses the focus. Both land in one frame and both are kept. +#[test] +fn posting_from_the_composer_keeps_a_pop_ups_unsaved_text() { + let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |app| app.set_option("quick", "note").unwrap()); + // Low on the left, so the composer stays clear of the window. + click_pt(&mut h, (30.0, 40.0)); + // The composer opens with its editor focused. + h.run_steps(2); + h.event(egui::Event::Text("New note".into())); + h.run_steps(2); + let box_at = h.query_all_by(|n| shows_text(n, "A note")).next().expect("the pop-up's box").rect().right_center() - egui::vec2(4.0, 0.0); + click(&mut h, box_at); + h.event(egui::Event::Text(" edited".into())); + h.run_steps(2); + assert!(h.state().views[0].comments.composer.is_some(), "the composer is still open"); + let post = h.get_by_label("Post").rect().center(); + click(&mut h, post); + h.run_steps(3); + let all = comments(&h); + assert!( + all.iter().any(|a| a.contents.as_deref() == Some("New note")), + "the composer's note: {:?}", + all.iter().map(|a| &a.contents).collect::>() + ); + assert!( + all.iter().any(|a| a.contents.as_deref() == Some("A note edited")), + "the pop-up's text: {:?}", + all.iter().map(|a| &a.contents).collect::>() + ); +} + +/// #2137 review: what a pop-up holds for a comment (a reply being typed, unsaved text) follows +/// that comment when a deletion moves it, and is dropped when an undo or a page edit leaves its +/// (page, index) naming another; other edits keep it. +#[test] +fn a_pop_ups_unsaved_work_follows_its_comment_or_is_dropped() { + use pdfcraft_engine::Edit; + let mut h = harness_with_bytes(NOTE_WITH_A_THREAD.to_vec(), |_| {}); + let key = |h: &Harness<'static, PdfCraftApp>, text: &str| { + comments(h).into_iter().find(|a| a.contents.as_deref() == Some(text)).map(|a| (a.page, a.index)).expect(text) + }; + let (second, third) = (key(&h, "Second"), key(&h, "Third")); + h.state_mut().views[0].comments.popup_reply = Some((third.0, third.1, "draft".into())); + + // Another edit, on another comment: the draft stays. + assert!(h.state_mut().apply_edit(Edit::SetAnnotationContents { page: second.0, index: second.1, text: "Second!".into() })); + h.run_steps(2); + assert_eq!(h.state().views[0].comments.popup_reply, Some((third.0, third.1, "draft".into()))); + + // Deleting "Second" (and its answer) moves "Third" up: the draft goes with it. + assert!(h.state_mut().apply_edit(Edit::DeleteAnnotation { page: second.0, index: second.1 })); + h.run_steps(2); + let moved = key(&h, "Third"); + assert_ne!(moved, third, "Third moved up"); + assert_eq!(h.state().views[0].comments.popup_reply, Some((moved.0, moved.1, "draft".into())), "the draft is still Third's"); + + // Undo puts the thread back: the draft is dropped rather than left on another reply. + h.state_mut().undo(); + h.run_steps(2); + assert_eq!(h.state().views[0].comments.popup_reply, None); + + // A page inserted before it: the comment's (page, index) now names nothing of it. + let note = key(&h, "First"); + h.state_mut().views[0].comments.popup_text = Some((note.0, note.1, "unsaved".into())); + assert!(h.state_mut().apply_edit(Edit::InsertBlankPage { at: 0, width: 300.0, height: 200.0 })); + h.run_steps(2); + assert_eq!(h.state().views[0].comments.popup_text, None); +} From b115a8a098a9e00445877e2184ab259d34f8aef4 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 10:33:48 +0800 Subject: [PATCH 13/19] M5: test that closing a pop-up saves its typed text (#2137 review) Text typed into a pop-up's box and not yet saved is saved when the window's x is clicked: the click takes the focus from the box and closes the window in one frame, as one undo step. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/tests/it/comments.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 64de66a640..5e66092d98 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3509,3 +3509,26 @@ fn a_pop_ups_unsaved_work_follows_its_comment_or_is_dropped() { h.run_steps(2); assert_eq!(h.state().views[0].comments.popup_text, None); } + +/// #2137 review: text typed into a pop-up and not yet saved is saved when × closes the window: +/// the click takes the focus from the box and closes it in one frame, one undo step for both. +#[test] +fn closing_a_pop_up_saves_its_typed_text() { + let mut h = harness_with_bytes(BLUE_NOTE_WITH_OPEN_POP_UP.to_vec(), |_| {}); + let box_at = h.query_all_by(|n| shows_text(n, "A note")).next().expect("the pop-up's box").rect().right_center() - egui::vec2(4.0, 0.0); + click(&mut h, box_at); + h.event(egui::Event::Text(" for you".into())); + h.run_steps(2); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the pop-up is open"); + let close = h.query_all_by_label("Close").map(|n| n.rect().center()).find(|c| window.contains(*c)).expect("the window's ×"); + click(&mut h, close); + h.run_steps(3); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!(note.contents.as_deref(), Some("A note for you"), "the typed text is saved"); + assert_eq!(note.popup.map(|p| p.open), Some(false), "and the window closed"); + assert!(h.state().views[0].comments.popup_text.is_none()); + h.state_mut().undo(); + h.run_steps(2); + let note = comments(&h).into_iter().find(|a| a.subtype == "Text").expect("the note"); + assert_eq!((note.contents.as_deref(), note.popup.map(|p| p.open)), (Some("A note"), Some(true)), "one undo step for both"); +} From c2251a4e1665fdeb4881fc19cf20160ada4d6490 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 16:38:13 +0800 Subject: [PATCH 14/19] M5: a pop-up /Rect too large for f32 reads as no pop-up; reversed corners are tested (#2137 review) A file's huge number became infinite as f32 and reached the window's place. Both readers (annot summaries and the render inspector) now take such a rectangle as no pop-up, as one that isn't four numbers; opening it gives it a new place. Reversed corners were already put in order; tests now cover both, read from a file and saved through set_popup. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/annot/src/info.rs | 4 +++ crates/annot/src/tests.rs | 31 ++++++++++++++++++++++++ crates/render/src/inspect/annotations.rs | 8 ++++-- crates/render/src/inspect/tests.rs | 16 ++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index 18a395a848..b6fa661482 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -76,7 +76,11 @@ pub(crate) fn popup_info(doc: &Document, own: Option, annot: &Dict) -> O let p = doc.get(r); let p = p.as_dict()?; let rect: Vec = doc.resolve(p.get(b"Rect")?).as_array()?.iter().filter_map(|v| doc.resolve(v).as_f64().map(|x| x as f32)).collect(); + // Four numbers that are finite as `f32` (a huge one becomes infinite), corners in any order. let [x0, y0, x1, y1] = rect[..] else { return None }; + if !rect.iter().all(|v| v.is_finite()) { + return None; + } let open = p.get(b"Open").map(|o| doc.resolve(o)).is_some_and(|o| matches!(&*o, Object::Bool(true))); Some(PopupInfo { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open }) } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 4e45d1d777..d25cbacdb9 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2320,3 +2320,34 @@ fn opening_a_pop_up_without_a_rect_gives_it_one() { assert!(popup.rect[2] - popup.rect[0] >= 100.0 && popup.rect[3] - popup.rect[1] >= 100.0, "a window's size: {:?}", popup.rect); assert_eq!(annot_dict(&doc, r).reference(b"Popup"), Some(pr), "the same pop-up, not a new one"); } + +/// #2137 review: a pop-up rectangle with its corners reversed is read, and saved, with them in +/// order (ISO 32000-2 §7.9.5: a reader normalises a rectangle). One whose numbers aren't finite +/// as `f32` (a file's huge number) reads as no pop-up, and opening it gives it a new place. +#[test] +fn a_pop_up_rect_with_reversed_corners_is_put_in_order_and_a_huge_one_is_none() { + let mut doc = fixture(); + let note = add_annotation(&mut doc, &new(0, Shape::Note { at: [20.0, 280.0], icon: NoteIcon::Comment }), &meta("n1")).unwrap(); + let (_, r) = annot_ref(&mut doc, 0, note).unwrap(); + let pr = annot_dict(&doc, r).reference(b"Popup").expect("new notes have a pop-up"); + let popup = |doc: &Document| summaries(doc).into_iter().find(|s| s.index == note).unwrap().popup; + let set_rect = |doc: &mut Document, rect: [f64; 4]| doc.update_dict(pr, |p| p.set(b"Rect".to_vec(), num_array(&rect))).unwrap(); + + // Reversed in the file: read in order. + set_rect(&mut doc, [290.0, 200.0, 150.0, 100.0]); + assert_eq!(popup(&doc), Some(PopupInfo { rect: [150.0, 100.0, 290.0, 200.0], open: false })); + // Reversed through `set_popup`: saved in order. + set_popup(&mut doc, 0, note, Some([300.0, 220.0, 160.0, 120.0]), None, &meta("n1")).unwrap(); + let saved: Vec = + doc.get(pr).as_dict().and_then(|p| p.get(b"Rect")).and_then(Object::as_array).unwrap().iter().filter_map(Object::as_f64).collect(); + assert_eq!(saved, [160.0, 120.0, 300.0, 220.0]); + + // A number too large for `f32` (or infinite): no pop-up, and opening it gives it a place. + for huge in [1e300, f64::INFINITY] { + set_rect(&mut doc, [0.0, 0.0, huge, 100.0]); + assert_eq!(popup(&doc), None, "{huge}"); + } + set_popup(&mut doc, 0, note, None, Some(true), &meta("n1")).unwrap(); + let given = popup(&doc).expect("the pop-up reads again"); + assert!(given.open && given.rect.iter().all(|v| v.is_finite()), "{given:?}"); +} diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 36e0556f65..7139a874ae 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -166,12 +166,16 @@ impl<'a> Inspector<'a> { && p.get(b"Parent") .map_or(true, |parent| parent.as_reference().is_ok() && self.resolved_id(parent).is_some_and(|id| Some(id) == own)) }; - // A pop-up without a `/Rect` of four numbers is none, as `pdfcraft_annot::summaries` - // reads it: both lists of the comments agree. + // A pop-up without a `/Rect` of four numbers finite as `f32` is none, as + // `pdfcraft_annot::summaries` reads it: both lists of the comments agree. Its corners + // may come in any order. let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).and_then(|p| { let rect = self.resolve(p.get(b"Rect").ok()?).as_array().ok()?; let nums: Vec = rect.iter().filter_map(|v| self.resolve(v).as_float().ok()).collect(); let [x0, y0, x1, y1] = nums[..] else { return None }; + if !nums.iter().all(|v| v.is_finite()) { + return None; + } Some(Popup { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open: p.get(b"Open").ok().and_then(|o| self.resolve(o).as_bool().ok()).unwrap_or(false), diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 62bc130290..69466b0e07 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1916,3 +1916,19 @@ fn a_pop_up_without_a_four_number_rect_is_none() { assert_eq!(note.popup.is_some(), read, "{rect:?}"); } } + +/// #2137 review: a pop-up `/Rect` with its corners reversed reads with them in order (ISO +/// 32000-2 §7.9.5), and one with a number too large for `f32` reads as no pop-up, as +/// `pdfcraft_annot::summaries` reads both. +#[test] +fn a_pop_up_rect_with_reversed_corners_is_put_in_order_and_a_huge_one_is_none() { + let bytes = "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] /Annots [4 0 R 5 0 R] >> endobj\n4 0 obj << /Type /Annot /Subtype /Text /Rect [20 260 40 280] /Contents (Hi) /Popup 5 0 R >> endobj\n5 0 obj << /Type /Annot /Subtype /Popup /Rect RECT /Parent 4 0 R /Open true >> endobj\ntrailer << /Root 1 0 R >>\n%%EOF\n"; + let popup = |rect: &str| { + let info = inspect(Arc::new(bytes.replace("RECT", rect).into_bytes()), None).expect("opens"); + info.annotations.iter().find(|a| a.subtype == "Text").expect("the note").popup.map(|p| p.rect) + }; + assert_eq!(popup("[150 200 50 100]"), Some([50.0, 100.0, 150.0, 200.0]), "reversed"); + let huge = format!("1{}", "0".repeat(40)); + assert_eq!(popup(&format!("[0 0 {huge} 100]")), None, "a number past f32"); + assert_eq!(popup(&format!("[0 0 {huge}.0 100]")), None, "a real past f32"); +} From f668a8a64b3486a6cc341bebd4a3480c89e9437f Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:40:27 +0800 Subject: [PATCH 15/19] M5: a Replace Text pair is one pop-up window, with the replacement as its text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A group member (/RT /Group, as Replace Text's strikeout and caret are) is part of its comment, not a reply (ISO 32000-2 §12.5.6.2), so the window lists no group member among the replies. For Replace Text as main writes it (the replacement on the caret, grouped under an empty strikeout, #5688), the window shows and edits the caret's text, the proposal's. A second test pins that a double-click on the proposal opens one window, the primary's, in both layouts; the page's hit test already picks only the group's primary. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/tools/comments/popups.rs | 23 +++++- crates/ui-egui/tests/it/comments.rs | 87 +++++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index fc902708af..b89e23caba 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -446,14 +446,31 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr // Replies, as the thread reads: text replies only, filed under this // comment by `/IRT` reference as in the Comments panel, each under the // one it answers. A review status or a checkmark (`/State`) is a reply too, - // but not one to read here. + // but not one to read here, and a member of the comment's group (`/RT + // /Group`, ISO 32000-2 §12.5.6.2) is part of the comment, not a reply. let replies: Vec<&Annotation> = info .annotations .iter() .filter(|r| { - r.state.is_none() && r.parent.is_some_and(|parent| tops.get(&(r.page, r.index)).copied().unwrap_or(parent) == key) + r.state.is_none() + && !r.grouped + && r.parent.is_some_and(|parent| tops.get(&(r.page, r.index)).copied().unwrap_or(parent) == key) }) .collect(); + // The comment's text. Replace Text as PdfCraft used to write it keeps the + // replacement on the caret grouped under an empty strikeout: the window shows + // and edits the caret's text, the proposal's. + let holder = info + .annotations + .iter() + .find(|r| { + r.grouped + && r.parent == Some(key) + && a.contents.as_deref().unwrap_or("").is_empty() + && r.contents.as_deref().is_some_and(|c| !c.is_empty()) + }) + .cloned() + .unwrap_or_else(|| a.clone()); let order = thread(key, &replies); let (nodes, roots) = nest(&order); let mut parts = Parts { allowed, author: &prefs.author, pal, cv: &mut view.comments, edits: &mut edits }; @@ -474,7 +491,7 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr egui::ScrollArea::vertical().id_salt(("popup-body", page, a.index)).max_height(body_height).auto_shrink([false, false]); scroll.show(ui, |ui| { ui.spacing_mut().item_spacing.y = 3.0; - text_box(ui, &mut parts, &a, tl!("Add a comment")); + text_box(ui, &mut parts, &holder, tl!("Add a comment")); footer(ui, &mut parts, &a); for root in &roots { reply_panel(ui, &mut parts, &nodes, *root); diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 5e66092d98..24f34eeb19 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3091,6 +3091,93 @@ trailer << /Root 1 0 R >> assert!(!in_window("Marked set by Ada"), "not the checkmark"); } +/// A Replace Text proposal: a strikeout and a caret grouped by `/IRT` and `/RT /Group`, with the +/// pop-up `open`. `caret_first` writes the caret as the group's primary, holding the replacement, +/// with the strikeout grouped under it; otherwise the strikeout is the primary and the +/// replacement is on the caret grouped under it, as PdfCraft used to write it. +fn replace_text_pdf(caret_first: bool, open: bool) -> Vec { + let primary = if caret_first { 4 } else { 5 }; + let strike = format!( + "<< /Type /Annot /Subtype /StrikeOut /Rect [40 100 140 120] /QuadPoints [40 120 140 120 40 100 140 100] /C [1 0 0] /T (Ada) /IT /StrikeOutTextEdit{} >>", + if caret_first { " /IRT 4 0 R /RT /Group" } else { " /Popup 6 0 R" } + ); + let caret = format!( + "<< /Type /Annot /Subtype /Caret /Rect [135 95 145 105] /C [0 0 1] /T (Bob) /Contents (slow red){} >>", + if caret_first { " /IT /Replace /Popup 6 0 R" } else { " /IRT 5 0 R /RT /Group" } + ); + format!( + "%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 [5 0 R 4 0 R 6 0 R] >> endobj +4 0 obj {caret} endobj +5 0 obj {strike} endobj +6 0 obj << /Type /Annot /Subtype /Popup /Parent {primary} 0 R /Rect [160 20 290 150] /Open {open} >> endobj +trailer << /Root 1 0 R >> +%%EOF", + ) + .into_bytes() +} + +/// Review on #2137: a Replace Text proposal's window shows the replacement as the comment's text, +/// and the other member of the group is part of the comment, not a reply panel (ISO 32000-2 +/// §12.5.6.2: a group's members are one unit). Both layouts: the caret as the primary, and the +/// strikeout as the primary with the replacement on the grouped caret. +#[test] +fn a_replace_text_window_shows_the_replacement_as_its_text_not_as_a_reply() { + for caret_first in [true, false] { + let pdf = replace_text_pdf(caret_first, true); + let mut h = harness(move |app| { + app.open_bytes("replace.pdf", None, pdf).expect("opens"); + app.close_tab(0); + }); + h.run_steps(6); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the open pop-up shows"); + let boxes: Vec = h + .query_all_by(|n| n.role() == Role::MultilineTextInput) + .filter(|n| window.contains(n.rect().center())) + .map(|n| n.value().unwrap_or_default()) + .collect(); + assert_eq!(boxes, ["slow red"], "caret first: {caret_first}: one box, the replacement"); + } +} + +/// Review on #2137: a double-click on a Replace Text proposal opens the group's one window, the +/// primary's, in either layout. (A double-click on the member grouped under it opens the same +/// window once members are picked on the page; main hit-tests only the primary.) +#[test] +fn a_double_click_on_a_replace_text_opens_its_one_window() { + for (caret_first, on_caret) in [(true, true), (false, false)] { + let pdf = replace_text_pdf(caret_first, false); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("replace.pdf", None, pdf.clone()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app + }); + h.run_steps(4); + crate::support::settle(&mut h); + h.run_steps(45); + let target = if on_caret { (140.0, 98.0) } else { (60.0, 110.0) }; + let p = at(&h, target.0, target.1); + h.hover_at(p); + h.run_steps(1); + for _ in 0..2 { + h.drag_at(p); + h.run_steps(1); + h.drop_at(p); + h.run_steps(1); + } + h.run_steps(4); + let primary_subtype = if caret_first { "Caret" } else { "StrikeOut" }; + let c = comments(&h); + let primary = c.iter().find(|a| a.subtype == primary_subtype).expect("the primary"); + assert_eq!(primary.popup.map(|w| w.open), Some(true), "caret first: {caret_first}, on the caret: {on_caret}: the group's window opened"); + assert_eq!(h.state().views[0].comments.popup_rects.len(), 1, "one window"); + } +} + /// A double-click on a sticky note opens its pop-up window on the page, where its text is read /// and edited; the title bar drags it (saved as the pop-up's place, one undo step); × closes it /// (comment.popups). From a3842f4a85471edfd4d516d6c2565794ce73f834 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:44:50 +0800 Subject: [PATCH 16/19] M5: a locked Replace Text proposal's window shows its replacement read-only For Replace Text as main writes it (#5688), the window shows and edits the caret's text, but the caret is the group's subordinate: locking the comment sets /Locked on the strikeout, the primary, and the caret's own flag left the text editable from the window. The text box is now read-only when either is locked. The tests' comments no longer call this a review point. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/tools/comments/popups.rs | 8 +++-- crates/ui-egui/tests/it/comments.rs | 33 +++++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index b89e23caba..84350212ec 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -457,10 +457,11 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr && r.parent.is_some_and(|parent| tops.get(&(r.page, r.index)).copied().unwrap_or(parent) == key) }) .collect(); - // The comment's text. Replace Text as PdfCraft used to write it keeps the + // The comment's text. Replace Text as `main` writes it (#5688) keeps the // replacement on the caret grouped under an empty strikeout: the window shows - // and edits the caret's text, the proposal's. - let holder = info + // and edits the caret's text, the proposal's. Locking the comment locks that + // text too. + let mut holder = info .annotations .iter() .find(|r| { @@ -471,6 +472,7 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr }) .cloned() .unwrap_or_else(|| a.clone()); + holder.locked |= a.locked; let order = thread(key, &replies); let (nodes, roots) = nest(&order); let mut parts = Parts { allowed, author: &prefs.author, pal, cv: &mut view.comments, edits: &mut edits }; diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index b3f18643f8..d5eb8b495c 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3119,10 +3119,10 @@ trailer << /Root 1 0 R >> .into_bytes() } -/// Review on #2137: a Replace Text proposal's window shows the replacement as the comment's text, -/// and the other member of the group is part of the comment, not a reply panel (ISO 32000-2 -/// §12.5.6.2: a group's members are one unit). Both layouts: the caret as the primary, and the -/// strikeout as the primary with the replacement on the grouped caret. +/// A Replace Text proposal's window shows the replacement as the comment's text, and the other +/// member of the group is part of the comment, not a reply panel (ISO 32000-2 §12.5.6.2: a group's +/// members are one unit). Both layouts: the caret as the primary, and the strikeout as the primary +/// with the replacement on the grouped caret, as `main` writes it (#5688). #[test] fn a_replace_text_window_shows_the_replacement_as_its_text_not_as_a_reply() { for caret_first in [true, false] { @@ -3142,8 +3142,29 @@ fn a_replace_text_window_shows_the_replacement_as_its_text_not_as_a_reply() { } } -/// Review on #2137: a double-click on a Replace Text proposal opens the group's one window, the -/// primary's, in either layout. (A double-click on the member grouped under it opens the same +/// A locked Replace Text proposal, as `main` writes it (the strikeout primary and locked, the +/// replacement on the grouped caret), shows the replacement in its window read-only: locking the +/// comment locks the text the window shows for it. +#[test] +fn a_locked_replace_text_window_shows_the_replacement_read_only() { + let pdf = String::from_utf8(replace_text_pdf(false, true)) + .unwrap() + .replace("/IT /StrikeOutTextEdit /Popup", "/IT /StrikeOutTextEdit /F 128 /Popup") + .into_bytes(); + let mut h = harness(move |app| { + app.open_bytes("replace.pdf", None, pdf).expect("opens"); + app.close_tab(0); + }); + h.run_steps(6); + let (_, window) = h.state().views[0].comments.popup_rects.first().copied().expect("the open pop-up shows"); + let inside = |n: &egui_kittest::Node<'_>| window.contains(n.rect().center()); + let boxes = h.query_all_by(|n| n.role() == Role::MultilineTextInput).filter(inside).count(); + assert_eq!(boxes, 0, "no box to type in while the comment is locked"); + assert!(h.query_all_by_label("slow red").any(|n| inside(&n)), "the replacement is shown"); +} + +/// A double-click on a Replace Text proposal opens the group's one window, the primary's, in +/// either layout. (A double-click on the member grouped under it opens the same /// window once members are picked on the page; main hit-tests only the primary.) #[test] fn a_double_click_on_a_replace_text_opens_its_one_window() { From 03da84523b061d6fcc8a1d1f7e9ad6d91111f74f Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Wed, 4 Nov 2026 01:56:46 +0800 Subject: [PATCH 17/19] M5: the edit-composer whitespace test opens a note's composer from its menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test #5954 added on main double-clicks a sticky note and expects the edit composer. On this branch a double-click on a note opens its pop-up window instead, so for a note the test now checks that the pop-up opened, then opens the edit composer with the page menu's "Edit text…" and goes on as before: the composer still trims trailing whitespace from a non-typewriter comment. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/ui-egui/tests/it/comments.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index e1dab26b61..cae06493b9 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -4035,7 +4035,25 @@ fn edit_composer_preserves_only_typewriter_whitespace(kind: u8) { }; support::input::double_click(&mut h, point); h.run_steps(3); - let c = h.state().views[0].comments.composer.as_ref().expect("double-click opens the edit composer"); + if kind == 2 { + // A double-click on a sticky note opens its pop-up window, as in Acrobat; its edit + // composer is the page menu's "Edit text…". + let popup_open = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + doc.info.annotations.iter().find(|a| a.page == 1 && a.index == 0).unwrap().popup.is_some_and(|p| p.open) + }; + assert!(h.state().views[0].comments.composer.is_none(), "a note's double-click opens no composer"); + assert!(popup_open(&h), "a note's double-click opens its pop-up"); + h.hover_at(point); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(3); + h.get_all_by_label("Edit text…").last().unwrap().click(); + h.run_steps(3); + } + let c = h.state().views[0].comments.composer.as_ref().expect("the edit composer is open"); assert_eq!(c.page, 1); assert_eq!(c.kind, pdfcraft_ui_egui::comments::ComposerKind::Edit(0)); h.input_mut().events.push(egui::Event::Text(" edited\n\n".into())); From abb29ba7e129bcb2a9ffe70d13cc776d3790412c Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 11:14:10 +0800 Subject: [PATCH 18/19] M5: keep the pop-up raster rule in a helper after merging main (clippy cognitive complexity) Main's signature-state redraw in apply_edit pushed the function over the cognitive-complexity limit with the pop-up exception inline; the rule moves into one_page_changed unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/ui-egui/src/editing.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 5923e9c435..969af5bdf4 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -219,12 +219,7 @@ impl PdfCraftApp { view.forms.unbaked.extend(unbaked); } } else if let Some(page) = one_page { - // Comment and content edits change one page: keep every other raster. Opening, - // closing or moving a pop-up changes none: the window is drawn over the page, and - // a /Popup annotation paints nothing in the raster (ISO 32000-2 §12.5.6.14). - if !matches!(edit, Edit::SetAnnotationPopup { .. }) { - view.page_changed(page); - } + one_page_changed(view, &edit, page); } else if let Some((changed_pages, unbaked)) = form_edit { view.forms.unbaked.extend(unbaked); for page in changed_pages { @@ -946,6 +941,15 @@ fn content_page(edit: &Edit) -> Option { } } +/// A comment or content edit changes one page, `page`: keep every other raster. Opening, closing +/// or moving a pop-up changes none: the window is drawn over the page, and a /Popup annotation +/// paints nothing in the raster (ISO 32000-2 §12.5.6.14). +fn one_page_changed(view: &mut crate::DocView, edit: &Edit, page: usize) { + if !matches!(edit, Edit::SetAnnotationPopup { .. }) { + view.page_changed(page); + } +} + fn comment_page(edit: &Edit) -> Option { match edit { Edit::AddAnnotation(a) => Some(a.page), From 389e0e983944e153ee18cb2562379b13d99e15cd Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 11:36:22 +0800 Subject: [PATCH 19/19] M5: pop-up title bars and resize grips move only with the primary button Main's page-pointer guard (NOT_PAGE_DRAGS) refuses any-button drag starts in page modules; a right-button drag on a pop-up's bar no longer moves it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/ui-egui/src/tools/comments/popups.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/popups.rs b/crates/ui-egui/src/tools/comments/popups.rs index 8b529dfa0a..839cdcd9c7 100644 --- a/crates/ui-egui/src/tools/comments/popups.rs +++ b/crates/ui-egui/src/tools/comments/popups.rs @@ -425,7 +425,7 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr if grip.hovered() || grip.dragged() { ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); } - if grip.drag_started() { + if grip.drag_started_by(egui::PointerButton::Primary) { view.comments.popup_drag = Some((page, a.index, pos)); } if grip.dragged() @@ -514,7 +514,7 @@ pub(crate) fn popups(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, pr let stroke = egui::Stroke::new(1.0, pal.faint); ui.painter().line_segment([corner - egui::vec2(3.0, inset), corner - egui::vec2(inset, 3.0)], stroke); } - if grip.drag_started() { + if grip.drag_started_by(egui::PointerButton::Primary) { view.comments.popup_resize = Some((page, a.index, size)); } if grip.dragged()