From ab433eb763dd45086ef16131f734f2de804031fc Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:15:49 -0500 Subject: [PATCH 1/6] M12: edit attachment descriptions --- crates/automation/src/dispatch.rs | 4 +++ crates/automation/src/tools.rs | 2 ++ crates/automation/tests/it/automation.rs | 29 ++++++++++++++++ crates/engine/src/edit.rs | 5 +++ crates/engine/src/edit/labels.rs | 2 ++ crates/engine/src/edit/run.rs | 4 +++ crates/engine/src/history.rs | 2 +- crates/organize/src/attachments.rs | 33 +++++++++++++++++- crates/organize/src/import.rs | 2 +- crates/organize/src/import/carry.rs | 39 +++++++++++++-------- crates/organize/src/lib.rs | 2 +- crates/organize/src/tests.rs | 37 ++++++++++++++++++++ crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/dialogs.rs | 41 +++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 2 ++ crates/ui-egui/src/panels/right.rs | 7 ++++ crates/ui-egui/src/state.rs | 2 ++ crates/ui-egui/tests/it/attachment_row.rs | 21 +++++++++++- parity/acrobat-features.toml | 8 ++--- 19 files changed, 220 insertions(+), 23 deletions(-) diff --git a/crates/automation/src/dispatch.rs b/crates/automation/src/dispatch.rs index 36e0d00454..4d3d3ec425 100644 --- a/crates/automation/src/dispatch.rs +++ b/crates/automation/src/dispatch.rs @@ -704,6 +704,10 @@ impl Automation { let edit = Edit::DeleteAttachment { key: a.str("name")?.into() }; self.apply(&a, edit)? } + "attachment_edit" => { + let edit = Edit::EditAttachmentDescription { key: a.str("name")?.into(), description: Some(a.str("description")?.into()) }; + self.apply(&a, edit)? + } "doc_remove_hidden" => self.doc_remove_hidden(&a)?, "doc_audit_redactions" => self.doc_audit_redactions(&a)?, "fill_sign_add" => self.fill_sign_add(&a)?, diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index c6c200cece..5206ce8bdf 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -138,6 +138,8 @@ pub fn tools() -> Vec { t("attachment_delete", "Delete an attachment", "Remove the document-level attachment with this name (as doc_info lists it). Undoable.") .destructive() .with(schema(json!({"doc": doc(), "name": {"type": "string"}}), &["doc", "name"])), + t("attachment_edit", "Edit an attachment description", "Change the description of the document-level attachment with this name (as doc_info lists it). An empty description clears it. The file and other attachment data are preserved. Undoable.") + .with(schema(json!({"doc": doc(), "name": {"type": "string"}, "description": {"type": "string"}}), &["doc", "name", "description"])), t("doc_open", "Open a PDF", "Open a PDF file and return its document id, page count and whether it can be edited.") .cmd("file.open") .with(schema(json!({ "path": { "type": "string" }, "password": { "type": "string", "description": "User or owner password for encrypted files." } }), &["path"])), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6a5765b911..4be863ac5b 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -9324,6 +9324,35 @@ fn attaching_and_deleting_files_through_the_tools() { assert!(a.call("attachment_add", &json!({ "doc": doc, "path": "missing.txt" })).is_err(), "a file that is not there is refused"); } +#[test] +fn attachment_descriptions_can_be_edited_cleared_undone_and_saved() { + let dir = workdir("attachment-description"); + let bytes = b"original attachment bytes"; + std::fs::write(dir.join("notes.txt"), bytes).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path": "a.pdf"}))["doc"].as_u64().unwrap(); + ok(&mut a, "attachment_add", json!({"doc": doc, "path": "notes.txt", "description": "Original"})); + + ok(&mut a, "attachment_edit", json!({"doc": doc, "name": "notes.txt", "description": "Updated"})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Updated"); + ok(&mut a, "edit_undo", json!({"doc": doc})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Original"); + ok(&mut a, "edit_redo", json!({"doc": doc})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Updated"); + + ok(&mut a, "attachment_edit", json!({"doc": doc, "name": "notes.txt", "description": ""})); + assert!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"].is_null()); + ok(&mut a, "edit_undo", json!({"doc": doc})); + ok(&mut a, "doc_save", json!({"doc": doc, "path": "edited.pdf", "full": true})); + let saved = std::fs::read(dir.join("edited.pdf")).unwrap(); + let info = pdfcraft_render::inspect(std::sync::Arc::new(saved.clone()), None).unwrap(); + let attachment = info.attachments.iter().find(|item| item.name == "notes.txt").unwrap(); + assert_eq!(attachment.description.as_deref(), Some("Updated")); + assert_eq!(pdfcraft_render::attachment_data(&saved, None, attachment).unwrap(), bytes); + let reopened = ok(&mut a, "doc_open", json!({"path": "edited.pdf"}))["doc"].as_u64().unwrap(); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": reopened}))["attachments"][0]["description"], "Updated"); +} + #[test] fn fdf_nested_comment_values_survive_export_import_and_save_through_tools() { use pdfcraft_cos::{Dict, Document, Object, PdfString, SaveOptions, write_full}; diff --git a/crates/engine/src/edit.rs b/crates/engine/src/edit.rs index 8f43689aa9..dd4833b4f8 100644 --- a/crates/engine/src/edit.rs +++ b/crates/engine/src/edit.rs @@ -317,6 +317,11 @@ pub enum Edit { DeleteAttachment { key: String, }, + /// Change or clear only `/Desc` on a document-level attachment's file specification. + EditAttachmentDescription { + key: String, + description: Option, + }, /// Build a tagged structure tree for an untagged, text-only document. AutoTagText, /// Add alternate text: set (or clear, with `None`) a figure's alternate text. `figure` is diff --git a/crates/engine/src/edit/labels.rs b/crates/engine/src/edit/labels.rs index c3339453af..cc5ca9508e 100644 --- a/crates/engine/src/edit/labels.rs +++ b/crates/engine/src/edit/labels.rs @@ -31,6 +31,7 @@ pub const EDIT_LABELS: &[&str] = &[ "Insert pages from {name} — fields renamed", "Attach {name}", "Delete attachment {key}", + "Edit attachment description", "Add bookmark", "Rename bookmark", "Delete bookmark", @@ -233,6 +234,7 @@ impl Edit { Edit::SetInitialView(_) => Label::of("Change initial view"), Edit::AddAttachment { name, .. } => Label::with("Attach {name}", name.as_str()), Edit::DeleteAttachment { key } => Label::with("Delete attachment {key}", key.as_str()), + Edit::EditAttachmentDescription { .. } => Label::of("Edit attachment description"), Edit::AutoTagText => Label::of("Automatically tag text"), Edit::SetAltText { .. } => Label::of("Set alternate text"), Edit::AutoTagFormFields => Label::of("Tag form fields"), diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index c6b1a7438b..ffce0e0c75 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -127,6 +127,7 @@ pub(crate) fn check_permission(edit: &Edit, p: &pdfcraft_cos::Permissions) -> Re | Edit::SetInitialView(_) | Edit::AddAttachment { .. } | Edit::DeleteAttachment { .. } + | Edit::EditAttachmentDescription { .. } | Edit::AutoTagText | Edit::SetAltText { .. } | Edit::AutoTagFormFields @@ -360,6 +361,9 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E pdfcraft_organize::add_attachment(doc, name, bytes, description.as_deref(), mime.as_deref(), cx.date.as_deref())?; } Edit::DeleteAttachment { key } => pdfcraft_organize::delete_attachment(doc, key)?, + Edit::EditAttachmentDescription { key, description } => { + pdfcraft_organize::set_attachment_description(doc, key, description.as_deref())?; + } Edit::ReplaceText { page, quads, text, author, strike, caret } => { // One `/NM` each: the caret replies to the strikeout, and XFDF names that reply by // the strikeout's `/NM` (#2666). diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index fafa65b4c2..29dd7ba59d 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -48,7 +48,7 @@ pub(crate) fn scope_of(edit: &Edit) -> Scope { match edit { Edit::SetInfo { .. } => Scope::Metadata, // The Attachments list is read from the whole document. - Edit::AddAttachment { .. } | Edit::DeleteAttachment { .. } => Scope::Full, + Edit::AddAttachment { .. } | Edit::DeleteAttachment { .. } | Edit::EditAttachmentDescription { .. } => Scope::Full, // A file attachment also changes the Attachments list. Edit::AddAnnotation(a) if matches!(a.shape, Shape::Attachment { .. }) => Scope::Full, Edit::AddMeasurement(_) diff --git a/crates/organize/src/attachments.rs b/crates/organize/src/attachments.rs index b0bf222991..694755c074 100644 --- a/crates/organize/src/attachments.rs +++ b/crates/organize/src/attachments.rs @@ -7,7 +7,7 @@ use pdfcraft_cos::{Dict, Document, Object, PdfString, Stream}; use crate::OrganizeError; -use crate::import::{name_tree_entries, set_attachments}; +use crate::import::{name_tree_entries, set_attachments, set_attachments_exact}; /// The keys of the document-level attachments, in name-tree order. pub fn attachment_keys(doc: &Document) -> Vec { @@ -82,6 +82,37 @@ pub fn delete_attachment(doc: &mut Document, key: &str) -> Result<(), OrganizeEr if all.is_empty() { clear_embedded_files(doc) } else { set_attachments(doc, all) } } +/// Set or clear the description of a document-level attachment without replacing its file +/// specification or embedded stream. +pub fn set_attachment_description(doc: &mut Document, key: &str, description: Option<&str>) -> Result<(), OrganizeError> { + let mut all = entries(doc); + let Some((_, value)) = all.iter_mut().find(|(name, _)| PdfString::literal(name.clone()).to_text() == key) else { + return Err(OrganizeError::Invalid(format!("there is no attachment named {key}"))); + }; + let description = description.map(str::trim).filter(|s| !s.is_empty()).map(PdfString::text).map(Object::String); + match value { + Object::Ref(reference) => { + doc.update_dict(*reference, |spec| match description { + Some(description) => spec.set(b"Desc".to_vec(), description), + None => { + spec.remove(b"Desc"); + } + })?; + } + Object::Dict(spec) => { + match description { + Some(description) => spec.set(b"Desc".to_vec(), description), + None => { + spec.remove(b"Desc"); + } + } + set_attachments_exact(doc, all)?; + } + _ => return Err(OrganizeError::Invalid(format!("attachment {key} has an invalid file specification"))), + } + Ok(()) +} + /// The `/EmbeddedFiles` entries (key, file specification), in tree order. fn entries(doc: &Document) -> Vec<(Vec, Object)> { let Some(tree) = doc diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 9f86d6455a..ab946e169b 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -28,8 +28,8 @@ mod combine; mod copy; mod fields; -pub(crate) use carry::set_attachments; use carry::{Mark, source_outline, write_outline_level}; +pub(crate) use carry::{set_attachments, set_attachments_exact}; pub use combine::{ CombineOutcome, Combiner, Run, SplitBy, combine, combine_grouped, combine_grouped_with, combine_selected, combine_selected_with, combine_with, extract_pages, split, split_ranges, diff --git a/crates/organize/src/import/carry.rs b/crates/organize/src/import/carry.rs index 15a412e231..c04c17ec9e 100644 --- a/crates/organize/src/import/carry.rs +++ b/crates/organize/src/import/carry.rs @@ -23,29 +23,40 @@ pub(super) fn collect_attachments(dst: &mut Document, src: &Document, out: &mut /// Write attachments as a flat, sorted `/EmbeddedFiles` name tree; duplicate names get " (2)"… pub(crate) fn set_attachments(dst: &mut Document, mut entries: Vec<(Vec, Object)>) -> Result<(), OrganizeError> { + write_attachments(dst, &mut entries, true) +} + +/// Rewrite the name tree without changing attachment keys (used when editing one file spec). +pub(crate) fn set_attachments_exact(dst: &mut Document, mut entries: Vec<(Vec, Object)>) -> Result<(), OrganizeError> { + write_attachments(dst, &mut entries, false) +} + +fn write_attachments(dst: &mut Document, entries: &mut [(Vec, Object)], uniquify: bool) -> Result<(), OrganizeError> { if entries.is_empty() { return Ok(()); } // Names are compared and numbered as text: a key written as UTF-16 (a name with an accent // or Chinese letters) must not get ASCII bytes appended to it. - let mut seen = std::collections::HashSet::new(); - for (k, _) in entries.iter_mut() { - let base = pdfcraft_cos::PdfString::literal(k.clone()).to_text(); - let mut text = base.clone(); - let mut n = 2; - while !seen.insert(text.clone()) { - text = format!("{base} ({n})"); - n += 1; - } - if text != base { - *k = pdfcraft_cos::PdfString::text(&text).bytes; + if uniquify { + let mut seen = std::collections::HashSet::new(); + for (k, _) in entries.iter_mut() { + let base = pdfcraft_cos::PdfString::literal(k.clone()).to_text(); + let mut text = base.clone(); + let mut n = 2; + while !seen.insert(text.clone()) { + text = format!("{base} ({n})"); + n += 1; + } + if text != base { + *k = pdfcraft_cos::PdfString::text(&text).bytes; + } } } entries.sort_by(|a, b| a.0.cmp(&b.0)); let mut names = Vec::new(); - for (k, v) in entries { - names.push(Object::String(pdfcraft_cos::PdfString::literal(k))); - names.push(v); + for (k, v) in entries.iter() { + names.push(Object::String(pdfcraft_cos::PdfString::literal(k.clone()))); + names.push(v.clone()); } let mut tree = Dict::new(); tree.set(b"Names".to_vec(), Object::Array(names)); diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index b890965d0c..63328a83dd 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -26,7 +26,7 @@ mod transition; pub mod view; mod xmp; -pub use attachments::{add_attachment, attachment_keys, delete_attachment, mime_for_name}; +pub use attachments::{add_attachment, attachment_keys, delete_attachment, mime_for_name, set_attachment_description}; pub use boxes::{BoxSpec, PageBox, page_boxes, set_page_box}; pub use comments::import_comments; pub use dedupe::dedupe_resources; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 33dc262fce..d196596b0a 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3440,6 +3440,43 @@ fn attachments_are_added_listed_and_deleted() { assert!(doc.get(doc.root().unwrap()).as_dict().unwrap().get(b"Names").is_none()); } +#[test] +fn attachment_description_changes_only_the_file_spec_and_can_be_cleared() { + let mut doc = Document::open(Arc::new(fixture())).unwrap(); + let key = add_attachment(&mut doc, "notes.txt", b"keep these bytes", Some("Original"), Some("text/plain"), None).unwrap(); + let root = doc.root().unwrap(); + let names = doc.get(root).as_dict().unwrap().get(b"Names").cloned().unwrap(); + let tree = doc.resolve(&names).as_dict().unwrap().get(b"EmbeddedFiles").cloned().unwrap(); + let tree = doc.resolve(&tree); + let pairs = tree.as_dict().unwrap().get(b"Names").cloned().unwrap(); + let resolved_pairs = doc.resolve(&pairs); + let pairs = resolved_pairs.as_array().unwrap(); + let spec_ref = pairs.get(1).and_then(Object::as_ref).unwrap(); + let stream_ref = doc.get(spec_ref).as_dict().unwrap().get(b"EF").and_then(Object::as_dict).unwrap().reference(b"F").unwrap(); + doc.update_dict(spec_ref, |spec| spec.set(b"CustomKey".to_vec(), Object::name("preserved"))).unwrap(); + + set_attachment_description(&mut doc, &key, Some("Revised description")).unwrap(); + let spec_object = doc.get(spec_ref); + let spec = spec_object.as_dict().unwrap(); + assert_eq!(spec.get(b"Desc").and_then(Object::as_string).map(pdfcraft_cos::PdfString::to_text).as_deref(), Some("Revised description")); + assert_eq!(spec.get(b"UF").and_then(Object::as_string).map(pdfcraft_cos::PdfString::to_text).as_deref(), Some("notes.txt")); + assert_eq!(spec.get(b"CustomKey"), Some(&Object::name("preserved"))); + assert_eq!(spec.get(b"EF").and_then(Object::as_dict).and_then(|ef| ef.reference(b"F")), Some(stream_ref)); + let stream_object = doc.get(stream_ref); + let pdfcraft_cos::Object::Stream(stream) = stream_object.as_ref() else { panic!("the attachment file remains a stream") }; + assert_eq!(stream.decoded().unwrap(), b"keep these bytes"); + assert_eq!(attachment_keys(&doc), ["notes.txt"]); + + set_attachment_description(&mut doc, &key, Some(" ")).unwrap(); + assert!(!doc.get(spec_ref).as_dict().unwrap().contains(b"Desc")); + let reopened = save_and_reopen(&doc); + assert_eq!(attachment_keys(&reopened), ["notes.txt"]); + assert_eq!( + set_attachment_description(&mut doc, "missing.txt", Some("x")), + Err(OrganizeError::Invalid("there is no attachment named missing.txt".into())) + ); +} + #[test] fn rotate_repeated_indices_accumulate_from_the_current_page_value() { for level in 0..3 { diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 425c2e26ce..d6ce979484 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -165,6 +165,7 @@ impl PdfCraftApp { pending_overwrite: None, save_override: None, props_draft: None, + attachment_description_draft: None, view_draft: None, custom_draft: None, requests: Default::default(), diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d24e62cb7d..220a04393a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -54,6 +54,7 @@ struct Pending { optimize_now: bool, duplicate_now: Option, page_props_now: Option<(pdfcraft_engine::DocId, u64, Edit)>, + attachment_description_now: Option<(pdfcraft_engine::DocId, Edit)>, replace_now: bool, open_revision: Option, a11y_now: bool, @@ -334,6 +335,24 @@ fn dialog_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &m req.close = req.apply || cancel; return false; } + Dialog::AttachmentDescription => { + let Some((doc, _, _)) = app.attachment_description_draft.as_ref() else { + req.close = true; + return false; + }; + if app.active_ids().is_none_or(|(_, active)| active != *doc) { + req.close = true; + return false; + } + let Some((_, key, description)) = app.attachment_description_draft.as_mut() else { + req.close = true; + return false; + }; + ui.heading(tl!("Edit attachment description")); + ui.label(key.as_str()); + let label = ui.label(tl!("Description")); + ui.add(widgets::line(description)).labelled_by(label.id); + } Dialog::Signature => { if !dialog_signature(ui, app, req) { return false; @@ -1585,6 +1604,17 @@ fn dialog_footer(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: if widgets::pill_button(ui, tl!("Cancel"), false).clicked() { req.close = true; } + } else if dialog == Dialog::AttachmentDescription { + if widgets::pill_button(ui, tl!("OK"), true).clicked() { + if let Some((doc, key, description)) = app.attachment_description_draft.as_ref() { + req.attachment_description_now = + Some((*doc, Edit::EditAttachmentDescription { key: key.clone(), description: Some(description.clone()) })); + } + req.close = true; + } + if widgets::pill_button(ui, tl!("Cancel"), false).clicked() { + req.close = true; + } } else if changed { if widgets::pill_button(ui, tl!("OK"), true).clicked() { req.apply = true; @@ -1660,6 +1690,14 @@ fn finish_requests(app: &mut PdfCraftApp, dialog: Dialog, modal_closed: bool, re /// Runs the document actions the dialog asked for. fn finish_actions(app: &mut PdfCraftApp, req: &mut Pending) { + if let Some((doc, edit)) = req.attachment_description_now.take() { + app.attachment_description_draft = None; + if app.active_ids().is_some_and(|(_, active)| active == doc) { + app.apply_edit(edit); + } else { + app.notify_tr("The document changed while the dialog was open, so nothing was changed. Open the description editor again."); + } + } if req.summarize_now { app.summarize_comments(); } @@ -1837,6 +1875,9 @@ fn finish_close(app: &mut PdfCraftApp, dialog: Dialog, modal_closed: bool, req: let replaces = req.link_command.is_some_and(|c| c.starts_with("protect.")) || req.open_revision.is_some(); if modal_closed || req.close || replaces { app.dialog = None; + if dialog == Dialog::AttachmentDescription { + app.attachment_description_draft = None; + } app.props_draft = None; app.view_draft = None; app.custom_draft = None; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 222bb988e4..e847a75dd7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -432,6 +432,8 @@ pub struct PdfCraftApp { pub save_override: Option, /// Document Properties ▸ Description fields being edited: (document, Title/Author/Subject/Keywords). pub props_draft: Option<(DocId, [String; 4])>, + /// Attachment description being edited: (document, name-tree key, description). + pub attachment_description_draft: Option<(DocId, String, String)>, /// Document Properties ▸ Initial View (and reading options) being edited. pub view_draft: Option<(DocId, pdfcraft_engine::InitialView)>, /// Document Properties ▸ Custom being edited. diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 79769867d3..500398651f 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -361,6 +361,13 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) { attachment_action = Some((ai, false)); ui.close(); } + if let Some(key) = deletable + && ui.button(tl!("Edit description")).clicked() + { + app.attachment_description_draft = Some((id, key.clone(), a.description.clone().unwrap_or_default())); + app.dialog = Some(crate::Dialog::AttachmentDescription); + ui.close(); + } if let Some(key) = deletable && ui.button(tl!("Delete attachment")).clicked() { diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 03df25f201..11c89af71e 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -125,6 +125,8 @@ pub enum Dialog { Signature, /// Comment Properties. CommentProps, + /// Edit the description of an embedded document attachment. + AttachmentDescription, /// Replace Pages (after choosing the file). ReplacePages, /// Prepare a form ▸ Field Properties. diff --git a/crates/ui-egui/tests/it/attachment_row.rs b/crates/ui-egui/tests/it/attachment_row.rs index c52b2892e0..144e03ab78 100644 --- a/crates/ui-egui/tests/it/attachment_row.rs +++ b/crates/ui-egui/tests/it/attachment_row.rs @@ -22,7 +22,7 @@ fn attachments_pdf() -> Vec { [format!("<< /Type /EmbeddedFile /Length {} >>\nstream\n", INNER.len()).as_bytes(), INNER, b"\nendstream"].concat(), b"<< /Type /Filespec /F (inner.pdf) /UF (inner.pdf) /EF << /F 4 0 R >> >>".to_vec(), b"<< /Type /EmbeddedFile /Length 11 >>\nstream\nhello notes\nendstream".to_vec(), - b"<< /Type /Filespec /F (notes.txt) /UF (notes.txt) /EF << /F 6 0 R >> >>".to_vec(), + b"<< /Type /Filespec /F (notes.txt) /UF (notes.txt) /Desc (Original) /EF << /F 6 0 R >> >>".to_vec(), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new(); @@ -151,6 +151,25 @@ fn the_right_click_menu_opens_saves_and_deletes() { assert!(doc.can_undo().is_some_and(|l| l.starts_with("Delete attachment")), "as one undoable edit"); } +#[test] +fn the_description_can_be_edited_from_the_row_menu() { + let mut h = harness(); + right_click(&mut h, "notes.txt"); + h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); + h.run_steps(2); + let description = h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Description"); + description.click(); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::A); + description.type_text("Edited in the panel"); + h.get_by_label("OK").click(); + h.run_steps(3); + let state = h.state(); + let doc = state.session.get(state.views[0].id).unwrap(); + let attachment = doc.info.attachments.iter().find(|a| a.name == "notes.txt").unwrap(); + assert_eq!(attachment.description.as_deref(), Some("Edited in the panel")); + assert!(doc.can_undo().is_some_and(|label| label == "Edit attachment description")); +} + #[test] fn the_row_buttons_still_take_their_own_clicks() { let dir = Dir::new("buttons"); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 1404247cd7..4997b56a1c 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3420,11 +3420,11 @@ title = "Edit attachment descriptions" area = "D" tier = "P2" milestone = "M12" -status = "planned" +status = "shipped" commands = [] -tools = [] -evidence = [] -notes = "" +tools = ["attachment_edit"] +evidence = ["crates/organize/src/tests.rs::attachment_description_changes_only_the_file_spec_and_can_be_cleared", "crates/automation/tests/it/automation.rs::attachment_descriptions_can_be_edited_cleared_undone_and_saved", "crates/ui-egui/tests/it/attachment_row.rs::the_description_can_be_edited_from_the_row_menu"] +notes = "The document-level attachment row menu edits only the Filespec /Desc; an empty description clears it. The embedded stream and other file-spec keys stay intact. Undo/redo and the headless attachment_edit tool use the same edit. Page paper-clip comments are not affected." [[feature]] id = "organize.associated-files" From 6d24eedac981daa6aef39c52d3a4b9c3aa567cc8 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:26:11 -0500 Subject: [PATCH 2/6] docs: cover attachment edit helpers --- crates/organize/src/import/carry.rs | 1 + crates/ui-egui/src/dialogs.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/organize/src/import/carry.rs b/crates/organize/src/import/carry.rs index c04c17ec9e..dbe3bd8d1b 100644 --- a/crates/organize/src/import/carry.rs +++ b/crates/organize/src/import/carry.rs @@ -31,6 +31,7 @@ pub(crate) fn set_attachments_exact(dst: &mut Document, mut entries: Vec<(Vec, Object)], uniquify: bool) -> Result<(), OrganizeError> { if entries.is_empty() { return Ok(()); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 220a04393a..9f03e77b79 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1572,6 +1572,7 @@ fn dialog_about(ui: &mut egui::Ui, req: &mut Pending) -> bool { true } +/// Draw the active dialog's footer and record any requested action in `req`. fn dialog_footer(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &mut Pending) { ui.add_space(16.0); let changed = draft_changes(app).is_some_and(|c| !c.is_empty()); From 6ee92add8f7ed7f3908460f93c5bb80b09ffb78f Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:23:51 -0600 Subject: [PATCH 3/6] Test Unicode attachment descriptions --- crates/automation/tests/it/automation.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index b14dd4a75b..ab14794448 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -9598,12 +9598,12 @@ fn attachment_descriptions_can_be_edited_cleared_undone_and_saved() { let doc = ok(&mut a, "doc_open", json!({"path": "a.pdf"}))["doc"].as_u64().unwrap(); ok(&mut a, "attachment_add", json!({"doc": doc, "path": "notes.txt", "description": "Original"})); - ok(&mut a, "attachment_edit", json!({"doc": doc, "name": "notes.txt", "description": "Updated"})); - assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Updated"); + ok(&mut a, "attachment_edit", json!({"doc": doc, "name": "notes.txt", "description": "Résumé 東京"})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Résumé 東京"); ok(&mut a, "edit_undo", json!({"doc": doc})); assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Original"); ok(&mut a, "edit_redo", json!({"doc": doc})); - assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Updated"); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"], "Résumé 東京"); ok(&mut a, "attachment_edit", json!({"doc": doc, "name": "notes.txt", "description": ""})); assert!(ok(&mut a, "doc_info", json!({"doc": doc}))["attachments"][0]["description"].is_null()); @@ -9612,10 +9612,10 @@ fn attachment_descriptions_can_be_edited_cleared_undone_and_saved() { let saved = std::fs::read(dir.join("edited.pdf")).unwrap(); let info = pdfcraft_render::inspect(std::sync::Arc::new(saved.clone()), None).unwrap(); let attachment = info.attachments.iter().find(|item| item.name == "notes.txt").unwrap(); - assert_eq!(attachment.description.as_deref(), Some("Updated")); + assert_eq!(attachment.description.as_deref(), Some("Résumé 東京")); assert_eq!(pdfcraft_render::attachment_data(&saved, None, attachment).unwrap(), bytes); let reopened = ok(&mut a, "doc_open", json!({"path": "edited.pdf"}))["doc"].as_u64().unwrap(); - assert_eq!(ok(&mut a, "doc_info", json!({"doc": reopened}))["attachments"][0]["description"], "Updated"); + assert_eq!(ok(&mut a, "doc_info", json!({"doc": reopened}))["attachments"][0]["description"], "Résumé 東京"); } #[test] From 22778b2be7a5e2719cb3b3e4471d38cd0e83a482 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:06:40 -0600 Subject: [PATCH 4/6] Guard attachment description edits against stale drafts --- crates/automation/src/dispatch.rs | 19 +++++++++- crates/automation/tests/it/automation.rs | 43 +++++++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 28 +++++++++++---- crates/ui-egui/src/lib.rs | 2 +- crates/ui-egui/src/panels/right.rs | 2 +- crates/ui-egui/tests/it/attachment_row.rs | 33 +++++++++++++++++ 6 files changed, 118 insertions(+), 9 deletions(-) diff --git a/crates/automation/src/dispatch.rs b/crates/automation/src/dispatch.rs index 3e29e7d861..6e97b4d4d7 100644 --- a/crates/automation/src/dispatch.rs +++ b/crates/automation/src/dispatch.rs @@ -755,7 +755,24 @@ impl Automation { self.apply(&a, edit)? } "attachment_edit" => { - let edit = Edit::EditAttachmentDescription { key: a.str("name")?.into(), description: Some(a.str("description")?.into()) }; + let name = a.str("name")?; + let keys: Vec = self + .doc(&a)? + .info + .attachments + .iter() + .filter(|attachment| attachment.name == name) + .filter_map(|attachment| match &attachment.source { + pdfcraft_render::AttachmentSource::Document { key } => Some(key.clone()), + pdfcraft_render::AttachmentSource::Annotation { .. } => None, + }) + .collect(); + let key = match keys.as_slice() { + [key] => key.clone(), + [] => return Err(ToolError::InvalidArgs(format!("there is no document attachment named {name:?}; use a name from doc_info"))), + _ => return Err(ToolError::InvalidArgs(format!("more than one document attachment is named {name:?}; choose an unambiguous name"))), + }; + let edit = Edit::EditAttachmentDescription { key, description: Some(a.str("description")?.into()) }; self.apply(&a, edit)? } "doc_remove_hidden" => self.doc_remove_hidden(&a)?, diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index ab14794448..16284baeab 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -9618,6 +9618,49 @@ fn attachment_descriptions_can_be_edited_cleared_undone_and_saved() { assert_eq!(ok(&mut a, "doc_info", json!({"doc": reopened}))["attachments"][0]["description"], "Résumé 東京"); } +#[test] +fn attachment_edit_resolves_the_listed_name_to_its_name_tree_key() { + let dir = workdir("attachment-name-tree-key"); + std::fs::write(dir.join("payload.txt"), b"payload").unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path":"a.pdf"}))["doc"].as_u64().unwrap(); + ok(&mut a, "attachment_add", json!({"doc":doc,"path":"payload.txt","name":"storage-key.txt"})); + ok(&mut a, "doc_save", json!({"doc":doc,"path":"seed.pdf","full":true})); + let mut source = pdfcraft_cos::Document::open(std::sync::Arc::new(std::fs::read(dir.join("seed.pdf")).unwrap())).unwrap(); + let key = "storage-key.txt"; + let root = source.root().unwrap(); + let catalog = source.get(root).as_dict().unwrap().clone(); + let names = source.resolve(catalog.get(b"Names").unwrap()).as_dict().unwrap().clone(); + let tree = source.resolve(names.get(b"EmbeddedFiles").unwrap()).as_dict().unwrap().clone(); + let pairs = source.resolve(tree.get(b"Names").unwrap()).as_array().unwrap().clone(); + let spec_ref = pairs.get(1).and_then(pdfcraft_cos::Object::as_ref).unwrap(); + source + .update_dict(spec_ref, |spec| { + let name = pdfcraft_cos::Object::String(pdfcraft_cos::PdfString::text("visible-name.txt")); + spec.set(b"F".to_vec(), name.clone()); + spec.set(b"UF".to_vec(), name); + }) + .unwrap(); + let bytes = pdfcraft_cos::write_full(&source, &pdfcraft_cos::SaveOptions::default()).unwrap(); + std::fs::write(dir.join("named.pdf"), bytes).unwrap(); + + let doc = ok(&mut a, "doc_open", json!({"path":"named.pdf"}))["doc"].as_u64().unwrap(); + assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc}))["attachments"][0]["name"], "visible-name.txt"); + ok(&mut a, "attachment_edit", json!({"doc":doc,"name":"visible-name.txt","description":"Updated"})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc}))["attachments"][0]["description"], "Updated"); + assert!(matches!( + a.call("attachment_edit", &json!({"doc":doc,"name":"missing.txt","description":"x"})), + Err(ToolError::InvalidArgs(message)) if message.contains("no document attachment named") + )); + ok(&mut a, "doc_save", json!({"doc":doc,"path":"edited.pdf","full":true})); + let saved = pdfcraft_cos::Document::open(std::sync::Arc::new(std::fs::read(dir.join("edited.pdf")).unwrap())).unwrap(); + let catalog = saved.get(saved.root().unwrap()).as_dict().unwrap().clone(); + let names = saved.resolve(catalog.get(b"Names").unwrap()).as_dict().unwrap().clone(); + let tree = saved.resolve(names.get(b"EmbeddedFiles").unwrap()).as_dict().unwrap().clone(); + let pairs = saved.resolve(tree.get(b"Names").unwrap()).as_array().unwrap().clone(); + assert_eq!(pairs.first().and_then(pdfcraft_cos::Object::as_string).map(pdfcraft_cos::PdfString::to_text).as_deref(), Some(key)); +} + #[test] fn fdf_nested_comment_values_survive_export_import_and_save_through_tools() { use pdfcraft_cos::{Dict, Document, Object, PdfString, SaveOptions, write_full}; diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 0450fa9d83..92e60dfe89 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -342,15 +342,18 @@ fn dialog_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &m return false; } Dialog::AttachmentDescription => { - let Some((doc, _, _)) = app.attachment_description_draft.as_ref() else { + let Some((doc, generation, _, _)) = app.attachment_description_draft.as_ref() else { req.close = true; return false; }; - if app.active_ids().is_none_or(|(_, active)| active != *doc) { + if app.active_ids().is_none_or(|(_, active)| active != *doc) + || app.session.get(*doc).is_none_or(|current| current.edit_generation() != *generation) + { + app.attachment_description_draft = None; req.close = true; return false; } - let Some((_, key, description)) = app.attachment_description_draft.as_mut() else { + let Some((_, _, key, description)) = app.attachment_description_draft.as_mut() else { req.close = true; return false; }; @@ -1735,9 +1738,22 @@ fn dialog_footer(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: } } else if dialog == Dialog::AttachmentDescription { if widgets::pill_button(ui, tl!("OK"), true).clicked() { - if let Some((doc, key, description)) = app.attachment_description_draft.as_ref() { - req.attachment_description_now = - Some((*doc, Edit::EditAttachmentDescription { key: key.clone(), description: Some(description.clone()) })); + if let Some((doc, generation, key, description)) = app.attachment_description_draft.as_ref() { + let current = app.session.get(*doc).and_then(|doc| { + doc.info + .attachments + .iter() + .find(|attachment| matches!(&attachment.source, pdfcraft_render::AttachmentSource::Document { key: attachment_key } if attachment_key == key)) + .map(|attachment| attachment.description.as_deref().unwrap_or_default().trim().to_owned()) + }); + let normalized = description.trim(); + let still_current = app.session.get(*doc).is_some_and(|doc| doc.edit_generation() == *generation); + if still_current && current.as_deref().is_some_and(|current| current != normalized) { + req.attachment_description_now = Some(( + *doc, + Edit::EditAttachmentDescription { key: key.clone(), description: Some(normalized.to_owned()) }, + )); + } } req.close = true; } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index b936c56a00..745fb0e87b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -462,7 +462,7 @@ pub struct PdfCraftApp { /// Document Properties ▸ Description fields being edited: (document, Title/Author/Subject/Keywords). pub props_draft: Option<(DocId, [String; 4])>, /// Attachment description being edited: (document, name-tree key, description). - pub attachment_description_draft: Option<(DocId, String, String)>, + pub attachment_description_draft: Option<(DocId, u64, String, String)>, /// Document Properties ▸ Initial View (and reading options) being edited. pub view_draft: Option<(DocId, pdfcraft_engine::InitialView)>, /// Document Properties ▸ Custom being edited. diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 500398651f..3e5fb18ad1 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -364,7 +364,7 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) { if let Some(key) = deletable && ui.button(tl!("Edit description")).clicked() { - app.attachment_description_draft = Some((id, key.clone(), a.description.clone().unwrap_or_default())); + app.attachment_description_draft = Some((id, doc.edit_generation(), key.clone(), a.description.clone().unwrap_or_default())); app.dialog = Some(crate::Dialog::AttachmentDescription); ui.close(); } diff --git a/crates/ui-egui/tests/it/attachment_row.rs b/crates/ui-egui/tests/it/attachment_row.rs index 144e03ab78..ec21a59cb4 100644 --- a/crates/ui-egui/tests/it/attachment_row.rs +++ b/crates/ui-egui/tests/it/attachment_row.rs @@ -170,6 +170,39 @@ fn the_description_can_be_edited_from_the_row_menu() { assert!(doc.can_undo().is_some_and(|label| label == "Edit attachment description")); } +#[test] +fn keeping_the_attachment_description_does_not_create_an_edit() { + let mut h = harness(); + right_click(&mut h, "notes.txt"); + h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + let state = h.state(); + let doc = state.session.get(state.views[0].id).unwrap(); + assert_eq!(doc.info.attachments.iter().find(|a| a.name == "notes.txt").unwrap().description.as_deref(), Some("Original")); + assert!(!doc.dirty, "confirming the unchanged description does not dirty the document"); + assert_eq!(doc.can_undo(), None, "confirming the unchanged description does not add undo history"); +} + +#[test] +fn editing_the_document_while_description_dialog_is_open_discards_its_stale_draft() { + let mut h = harness(); + right_click(&mut h, "notes.txt"); + h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); + h.run_steps(2); + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::EditAttachmentDescription { + key: "notes.txt".into(), + description: Some("Changed elsewhere".into()), + })); + h.get_by_label("OK").click(); + h.run_steps(3); + let state = h.state(); + let doc = state.session.get(state.views[0].id).unwrap(); + assert_eq!(doc.info.attachments.iter().find(|a| a.name == "notes.txt").unwrap().description.as_deref(), Some("Changed elsewhere")); + assert_eq!(doc.can_undo(), Some("Edit attachment description")); +} + #[test] fn the_row_buttons_still_take_their_own_clicks() { let dir = Dir::new("buttons"); From 963d3f1f8c323d32a591b25bfc0b9390c4f844c8 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:10:05 -0600 Subject: [PATCH 5/6] Format attachment description safeguards --- crates/automation/src/dispatch.rs | 6 +++++- crates/ui-egui/src/panels/right.rs | 3 ++- crates/ui-egui/tests/it/attachment_row.rs | 8 ++++---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/automation/src/dispatch.rs b/crates/automation/src/dispatch.rs index 6e97b4d4d7..d7c29d09bf 100644 --- a/crates/automation/src/dispatch.rs +++ b/crates/automation/src/dispatch.rs @@ -770,7 +770,11 @@ impl Automation { let key = match keys.as_slice() { [key] => key.clone(), [] => return Err(ToolError::InvalidArgs(format!("there is no document attachment named {name:?}; use a name from doc_info"))), - _ => return Err(ToolError::InvalidArgs(format!("more than one document attachment is named {name:?}; choose an unambiguous name"))), + _ => { + return Err(ToolError::InvalidArgs(format!( + "more than one document attachment is named {name:?}; choose an unambiguous name" + ))); + } }; let edit = Edit::EditAttachmentDescription { key, description: Some(a.str("description")?.into()) }; self.apply(&a, edit)? diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 3e5fb18ad1..97256ac725 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -364,7 +364,8 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) { if let Some(key) = deletable && ui.button(tl!("Edit description")).clicked() { - app.attachment_description_draft = Some((id, doc.edit_generation(), key.clone(), a.description.clone().unwrap_or_default())); + app.attachment_description_draft = + Some((id, doc.edit_generation(), key.clone(), a.description.clone().unwrap_or_default())); app.dialog = Some(crate::Dialog::AttachmentDescription); ui.close(); } diff --git a/crates/ui-egui/tests/it/attachment_row.rs b/crates/ui-egui/tests/it/attachment_row.rs index ec21a59cb4..b45a3e50cf 100644 --- a/crates/ui-egui/tests/it/attachment_row.rs +++ b/crates/ui-egui/tests/it/attachment_row.rs @@ -191,10 +191,10 @@ fn editing_the_document_while_description_dialog_is_open_discards_its_stale_draf right_click(&mut h, "notes.txt"); h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); h.run_steps(2); - assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::EditAttachmentDescription { - key: "notes.txt".into(), - description: Some("Changed elsewhere".into()), - })); + assert!( + h.state_mut() + .apply_edit(pdfcraft_engine::Edit::EditAttachmentDescription { key: "notes.txt".into(), description: Some("Changed elsewhere".into()) }) + ); h.get_by_label("OK").click(); h.run_steps(3); let state = h.state(); From a47c825befcd6caa642566b95518338a6925aa51 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:23:19 -0600 Subject: [PATCH 6/6] Fix attachment description selection and clearing --- crates/automation/src/dispatch.rs | 27 ++++++++++++--- crates/automation/src/json.rs | 2 +- crates/automation/src/tools.rs | 4 +-- crates/automation/tests/it/automation.rs | 6 ++-- crates/render/src/inspect/model.rs | 2 ++ crates/render/src/inspect/resources.rs | 3 +- crates/ui-egui/src/dialogs.rs | 12 +++---- crates/ui-egui/src/lib.rs | 2 +- crates/ui-egui/src/panels/right.rs | 10 ++++-- crates/ui-egui/tests/it/attachment_row.rs | 42 +++++++++++++++++++++-- 10 files changed, 88 insertions(+), 22 deletions(-) diff --git a/crates/automation/src/dispatch.rs b/crates/automation/src/dispatch.rs index d7c29d09bf..dcd0ae15c6 100644 --- a/crates/automation/src/dispatch.rs +++ b/crates/automation/src/dispatch.rs @@ -755,24 +755,41 @@ impl Automation { self.apply(&a, edit)? } "attachment_edit" => { - let name = a.str("name")?; + let key = a.opt_str("key")?; + let name = a.opt_str("name")?; + if key.is_some() == name.is_some() { + return Err(ToolError::InvalidArgs("provide exactly one of key or name".into())); + } let keys: Vec = self .doc(&a)? .info .attachments .iter() - .filter(|attachment| attachment.name == name) .filter_map(|attachment| match &attachment.source { - pdfcraft_render::AttachmentSource::Document { key } => Some(key.clone()), + pdfcraft_render::AttachmentSource::Document { key: attachment_key } + if key.is_some_and(|key| key == attachment_key) || name.is_some_and(|name| name == attachment.name) => + { + Some(attachment_key.clone()) + } + pdfcraft_render::AttachmentSource::Document { .. } => None, pdfcraft_render::AttachmentSource::Annotation { .. } => None, }) .collect(); let key = match keys.as_slice() { [key] => key.clone(), - [] => return Err(ToolError::InvalidArgs(format!("there is no document attachment named {name:?}; use a name from doc_info"))), + [] if key.is_some() => { + return Err(ToolError::InvalidArgs(format!("there is no document attachment with key {:?}", key.unwrap_or_default()))); + } + [] => { + return Err(ToolError::InvalidArgs(format!( + "there is no document attachment named {:?}; use a name from doc_info", + name.unwrap_or_default() + ))); + } _ => { return Err(ToolError::InvalidArgs(format!( - "more than one document attachment is named {name:?}; choose an unambiguous name" + "more than one document attachment is named {:?}; provide its key from doc_info", + name.unwrap_or_default() ))); } }; diff --git a/crates/automation/src/json.rs b/crates/automation/src/json.rs index 9e93d4c347..7360785be7 100644 --- a/crates/automation/src/json.rs +++ b/crates/automation/src/json.rs @@ -364,7 +364,7 @@ pub(crate) fn info(d: &Document) -> Value { "horizontal": t.horizontal, "inward": t.inward, "direction": t.direction, "scale": t.scale, "opaque": t.opaque, "display": t.display, })).collect::>(), - "attachments": i.attachments.iter().map(|a| json!({ "name": a.name, "description": a.description, "size": a.size })).collect::>(), + "attachments": i.attachments.iter().map(|a| json!({ "name": a.name, "key": match &a.source { pdfcraft_render::AttachmentSource::Document { key } => Some(key), pdfcraft_render::AttachmentSource::Annotation { .. } => None }, "description": a.description, "size": a.size })).collect::>(), // `substitute` names the face standing in for a program the document does not embed, and // `substitution` says why. Both are null for an embedded font: nothing stands in. A null // `substitute` with a non-null `substitution` means no face was available at all, so diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index d3b1030cb8..a8ef3a445b 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -146,8 +146,8 @@ pub fn tools() -> Vec { t("attachment_delete", "Delete an attachment", "Remove the document-level attachment with this name (as doc_info lists it). Undoable.") .destructive() .with(schema(json!({"doc": doc(), "name": {"type": "string"}}), &["doc", "name"])), - t("attachment_edit", "Edit an attachment description", "Change the description of the document-level attachment with this name (as doc_info lists it). An empty description clears it. The file and other attachment data are preserved. Undoable.") - .with(schema(json!({"doc": doc(), "name": {"type": "string"}, "description": {"type": "string"}}), &["doc", "name", "description"])), + t("attachment_edit", "Edit an attachment description", "Change the description of a document attachment by its exact key from doc_info, or by a unique displayed name. An empty description clears it. The file and other attachment data are preserved. Undoable.") + .with(schema(json!({"doc": doc(), "key": {"type": "string"}, "name": {"type": "string"}, "description": {"type": "string"}}), &["doc", "description"])), t("doc_open", "Open a PDF", "Open a PDF file and return its document id, page count and whether it can be edited.") .cmd("file.open") .with(schema(json!({ "path": { "type": "string" }, "password": { "type": "string", "description": "User or owner password for encrypted files." } }), &["path"])), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 16284baeab..93c062526c 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -9645,8 +9645,10 @@ fn attachment_edit_resolves_the_listed_name_to_its_name_tree_key() { std::fs::write(dir.join("named.pdf"), bytes).unwrap(); let doc = ok(&mut a, "doc_open", json!({"path":"named.pdf"}))["doc"].as_u64().unwrap(); - assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc}))["attachments"][0]["name"], "visible-name.txt"); - ok(&mut a, "attachment_edit", json!({"doc":doc,"name":"visible-name.txt","description":"Updated"})); + let attachment = &ok(&mut a, "doc_info", json!({"doc":doc}))["attachments"][0]; + assert_eq!(attachment["name"], "visible-name.txt"); + assert_eq!(attachment["key"], key); + ok(&mut a, "attachment_edit", json!({"doc":doc,"key":key,"description":"Updated"})); assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc}))["attachments"][0]["description"], "Updated"); assert!(matches!( a.call("attachment_edit", &json!({"doc":doc,"name":"missing.txt","description":"x"})), diff --git a/crates/render/src/inspect/model.rs b/crates/render/src/inspect/model.rs index 490897b60f..63aac3d4cc 100644 --- a/crates/render/src/inspect/model.rs +++ b/crates/render/src/inspect/model.rs @@ -506,6 +506,8 @@ pub struct Layer { pub struct Attachment { pub name: String, pub description: Option, + /// Whether the file specification contained `/Desc`, including an empty value. + pub has_description: bool, pub size: Option, pub source: AttachmentSource, } diff --git a/crates/render/src/inspect/resources.rs b/crates/render/src/inspect/resources.rs index 0895c64f8a..a91439885d 100644 --- a/crates/render/src/inspect/resources.rs +++ b/crates/render/src/inspect/resources.rs @@ -227,7 +227,8 @@ impl<'a> Inspector<'a> { .and_then(|d| self.budgeted_text(d, b"UF", text_budget).or_else(|| self.budgeted_text(d, b"F", text_budget))) .or_else(|| fallback(text_budget)) .unwrap_or_else(|| "attachment".into()); - Attachment { name: display, description: d.and_then(|d| self.budgeted_text(d, b"Desc", text_budget)), size, source } + let has_description = d.is_some_and(|d| d.get(b"Desc").is_ok()); + Attachment { name: display, description: d.and_then(|d| self.budgeted_text(d, b"Desc", text_budget)), has_description, size, source } } } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 92e60dfe89..67d225323a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -342,7 +342,7 @@ fn dialog_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &m return false; } Dialog::AttachmentDescription => { - let Some((doc, generation, _, _)) = app.attachment_description_draft.as_ref() else { + let Some((doc, generation, _, _, _, _)) = app.attachment_description_draft.as_ref() else { req.close = true; return false; }; @@ -353,12 +353,12 @@ fn dialog_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &m req.close = true; return false; } - let Some((_, _, key, description)) = app.attachment_description_draft.as_mut() else { + let Some((_, _, _, display_name, description, _)) = app.attachment_description_draft.as_mut() else { req.close = true; return false; }; ui.heading(tl!("Edit attachment description")); - ui.label(key.as_str()); + ui.label(display_name.as_str()); let label = ui.label(tl!("Description")); ui.add(widgets::line(description)).labelled_by(label.id); } @@ -1738,17 +1738,17 @@ fn dialog_footer(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: } } else if dialog == Dialog::AttachmentDescription { if widgets::pill_button(ui, tl!("OK"), true).clicked() { - if let Some((doc, generation, key, description)) = app.attachment_description_draft.as_ref() { + if let Some((doc, generation, key, _, description, _)) = app.attachment_description_draft.as_ref() { let current = app.session.get(*doc).and_then(|doc| { doc.info .attachments .iter() .find(|attachment| matches!(&attachment.source, pdfcraft_render::AttachmentSource::Document { key: attachment_key } if attachment_key == key)) - .map(|attachment| attachment.description.as_deref().unwrap_or_default().trim().to_owned()) + .map(|attachment| (attachment.description.as_deref().unwrap_or_default().trim().to_owned(), attachment.has_description)) }); let normalized = description.trim(); let still_current = app.session.get(*doc).is_some_and(|doc| doc.edit_generation() == *generation); - if still_current && current.as_deref().is_some_and(|current| current != normalized) { + if still_current && current.as_ref().is_some_and(|(current, has_description)| current != normalized || (normalized.is_empty() && *has_description)) { req.attachment_description_now = Some(( *doc, Edit::EditAttachmentDescription { key: key.clone(), description: Some(normalized.to_owned()) }, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 745fb0e87b..f841059311 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -462,7 +462,7 @@ pub struct PdfCraftApp { /// Document Properties ▸ Description fields being edited: (document, Title/Author/Subject/Keywords). pub props_draft: Option<(DocId, [String; 4])>, /// Attachment description being edited: (document, name-tree key, description). - pub attachment_description_draft: Option<(DocId, u64, String, String)>, + pub attachment_description_draft: Option<(DocId, u64, String, String, String, bool)>, /// Document Properties ▸ Initial View (and reading options) being edited. pub view_draft: Option<(DocId, pdfcraft_engine::InitialView)>, /// Document Properties ▸ Custom being edited. diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 97256ac725..677a2bdf73 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -364,8 +364,14 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) { if let Some(key) = deletable && ui.button(tl!("Edit description")).clicked() { - app.attachment_description_draft = - Some((id, doc.edit_generation(), key.clone(), a.description.clone().unwrap_or_default())); + app.attachment_description_draft = Some(( + id, + doc.edit_generation(), + key.clone(), + a.name.clone(), + a.description.clone().unwrap_or_default(), + a.has_description, + )); app.dialog = Some(crate::Dialog::AttachmentDescription); ui.close(); } diff --git a/crates/ui-egui/tests/it/attachment_row.rs b/crates/ui-egui/tests/it/attachment_row.rs index b45a3e50cf..5fdc7cfda4 100644 --- a/crates/ui-egui/tests/it/attachment_row.rs +++ b/crates/ui-egui/tests/it/attachment_row.rs @@ -44,10 +44,14 @@ fn attachments_pdf() -> Vec { /// The app with [`attachments_pdf`] open and the Attachments panel showing, at 60 frames a /// second so two clicks a frame apart make a double-click. fn harness() -> Harness<'static, PdfCraftApp> { - let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_eframe(|_cc| { + harness_with_pdf(attachments_pdf()) +} + +fn harness_with_pdf(pdf: Vec) -> Harness<'static, PdfCraftApp> { + 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("doc.pdf", None, attachments_pdf()).unwrap(); + app.open_bytes("doc.pdf", None, pdf).unwrap(); app.set_option("panel", "attachments").unwrap(); app }); @@ -55,6 +59,40 @@ fn harness() -> Harness<'static, PdfCraftApp> { h } +#[test] +fn submitting_blank_removes_an_existing_whitespace_description() { + let mut pdf = attachments_pdf(); + let desc = pdf.windows(b"(Original)".len()).position(|window| window == b"(Original)").unwrap(); + pdf[desc..desc + b"(Original)".len()].copy_from_slice(b"( )"); + let mut h = harness_with_pdf(pdf); + let doc_id = h.state().views[0].id; + assert!(h.state().session.get(doc_id).unwrap().info.attachments.iter().find(|a| a.name == "notes.txt").unwrap().has_description); + right_click(&mut h, "notes.txt"); + h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + let doc = h.state().session.get(doc_id).unwrap(); + let attachment = doc.info.attachments.iter().find(|a| a.name == "notes.txt").unwrap(); + assert!(!attachment.has_description); + assert!(doc.dirty); +} + +#[test] +fn submitting_blank_leaves_an_absent_description_untouched() { + let mut h = harness(); + right_click(&mut h, "inner.pdf"); + h.get_by_role(egui::accesskit::Role::Menu).get_by_label("Edit description").click(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + let doc = h.state().session.get(h.state().views[0].id).unwrap(); + let attachment = doc.info.attachments.iter().find(|a| a.name == "inner.pdf").unwrap(); + assert!(!attachment.has_description); + assert!(!doc.dirty); + assert_eq!(doc.can_undo(), None); +} + /// A folder of its own for one test's saved file, removed afterwards. struct Dir(std::path::PathBuf);