diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 38ee463577..0c72f09e4b 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -111,6 +111,11 @@ impl PdfCraftApp { system_text_scale: 1.0, ui_scale_applied: None, flatten_fill_sign_on_save: false, + ask_to_lock_signatures: true, + placed_signatures: Default::default(), + pending_lock: None, + lock_answered: None, + lock_remember: false, fill_text_size: fill_sign::TEXT_SIZE, fill_tool: fill_sign::FillTool::Signature, fill_text_color: [0.0; 3], diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index a86c72ef7d..ef3d31a256 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -851,6 +851,7 @@ impl PdfCraftApp { } self.closed_tabs.push(path); } + self.placed_signatures.remove(&id); self.forget_recovery(id); self.session.close(id); } diff --git a/crates/ui-egui/src/app/prefs.rs b/crates/ui-egui/src/app/prefs.rs index 37a11e210e..3455029a0d 100644 --- a/crates/ui-egui/src/app/prefs.rs +++ b/crates/ui-egui/src/app/prefs.rs @@ -103,6 +103,7 @@ impl PdfCraftApp { // Null follows the system's text scaling. "ui_scale": self.ui_scale, "flatten_fill_sign": self.flatten_fill_sign_on_save, + "ask_to_lock_signatures": self.ask_to_lock_signatures, "fill_text_size": self.fill_text_size, "fill_text_border": self.fill_text_default.width, "fill_text_border_color": self.fill_text_default.color, @@ -320,6 +321,9 @@ impl PdfCraftApp { if let Some(on) = v["flatten_fill_sign"].as_bool() { self.flatten_fill_sign_on_save = on; } + if let Some(on) = v["ask_to_lock_signatures"].as_bool() { + self.ask_to_lock_signatures = on; + } if let Some(size) = v["fill_text_size"].as_f64().and_then(fill_sign::valid_text_size) { self.fill_text_size = size; } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 5af43213de..f4ad716137 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -736,6 +736,9 @@ pub struct DocView { pub fill_text: Option, /// A queued Fill & Sign signature: select it after its edit succeeds, then leave placement. pub(crate) fill_signature_page: Option, + /// The "Signature added … Lock now" notice was dismissed; placing another signature shows it + /// again (#2149). + pub(crate) lock_offer_dismissed: bool, pub(crate) signature_drag: crate::signature_drag::SignatureDrag, /// What the page modules claim for the cursor this frame; `document_area` sets it once. pub(crate) cursor: crate::page_cursor::PageCursor, @@ -842,6 +845,8 @@ pub enum ViewAction { CameraLevel, /// After Enhance page: leave the tool for this command (Crop pages, Organize pages). CameraNext(&'static str), + /// Lock now, on the notice shown after a signature is placed (#2149). + LockSignatures, } /// Default page right-click menu actions. @@ -1140,6 +1145,7 @@ impl DocView { marquee_done: None, fill_text: None, fill_signature_page: None, + lock_offer_dismissed: false, signature_drag: Default::default(), cursor: Default::default(), router: Default::default(), @@ -2538,6 +2544,12 @@ fn apply_notice(app: &mut PdfCraftApp, index: usize, notice: Option) { Some(Notice::Console) => app.dialog = Some(crate::Dialog::JsConsole), // Back on in Preferences ▸ General. Some(Notice::HideSecured) => app.show_secured_notice = false, + // Handled by the app with the other view actions (`ViewAction`). + Some(Notice::LockSignatures) => { + if let Some(view) = app.views.get_mut(index) { + view.pending_action = Some(ViewAction::LockSignatures); + } + } None => {} } } @@ -3077,13 +3089,19 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { (app.left_open && app.left == crate::LeftPanel::Tool("edit")) || matches!(app.quick_tool, QuickTool::AddText | QuickTool::EditText); let text_style = app.text_style.clone(); let show_secured_notice = app.show_secured_notice; + // Offer to lock a just-placed signature (#2149) on the same terms the save-time question + // asks: one is still there, saving doesn't lock it anyway, and the document isn't signed. + let lock_offer = !app.flatten_fill_sign_on_save + && app.placed_signatures.contains(&app.views[index].id) + && doc.info.annotations.iter().any(|a| a.fill_sign) + && !doc.is_signed(); let view = &mut app.views[index]; // Opened without the owner password and something is restricted. The notice says so unless // the user turned it off (#4035): the restrictions themselves, and Document Properties ▸ // Security, stay as they are. let secured = show_secured_notice && doc.security_summary().is_some_and(|s| !(s.owner || (s.permissions.modify() && s.permissions.assemble()))); let repaired = !doc.repair_log().is_empty(); - let notice = notices(view, doc, secured, repaired, crate::sign_ui::banner(&doc.signatures), ui, &t); + let notice = notices(view, doc, secured, repaired, lock_offer, crate::sign_ui::banner(&doc.signatures), ui, &t); apply_notice(app, index, notice); let Some(doc) = app.session.get(app.views[index].id) else { return }; let info = &doc.info; diff --git a/crates/ui-egui/src/canvas/bars.rs b/crates/ui-egui/src/canvas/bars.rs index a8a538e86c..85807e28fe 100644 --- a/crates/ui-egui/src/canvas/bars.rs +++ b/crates/ui-egui/src/canvas/bars.rs @@ -56,16 +56,21 @@ pub(super) enum Notice { Close, /// Stop showing the secured-document notice, in every document (#4035). HideSecured, + /// Lock the placed signatures into the page (#2149). + LockSignatures, } /// The notice bar above the pages: a file changed on disk first, then what a historical -/// snapshot is, then pages cut at the renderer's limits, then the signature status bar, a -/// form's scripts having been turned off, security, forms and warnings. +/// snapshot is, then pages cut at the renderer's limits, then the offer to lock a signature just +/// placed, then the signature status bar, a form's scripts having been turned off, security, +/// forms and warnings. +#[allow(clippy::too_many_arguments)] pub(super) fn notices( view: &mut DocView, doc: &pdfcraft_engine::Document, secured: bool, repaired: bool, + lock_offer: bool, signed: Option<(&str, Color32, &str, String)>, ui: &mut egui::Ui, t: &Tokens, @@ -126,6 +131,22 @@ pub(super) fn notices( if view.cut_pages.iter().any(|p| !view.cut_notice_dismissed.contains(p)) { cut_pages_bar(view, ui, t); } + // A signature just placed with Fill & Sign is a comment others can move (#2149): offer to + // lock it into the page now. `lock_offer` is false for a digitally signed document. + if lock_offer && !view.lock_offer_dismissed { + let mut lock = false; + notice_bar(ui, t, "lock", t.accent_text, tl!("Signature added. Others can move it until it's locked.").to_string(), |ui| { + if icons::button(ui, "x", 22.0, false, tl!("Dismiss")).clicked() { + view.lock_offer_dismissed = true; + } + if crate::widgets::pill_button(ui, tl!("Lock now"), true).clicked() { + lock = true; + } + }); + if lock { + return Some(Notice::LockSignatures); + } + } if let Some((icon, color, template, arg)) = signed { let mut open = false; notice_bar(ui, t, icon, color, crate::i18n::fmt(tl!(template), &[("by", &arg)]), |ui| { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 4daf5878db..a2c11c3900 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -60,6 +60,7 @@ struct Pending { revert_now: bool, overwrite_now: bool, overwrite_elsewhere: bool, + lock_answer: Option, summarize_now: bool, optimize_now: bool, duplicate_now: Option, @@ -298,6 +299,11 @@ fn dialog_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, dialog: Dialog, req: &m return false; } } + Dialog::LockSignatures => { + if !dialog_lock_signatures(ui, app, req) { + return false; + } + } Dialog::Revert => { if !dialog_revert(ui, app, req) { return false; @@ -1294,6 +1300,31 @@ fn dialog_summarize_comments(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut false } +/// `Dialog::LockSignatures` content (#2149). Returns `false` when it has its own buttons and no +/// footer is wanted. +fn dialog_lock_signatures(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pending) -> bool { + ui.label(egui::RichText::new(tl!("Lock your signature into the page?")).font(theme::semibold(18.0))); + ui.add_space(8.0); + ui.label(tl!("A placed signature is a comment until it is locked: anyone with a PDF editor can move or remove it. Locking makes your Fill & Sign text, marks and signatures part of the page.")); + ui.add_space(8.0); + ui.add(widgets::checkbox(&mut app.lock_remember, tl!("Don't ask again"))); + ui.add_space(12.0); + ui.with_layout(Layout::right_to_left(Align::Center), |ui| { + if widgets::pill_button(ui, tl!("Lock and save"), true).clicked() { + req.lock_answer = Some(true); + req.close = true; + } + if widgets::pill_button(ui, tl!("Keep editable"), false).clicked() { + req.lock_answer = Some(false); + req.close = true; + } + if widgets::pill_button(ui, tl!("Cancel"), false).clicked() { + req.close = true; + } + }); + false +} + /// `Dialog::Revert` content. Returns `false` when it has its own buttons and no footer is wanted. fn dialog_revert(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pending) -> bool { let name = app.active_ids().and_then(|(_, id)| app.session.get(id)).map(|d| d.name.clone()).unwrap_or_default(); @@ -1832,6 +1863,14 @@ fn finish_requests(app: &mut PdfCraftApp, dialog: Dialog, modal_closed: bool, re if dialog == Dialog::OverwriteChanged && (modal_closed || req.close) { app.pending_overwrite = None; } + if let Some(lock) = req.lock_answer { + let remember = std::mem::take(&mut app.lock_remember); + app.answer_lock_signatures(lock, remember); + } else if dialog == Dialog::LockSignatures && (modal_closed || req.close) { + // Cancelled or dismissed: nothing is saved. + app.pending_lock = None; + app.lock_remember = false; + } } /// Runs the document actions the dialog asked for. @@ -2095,7 +2134,11 @@ fn finish_close(app: &mut PdfCraftApp, dialog: Dialog, modal_closed: bool, req: if modal_closed || req.close || replaces { // Only now: the number in Preferences can pass through 0 on its way to the one wanted. app.recent.truncate(app.recent_limit); - app.dialog = None; + // An answer may have opened another dialog this frame (saving a file that changed on + // disk asks about that): it stays. + if app.dialog == Some(dialog) { + app.dialog = None; + } app.props_draft = None; app.view_draft = None; app.custom_draft = None; diff --git a/crates/ui-egui/src/dialogs/preferences.rs b/crates/ui-egui/src/dialogs/preferences.rs index 830144277c..883d0b68a6 100644 --- a/crates/ui-egui/src/dialogs/preferences.rs +++ b/crates/ui-egui/src/dialogs/preferences.rs @@ -796,6 +796,9 @@ fn card_identity(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, ctx: &mut /// Content for Card 5: Fill & Sign fn card_fill_and_sign(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, ctx: &mut PrefSearchContext) { ctx.checkbox(ui, &mut app.flatten_fill_sign_on_save, tl!("Flatten Fill & Sign when saving"), t); + ui.add_enabled_ui(!app.flatten_fill_sign_on_save, |ui| { + ctx.checkbox(ui, &mut app.ask_to_lock_signatures, tl!("Otherwise, ask on the first save after a signature is placed"), t); + }); ctx.label_secondary(ui, tl!("Text, marks and signatures become part of the page. Other comments stay editable."), t); ui.add_space(8.0); date_format(ui, app, t, ctx); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 0e0b36b787..04d762c9e7 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -10,6 +10,7 @@ mod close; mod save; pub use close::CloseRequest; +pub use save::PendingLock; #[cfg(not(target_arch = "wasm32"))] pub use save::PendingOverwrite; #[cfg(target_arch = "wasm32")] @@ -523,6 +524,11 @@ impl PdfCraftApp { } } Some(crate::canvas::ViewAction::CameraLevel) => self.camera_level(i), + Some(crate::canvas::ViewAction::LockSignatures) => { + if let Some(id) = self.views.get(i).map(|v| v.id) { + self.lock_signatures_now(id); + } + } Some(crate::canvas::ViewAction::CameraNext(command)) => { self.leave_camera(i); self.execute(command); @@ -762,12 +768,22 @@ impl PdfCraftApp { fn apply_queued_edit(&mut self, i: usize) -> bool { let Some(edit) = self.views.get_mut(i).and_then(|v| v.pending_edit.take()) else { return true }; let signature_page = self.views.get_mut(i).and_then(|v| v.fill_signature_page.take()); + let doc_id = self.views.get(i).map(|v| v.id); let committed = self.views.get_mut(i).and_then(|v| v.forms.committed.take()); let typed = match (&edit, &committed) { (Edit::SetFieldValue { name, .. }, Some(draft)) => *name == draft.name, _ => false, }; if self.apply_edit(edit) { + if signature_page.is_some() + && let Some(id) = doc_id + { + self.placed_signatures.insert(id); + // A new signature shows the Lock now notice again, even if the last was dismissed. + if let Some(view) = self.views.get_mut(i) { + view.lock_offer_dismissed = false; + } + } // Tab can wrap to the same field (including another widget of that field). // Its new editor was opened before this edit ran, so it still holds the old // value. Refresh from the accepted form before Save can commit that stale draft. diff --git a/crates/ui-egui/src/editing/close.rs b/crates/ui-egui/src/editing/close.rs index c6d91b64e8..1cc49e321e 100644 --- a/crates/ui-egui/src/editing/close.rs +++ b/crates/ui-egui/src/editing/close.rs @@ -95,6 +95,9 @@ impl PdfCraftApp { view.forms.unbaked.clear(); view.pending_edit = None; } + // A signature placed since the last save is gone with the rest: the next save + // doesn't ask to lock it (#2149). + self.placed_signatures.remove(&id); self.notify_tr("Reverted to the last saved version"); } Err(e) => self.notify_error(e), diff --git a/crates/ui-egui/src/editing/save.rs b/crates/ui-egui/src/editing/save.rs index 35867559f6..6372b99d28 100644 --- a/crates/ui-egui/src/editing/save.rs +++ b/crates/ui-egui/src/editing/save.rs @@ -13,6 +13,14 @@ pub struct PendingOverwrite { after: Box, } +/// A save held back by [`Dialog::LockSignatures`](crate::Dialog::LockSignatures) (#2149). +pub struct PendingLock { + pub id: pdfcraft_engine::DocId, + target: SaveTarget, + /// What was to run once the document is written (close its tab, quit). + after: Box, +} + /// Where Save writes. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum SaveTarget { @@ -48,6 +56,24 @@ impl PdfCraftApp { return false; } let Some(id) = self.views.get(index).map(|v| v.id) else { return false }; + // A signature just placed with Fill & Sign is a removable comment unless it is flattened: + // ask once whether to lock it into the page (#2149). Only while a Fill & Sign mark is still + // there (undo may have taken it away), and never on a digitally signed document: locking + // changes page content in an update, which validators report as a change after signing. + let still_marked = self.session.get(id).is_some_and(|d| d.info.annotations.iter().any(|a| a.fill_sign)); + let signed = self.session.get(id).is_some_and(|d| d.is_signed()); + let answered = self.lock_answered.take() == Some(id); + if self.ask_to_lock_signatures + && !self.flatten_fill_sign_on_save + && self.placed_signatures.contains(&id) + && still_marked + && !signed + && !answered + { + self.pending_lock = Some(PendingLock { id, target, after: Box::new(after) }); + self.dialog = Some(crate::Dialog::LockSignatures); + return false; + } let Some(doc) = self.session.get(id) else { return false }; let (name, path) = (doc.name.clone(), doc.path.clone()); #[cfg(not(target_arch = "wasm32"))] @@ -95,7 +121,10 @@ impl PdfCraftApp { }; match delivered { Ok(()) => { - let _ = self.session.mark_saved(id, bytes, None); + // As the native save does: what was saved isn't asked about again (#2149). + if self.session.mark_saved(id, bytes, None).is_ok() { + self.placed_signatures.remove(&id); + } if let Some(doc) = self.session.get(id) && let Some(view) = self.views.get_mut(index) { @@ -158,6 +187,64 @@ impl PdfCraftApp { } } + /// The Lock signatures dialog answered (#2149): lock the document's Fill & Sign marks into + /// its pages first when `lock`, then save as was asked. With `remember`, the answer becomes + /// the preference (flatten on every save, or never ask again). Nothing changes when locking + /// fails: the document still counts as holding a placed signature, so the next save asks + /// again, and the answer isn't remembered. The placed-signature mark stays until the save + /// succeeds, so a save that fails or is cancelled asks again too. + pub(crate) fn answer_lock_signatures(&mut self, lock: bool, remember: bool) { + let Some(p) = self.pending_lock.take() else { return }; + if lock && !self.flatten_fill_sign_on_save && !self.flatten_fill_sign_in(p.id) { + return; + } + self.lock_answered = Some(p.id); + if remember { + if lock { + self.flatten_fill_sign_on_save = true; + } else { + self.ask_to_lock_signatures = false; + } + } + if let Some(index) = self.views.iter().position(|v| v.id == p.id) { + self.save_then(index, p.target, p.after); + } + } + + /// Lock now, on the notice shown after a signature is placed (#2149): lock document `id`'s + /// Fill & Sign marks into its pages, as the save-time question's Lock does but without + /// saving. Undoable. The next save doesn't ask, as no Fill & Sign mark is left to lock; the + /// placed-signature mark itself stays, so undoing the lock brings back both the notice and the + /// save-time question. Never on a digitally signed document. + pub(crate) fn lock_signatures_now(&mut self, id: pdfcraft_engine::DocId) { + if self.session.get(id).is_none_or(|d| d.is_signed()) { + return; + } + if self.flatten_fill_sign_in(id) { + self.notify_tr("Signature locked into the page."); + } + } + + /// Lock document `id`'s Fill & Sign marks into its pages (an undoable edit). Returns false, + /// having told the user why, when that fails. + fn flatten_fill_sign_in(&mut self, id: pdfcraft_engine::DocId) -> bool { + let before = self.session.get(id).map(|d| d.edit_generation()); + if let Err(e) = self.session.apply(id, pdfcraft_engine::Edit::FlattenFillSign) { + self.notify_error(e); + return false; + } + if before != self.session.get(id).map(|d| d.edit_generation()) + && let Some(doc) = self.session.get(id) + && let Some(view) = self.views.iter_mut().find(|v| v.id == id) + { + view.document_changed(&doc.info); + // The marks left the comments: a selected one is gone, and its (page, index) could + // now name another comment. + view.comments.selected = None; + } + true + } + /// Write document `id` to `dest` and make it the document's file. Returns `true` if written. #[cfg(not(target_arch = "wasm32"))] fn save_doc_to(&mut self, id: pdfcraft_engine::DocId, dest: &str, target: SaveTarget) -> bool { @@ -197,6 +284,7 @@ impl PdfCraftApp { match self.session.mark_saved(id, bytes, Some(dest.to_string())) { Ok(()) => { self.forget_recovery(id); + self.placed_signatures.remove(&id); // The tab in use: `sync_views` brings any other tab of the document up to date. if let Some(doc) = self.session.get(id) && let Some(view) = self.asking_tab(id).and_then(|i| self.views.get_mut(i)) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4a6ab15891..c96765fda6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -368,6 +368,19 @@ pub struct PdfCraftApp { ui_scale_applied: Option, /// Preferences: bake Fill & Sign marks into the page when saving. Off, so a normal save stays editable. pub flatten_fill_sign_on_save: bool, + /// Ask on the first save after Fill & Sign places a signature or initials whether to lock + /// them into the page (#2149). Off once the user says not to ask again. + pub ask_to_lock_signatures: bool, + /// Documents with a Fill & Sign signature placed since their last save, asked about on + /// saving (#2149). Not part of the recovery data: a recovered document isn't asked. + pub(crate) placed_signatures: std::collections::HashSet, + /// A save waiting behind [`Dialog::LockSignatures`], and its Don't ask again box. + pub(crate) pending_lock: Option, + /// The document whose lock question was just answered: the save that follows the answer + /// doesn't ask again. The placed-signature mark itself stays until that save succeeds, so a + /// save that fails or is cancelled asks again next time. + pub(crate) lock_answered: Option, + pub(crate) lock_remember: bool, /// Point size for new Fill & Sign text and dates, kept across launches. pub fill_text_size: f64, pub fill_tool: fill_sign::FillTool, diff --git a/crates/ui-egui/src/panels/tools.rs b/crates/ui-egui/src/panels/tools.rs index 98063e6ef4..24b46cb663 100644 --- a/crates/ui-egui/src/panels/tools.rs +++ b/crates/ui-egui/src/panels/tools.rs @@ -301,6 +301,9 @@ fn tool_detail(app: &mut PdfCraftApp, ui: &mut egui::Ui, t: &Tokens, g: &'static if let Some(cmd) = crate::fill_sign::signature_entries(ui, app, t) { run = Some(cmd); } + // Where the signing happens, not only in Preferences, and once: not also in the Sign + // menu over the page, where it was easy to miss (#2149). + ui.add(widgets::checkbox(&mut app.flatten_fill_sign_on_save, tl!("Lock into the page when saving"))); } }); if footer { diff --git a/crates/ui-egui/src/platform/file_watch.rs b/crates/ui-egui/src/platform/file_watch.rs index 49013d1b74..85ec4d0f5b 100644 --- a/crates/ui-egui/src/platform/file_watch.rs +++ b/crates/ui-egui/src/platform/file_watch.rs @@ -476,8 +476,10 @@ impl PdfCraftApp { let Some(id) = self.views.get(i).map(|v| v.id) else { return }; match self.session.reload(id, bytes) { Ok(()) => { - // Whatever was unsaved is gone, so there is nothing left to recover. + // Whatever was unsaved is gone, so there is nothing left to recover, and no + // signature placed since the last save is left to ask about locking (#2149). self.forget_recovery(id); + self.placed_signatures.remove(&id); if let (Some(view), Some(doc)) = (self.views.get_mut(i), self.session.get(id)) { view.document_replaced(&doc.info); } @@ -581,6 +583,22 @@ mod tests { app.session.get(app.views[0].id).unwrap() } + /// #2348 review (CodeRabbit): a reload from disk replaces the document, so a signature + /// placed since the last save is gone and the next save doesn't ask to lock it. A reload + /// that fails keeps the document, and the mark with it. + #[test] + fn reloading_from_disk_forgets_a_signature_just_placed() { + let (_dir, path) = TempDir::new("lock-reload", &fixture(1)); + let mut h = harness(&path); + h.run_steps(3); + let id = h.state().views[0].id; + h.state_mut().placed_signatures.insert(id); + h.state_mut().reload_view(0, Arc::new(b"not a pdf".to_vec())); + assert!(h.state().placed_signatures.contains(&id), "a failed reload keeps the mark"); + h.state_mut().reload_view(0, Arc::new(fixture(2))); + assert!(!h.state().placed_signatures.contains(&id), "the reloaded document has no placed signature"); + } + /// Association recovery changes ctime without replacing PDF contents. It must neither /// discard undo history nor make the next Save ask to overwrite our own metadata change. #[cfg(target_os = "macos")] diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 7d298aa5ac..65d45c7fcb 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -148,6 +148,9 @@ pub enum Dialog { Revert, /// Save over a file that changed on disk since the document came from it (#2166). OverwriteChanged, + /// Saving a document with a signature just placed by Fill & Sign: lock it into the page + /// first? (#2149) + LockSignatures, /// Link Properties. LinkProps, /// Organize ▸ Extract (options), Rotate Pages. diff --git a/crates/ui-egui/tests/it/fill_sign.rs b/crates/ui-egui/tests/it/fill_sign.rs index 2807f1e2f0..1ae967ea44 100644 --- a/crates/ui-egui/tests/it/fill_sign.rs +++ b/crates/ui-egui/tests/it/fill_sign.rs @@ -4,7 +4,7 @@ use egui::{Pos2, pos2}; use egui_kittest::Harness; use egui_kittest::kittest::{NodeT, Queryable}; use pdfcraft_ui_egui::fill_sign::{FillTool, SavedSig}; -use pdfcraft_ui_egui::{Dialog, PdfCraftApp, QuickTool}; +use pdfcraft_ui_egui::{Dialog, PdfCraftApp, QuickTool, SaveTarget}; const FIXTURE: &[u8] = b"%PDF-1.7 1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj @@ -2135,6 +2135,360 @@ fn photos_and_images_dropped_on_the_signature_pad_are_imported_not_opened() { std::fs::remove_file(photo).unwrap(); } +/// #2149: the first save after Fill & Sign places a signature asks whether to lock it into the +/// page. Lock and save flattens it; Keep editable keeps it a comment; Don't ask again becomes +/// the preference; the Fill & Sign panel has the switch too, once, and the Sign menu no longer +/// does (review on #2348). +#[test] +fn the_first_save_after_placing_a_signature_asks_to_lock_it() { + let out = std::env::temp_dir().join(format!("pdfcraft-lock-signature-{}.pdf", std::process::id())); + let mut h = harness(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + let signed = |h: &Harness<'static, PdfCraftApp>| { + h.state().session.get(h.state().views[0].id).unwrap().info.annotations.iter().filter(|a| a.fill_sign).count() + }; + assert_eq!(signed(&h), 1); + // Keep editable: saved, still a comment, and not asked again for this placement. + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Lock your signature into the page?"); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert!(!h.state().session.get(h.state().views[0].id).unwrap().dirty, "saved"); + assert_eq!(signed(&h), 1, "still a removable comment"); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "no new signature: no question"); + // A new signature asks again; Lock and save with Don't ask again flattens and remembers. + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 200.0); + assert!(!h.state_mut().save_active(SaveTarget::InPlace)); + h.run_steps(2); + h.get_by_label("Don't ask again").click(); + h.run_steps(1); + h.get_by_label("Lock and save").click(); + h.run_steps(3); + assert_eq!(signed(&h), 0, "locked into the page"); + assert!(!h.state().session.get(h.state().views[0].id).unwrap().dirty); + assert!(h.state().flatten_fill_sign_on_save, "remembered as the preference"); + // The switch is in the panel's Sign yourself section, once; the Sign menu over the page, + // where it was easy to miss, no longer has it. + h.state_mut().flatten_fill_sign_on_save = false; + h.get_by_label("Fill & Sign").click(); + h.run_steps(2); + h.get_by_label("Fill & Sign").click(); + h.run_steps(2); + h.get_by_label("Use signature"); + assert!(h.query_by_label("Lock into the page when saving").is_none(), "not in the Sign menu"); + h.key_press(egui::Key::Escape); + h.run_steps(2); + assert!(h.query_by_label("Lock into the page when saving").is_none(), "the panel is closed"); + h.state_mut().left = pdfcraft_ui_egui::LeftPanel::Tool("fill_sign"); + h.state_mut().left_open = true; + h.run_steps(3); + h.get_by_label("Use signature"); + assert_eq!(h.query_all_by_label("Lock into the page when saving").count(), 1, "once, in the panel"); + h.get_by_label("Lock into the page when saving").click(); + h.run_steps(2); + assert!(h.state().flatten_fill_sign_on_save); + let _ = std::fs::remove_file(&out); +} + +/// #2348 review (CodeRabbit): answering the lock question doesn't forget the signature; only a +/// save that succeeds does. When the save after the answer fails, the next save asks again. +#[test] +fn a_save_that_fails_after_the_lock_question_asks_again() { + let bad = std::env::temp_dir().join(format!("pdfcraft-no-such-dir-{}", std::process::id())).join("out.pdf"); + let good = std::env::temp_dir().join(format!("pdfcraft-lock-retry-{}.pdf", std::process::id())); + let mut h = harness(); + h.state_mut().save_override = Some(bad.to_string_lossy().into_owned()); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert!(h.state().session.get(h.state().views[0].id).unwrap().dirty, "the save failed"); + // The save after the answer doesn't ask twice in a row, but the failed one left the signature + // unsaved: the next save asks again. + h.state_mut().save_override = Some(good.to_string_lossy().into_owned()); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks again"); + h.run_steps(2); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert!(!h.state().session.get(h.state().views[0].id).unwrap().dirty, "saved this time"); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "saved: no question"); + let _ = std::fs::remove_file(&good); +} + +/// #2348 review (CodeRabbit): File ▸ Revert discards a signature just placed, so the save after it +/// doesn't ask to lock one, even when the saved file holds an earlier Fill & Sign signature. +#[test] +fn reverting_forgets_a_signature_just_placed() { + let out = std::env::temp_dir().join(format!("pdfcraft-lock-revert-{}.pdf", std::process::id())); + let mut h = harness(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + // An earlier signature, saved as a comment: the file keeps a Fill & Sign mark. + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + // A new signature, then Revert: the new one is gone, the saved one stays. + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 200.0); + h.state_mut().revert_active(); + h.run_steps(3); + let marks = h.state().session.get(h.state().views[0].id).unwrap().info.annotations.iter().filter(|a| a.fill_sign).count(); + assert_eq!(marks, 1, "the saved signature is back, the new one is gone"); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "no signature was placed since the last save: no question"); + let _ = std::fs::remove_file(&out); +} + +/// #2348 review (benandez): right after a signature is placed, a notice offers to lock it into +/// the page now. Lock now flattens it (undoable) without saving, so the save that follows doesn't +/// ask; undoing the lock brings back both the notice and the question. Dismiss hides the notice +/// until the next signature. A digitally signed document, or one whose saves lock anyway, gets no +/// notice. +#[test] +fn placing_a_signature_offers_to_lock_it_now() { + const NOTICE: &str = "Signature added. Others can move it until it's locked."; + let out = std::env::temp_dir().join(format!("pdfcraft-lock-now-{}.pdf", std::process::id())); + let place = |h: &mut Harness<'static, PdfCraftApp>, y: f32| { + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(h, 40.0, y); + h.run_steps(2); + }; + let marks = |h: &Harness<'static, PdfCraftApp>| { + h.state().session.get(h.state().views[0].id).unwrap().info.annotations.iter().filter(|a| a.fill_sign).count() + }; + let mut h = harness(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + assert!(h.query_by_label(NOTICE).is_none(), "nothing placed yet"); + place(&mut h, 300.0); + h.get_by_label(NOTICE); + h.get_by_label("Lock now").click(); + h.run_steps(3); + assert_eq!(marks(&h), 0, "locked into the page"); + assert!(h.query_by_label(NOTICE).is_none(), "nothing left to lock"); + assert!(h.state().session.get(h.state().views[0].id).unwrap().dirty, "locked, not saved"); + // Undo brings the movable signature back, with the notice and the save-time question. + assert!(h.state_mut().execute("edit.undo")); + h.run_steps(3); + assert_eq!(marks(&h), 1); + h.get_by_label(NOTICE); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks again after undoing the lock"); + h.run_steps(2); + h.get_by_label("Cancel").click(); + h.run_steps(2); + // Locked again, the save doesn't ask. + h.get_by_label("Lock now").click(); + h.run_steps(3); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "already locked: no question"); + // Dismiss hides it until the next signature. + place(&mut h, 200.0); + h.get_by_label(NOTICE); + h.get_by_label("Dismiss").click(); + h.run_steps(2); + assert!(h.query_by_label(NOTICE).is_none(), "dismissed"); + place(&mut h, 100.0); + h.get_by_label(NOTICE); + // Once saved, a signature kept as a comment is no longer just placed: no notice. + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert!(marks(&h) > 0, "still a comment"); + assert!(h.query_by_label(NOTICE).is_none(), "saved: nothing just placed"); + place(&mut h, 50.0); + h.get_by_label(NOTICE); + // Saves that lock anyway need no offer. + h.state_mut().flatten_fill_sign_on_save = true; + h.run_steps(2); + assert!(h.query_by_label(NOTICE).is_none(), "saving locks it anyway"); + let _ = std::fs::remove_file(&out); + + // Digitally signed: no notice, as locking would change the document after signing. + let mut doc = { + let mut h = harness(); + let app = h.state_mut(); + let id = app.views[0].id; + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = pdfcraft_engine::sign::pkcs12::open(&p12, "test").unwrap(); + let opts = pdfcraft_engine::SignOptions { page: 0, rect: Some([20.0, 20.0, 160.0, 50.0]), ..Default::default() }; + app.session.sign(id, &digital_id, opts).expect("signs") + }; + let mut h = harness_bytes(&std::mem::take(&mut doc)); + place(&mut h, 300.0); + assert_eq!(marks(&h), 1); + assert!(h.query_by_label(NOTICE).is_none(), "a signed document isn't offered the lock"); +} + +/// #2348 review: locking takes the placed signature out of the comments, so its selection goes +/// too; a stale (page, index) would point at nothing, or at another comment. +#[test] +fn locking_a_signature_clears_the_comment_selection() { + let mut h = harness(); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + h.run_steps(2); + assert!(h.state().views[0].comments.selected.is_some(), "the placed signature is selected"); + h.get_by_label("Lock now").click(); + h.run_steps(3); + assert!(h.state().views[0].comments.selected.is_none(), "nothing left to select"); +} + +/// #2348 review: when locking the signature fails (here the document's security allows comments +/// but not changes), nothing is lost: the next save asks again, and "Don't ask again" isn't +/// remembered. +#[test] +fn a_lock_that_fails_keeps_the_question() { + let out = std::env::temp_dir().join(format!("pdfcraft-lock-fails-{}.pdf", std::process::id())); + let mut doc = pdfcraft_cos::Document::open(std::sync::Arc::new(FIXTURE.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 mut h = harness_bytes(&pdfcraft_cos::write_full(&doc, &Default::default()).unwrap()); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + let signed = |h: &Harness<'static, PdfCraftApp>| { + h.state().session.get(h.state().views[0].id).unwrap().info.annotations.iter().filter(|a| a.fill_sign).count() + }; + assert_eq!(signed(&h), 1, "comments are allowed"); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Don't ask again").click(); + h.run_steps(1); + h.get_by_label("Lock and save").click(); + h.run_steps(3); + assert_eq!(signed(&h), 1, "changes aren't allowed: still a comment"); + assert!(!h.state().flatten_fill_sign_on_save, "the failed answer isn't remembered"); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "the next save asks again"); + h.run_steps(2); + h.get_by_label("Lock your signature into the page?"); + let _ = std::fs::remove_file(&out); +} + +/// #2348 review: the lock question stays out of the way where it must. Answering it on a file +/// that changed on disk leaves the changed-on-disk question showing (it was closed with the lock +/// question, so the save silently didn't happen); a signature placed and then undone asks +/// nothing; and a digitally signed document isn't asked, since locking would change it after +/// signing. +#[test] +fn the_lock_question_is_skipped_or_kept_where_it_must_be() { + let place = |h: &mut Harness<'static, PdfCraftApp>| { + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(h, 40.0, 300.0); + }; + // A file that changed on disk. + let path = std::env::temp_dir().join(format!("pdfcraft-lock-changed-{}.pdf", std::process::id())); + std::fs::write(&path, FIXTURE).unwrap(); + let mut h = harness_bytes(FIXTURE); + h.state_mut().close_tab(0); + h.state_mut().open_path(&path.to_string_lossy()); + for _ in 0..100 { + if !h.state().views.is_empty() { + break; + } + h.run_steps(1); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + h.run_steps(3); + place(&mut h); + std::fs::write(&path, [FIXTURE, b"\n% rewritten elsewhere\n".as_slice()].concat()).unwrap(); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asks first"); + h.run_steps(2); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(Dialog::OverwriteChanged), "the changed-on-disk question shows"); + let _ = std::fs::remove_file(&path); + + // Placed, then undone: nothing to lock, so the save just saves. + let out = std::env::temp_dir().join(format!("pdfcraft-lock-undone-{}.pdf", std::process::id())); + let mut h = harness(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + place(&mut h); + assert!(h.state_mut().execute("edit.undo")); + h.run_steps(2); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "no question after the signature was undone"); + let _ = std::fs::remove_file(&out); + + // Digitally signed: never asked. + let mut doc = { + let mut h = harness(); + let app = h.state_mut(); + let id = app.views[0].id; + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = pdfcraft_engine::sign::pkcs12::open(&p12, "test").unwrap(); + let opts = pdfcraft_engine::SignOptions { page: 0, rect: Some([20.0, 20.0, 160.0, 50.0]), ..Default::default() }; + app.session.sign(id, &digital_id, opts).expect("signs") + }; + let mut h = harness_bytes(&std::mem::take(&mut doc)); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + place(&mut h); + let id = h.state().views[0].id; + assert!(h.state().session.get(id).unwrap().is_signed()); + assert!(h.state_mut().save_active(SaveTarget::InPlace), "a signed document isn't asked"); + let _ = std::fs::remove_file(&out); +} + +/// #2149: Cancel saves nothing, and Keep editable with Don't ask again turns the question off. +#[test] +fn cancelling_the_lock_question_saves_nothing() { + let out = std::env::temp_dir().join(format!("pdfcraft-lock-cancel-{}.pdf", std::process::id())); + let _ = std::fs::remove_file(&out); + let mut h = harness(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.state_mut().signature = Some(SavedSig::Typed("Ada Lovelace".into())); + h.state_mut().execute("sign.fill.signature"); + h.run_steps(2); + click(&mut h, 40.0, 300.0); + assert!(!h.state_mut().save_active(SaveTarget::InPlace)); + h.run_steps(2); + h.get_by_label("Cancel").click(); + h.run_steps(3); + assert!(!out.exists() && h.state().session.get(h.state().views[0].id).unwrap().dirty, "nothing saved"); + assert!(!h.state_mut().save_active(SaveTarget::InPlace), "asked again"); + h.run_steps(2); + h.get_by_label("Don't ask again").click(); + h.run_steps(1); + h.get_by_label("Keep editable").click(); + h.run_steps(3); + assert!(out.exists() && !h.state().ask_to_lock_signatures); + let restored = { + let mut a = PdfCraftApp::new(); + a.restore(&h.state().persist()); + a.ask_to_lock_signatures + }; + assert!(!restored, "kept with the settings"); + let _ = std::fs::remove_file(&out); +} + /// Where a mode bar tab is, by its label. fn mode_tab(h: &Harness<'static, PdfCraftApp>, label: &str) -> egui::Rect { h.get_all_by_role_and_label(egui::accesskit::Role::Button, label).map(|n| n.rect()).find(|r| r.bottom() < 120.0).expect("a mode tab") diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 2ed8365a73..126496547d 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -137,7 +137,7 @@ workflows, exchange edge cases and everything outside the mainstream slice are e | Print | 8% | 55% | Good on macOS and Linux; Windows prints since #847 (#756), not yet re-scored | | Highlight, sticky notes, text comments | 12% | 85% | All markup types, replies, properties | | Fill in forms | 10% | 85% | Every AcroForm field type, Acrobat's formatting and calculations | -| Fill & Sign: typed, drawn or image signature | 10% | 85% | Fill & Sign tools; flatten on save | +| Fill & Sign: typed, drawn or image signature | 10% | 85% | Fill & Sign tools; flatten on save (opt-in, and asked on the first save after signing) | | Rotate, delete, reorder and extract pages | 8% | 85% | Organize grid, drag reorder, thumbnails | | Combine files | 6% | 85% | Combine tab with bookmarks per file | | Fix a typo in existing text | 5% | 40% | Latin paragraphs only; font may be substituted | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 92fe95cbb0..577104746d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5543,8 +5543,8 @@ milestone = "M5" status = "shipped" commands = ["comment.flatten"] tools = ["doc_save", "doc_flatten"] -evidence = ["crates/edit/src/tests.rs::fill_sign_flatten_bakes_marks_keeps_other_comments_and_does_not_reuse_pcfl0", "crates/engine/src/tests.rs::flattening_fill_and_sign_keeps_other_comments_fields_and_undoes", "crates/engine/src/tests.rs::flattening_fill_and_sign_is_refused_without_permission_to_change", "crates/automation/tests/it/automation.rs::saving_with_flatten_fill_sign_bakes_marks_and_leaves_other_comments", "crates/ui-egui/tests/it/fill_sign.rs::saving_flattens_fill_and_sign_only_when_the_preference_is_on"] -notes = "Preferences ▸ Fill & Sign, and doc_save flatten_fill_sign, bake text, marks and signatures into the page. The option defaults off, so a normal save leaves them editable. Other comments and form fields stay. Flatten comments still flattens every comment." +evidence = ["crates/edit/src/tests.rs::fill_sign_flatten_bakes_marks_keeps_other_comments_and_does_not_reuse_pcfl0", "crates/engine/src/tests.rs::flattening_fill_and_sign_keeps_other_comments_fields_and_undoes", "crates/engine/src/tests.rs::flattening_fill_and_sign_is_refused_without_permission_to_change", "crates/automation/tests/it/automation.rs::saving_with_flatten_fill_sign_bakes_marks_and_leaves_other_comments", "crates/ui-egui/tests/it/fill_sign.rs::saving_flattens_fill_and_sign_only_when_the_preference_is_on", "crates/ui-egui/tests/it/fill_sign.rs::placing_a_signature_offers_to_lock_it_now", "crates/ui-egui/tests/it/fill_sign.rs::the_first_save_after_placing_a_signature_asks_to_lock_it", "crates/ui-egui/tests/it/fill_sign.rs::cancelling_the_lock_question_saves_nothing", "crates/ui-egui/tests/it/fill_sign.rs::a_lock_that_fails_keeps_the_question", "crates/ui-egui/tests/it/fill_sign.rs::the_lock_question_is_skipped_or_kept_where_it_must_be"] +notes = "Preferences ▸ Fill & Sign, and doc_save flatten_fill_sign, bake text, marks and signatures into the page. The option defaults off, so a normal save leaves them editable; right after a signature or initials is placed, a notice offers Lock now (undoable, no save), and the first save after one is placed asks whether to lock it into the page (Lock and save / Keep editable, with Don't ask again); the Fill & Sign panel's Sign yourself section carries the same switch (#2149). Not on a digitally signed document. Other comments and form fields stay. Flatten comments still flattens every comment." [[feature]] id = "form.autofill-profile"