Merge #2348: M5: the first save after placing a signature asks to lock it into the page (#2149)

# Conflicts:
#	crates/ui-egui/src/app/files.rs
#	crates/ui-egui/src/canvas/bars.rs
#	crates/ui-egui/src/editing/save.rs

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],

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);
         }

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;
         }

crates/ui-egui/src/canvas.rs

@@ -736,6 +736,9 @@ pub struct DocView {
     pub fill_text: Option<crate::fill_sign::TypeBox>,
     /// A queued Fill & Sign signature: select it after its edit succeeds, then leave placement.
     pub(crate) fill_signature_page: Option<usize>,
+    /// 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<Notice>) {
         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;

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| {

crates/ui-egui/src/dialogs.rs

@@ -60,6 +60,7 @@ struct Pending {
     revert_now: bool,
     overwrite_now: bool,
     overwrite_elsewhere: bool,
+    lock_answer: Option<bool>,
     summarize_now: bool,
     optimize_now: bool,
     duplicate_now: Option<Edit>,
@@ -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;

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);

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.

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),

crates/ui-egui/src/editing/save.rs

@@ -13,6 +13,14 @@ pub struct PendingOverwrite {
     after: Box<dyn FnOnce(&mut PdfCraftApp) + Send>,
 }
 
+/// 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<dyn FnOnce(&mut PdfCraftApp) + Send>,
+}
+
 /// 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))

crates/ui-egui/src/lib.rs

@@ -368,6 +368,19 @@ pub struct PdfCraftApp {
     ui_scale_applied: Option<f32>,
     /// 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<pdfcraft_engine::DocId>,
+    /// A save waiting behind [`Dialog::LockSignatures`], and its Don't ask again box.
+    pub(crate) pending_lock: Option<editing::PendingLock>,
+    /// 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<pdfcraft_engine::DocId>,
+    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,

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 {

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")]

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.

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")

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 |

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"