From 9f2188bf7ec908e16a175b0c7b669bdbaddef187 Mon Sep 17 00:00:00 2001 From: Hal Goff Date: Thu, 8 Oct 2026 15:02:18 +0100 Subject: [PATCH] Issue #166: Save, Print and Export include the text still being typed in a form field (#278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Issue #166: Save, Print and Export include the text still being typed in a form field A field's editor kept its text in a draft until Enter, Tab or clicking away, and ⌘S (which runs while a field has focus) saved the document first. Typing also didn't count as unsaved, so closing or quitting didn't ask. - commit_form_typing(): apply the tab's queued edit, then commit the open field's draft; if the field refuses the value, keep the editor and text and stop. Called where the document is read: Save, Save As (and its later callback), Print, Export, Reduce/Optimize, Sign, Extract, Split, Copy pages, Flatten, button scripts. - Uncommitted typing counts as unsaved work for Close, Close All and Quit; saving from the prompt brings that tab forward first. - Shortcuts pressed while a form field's editor is open run one frame later, for that document only, so the field takes the frame's typing and Escape. - A value refused on Enter/Tab/blur reopens the field with the text. - Notices show in full screen too. Co-authored-by: Hal Goff Co-Authored-By: Claude Opus 5.5 * Add my contributor name (AutoHenry) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- contributors/people.toml | 5 + crates/ui-egui/src/commands.rs | 21 ++- crates/ui-egui/src/create_ui.rs | 4 + crates/ui-egui/src/editing.rs | 115 +++++++++++++- crates/ui-egui/src/export_ui.rs | 4 + crates/ui-egui/src/files.rs | 20 +++ crates/ui-egui/src/forms_ui.rs | 28 +++- crates/ui-egui/src/js_ui.rs | 4 + crates/ui-egui/src/lib.rs | 19 ++- crates/ui-egui/src/optimize_ui.rs | 4 + crates/ui-egui/src/print_ui.rs | 4 + crates/ui-egui/src/sign_ui.rs | 14 +- crates/ui-egui/tests/forms.rs | 253 ++++++++++++++++++++++++++++++ 13 files changed, 473 insertions(+), 22 deletions(-) diff --git a/contributors/people.toml b/contributors/people.toml index e0de8278d9..c0549e9d50 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -31,3 +31,8 @@ sync_github_name = false real_name = "Jose Antonio Abad" display_name = "jaabadmi" sync_github_name = false + +[people.AutoHenry] +real_name = "Hal Goff" +display_name = "Hal Goff, HG Studio" +sync_github_name = false diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 34a86cc557..34e96c91dd 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -187,10 +187,10 @@ impl PdfCraftApp { self.apply_edit(Edit::Flatten { comments: true, fields: false }); } "form.flatten" => { - if let Some(i) = active { - self.views[i].forms.focus = None; + // Flatten what's typed in a field too (#166); a refused value flattens nothing. + if self.commit_form_typing() { + self.apply_edit(Edit::Flatten { comments: false, fields: true }); } - self.apply_edit(Edit::Flatten { comments: false, fields: true }); } "form.clear" => { if let Some(i) = active { @@ -503,6 +503,11 @@ impl PdfCraftApp { pub(crate) fn registry_shortcuts(&mut self, ctx: &egui::Context) { use egui::{Key, KeyboardShortcut, Modifiers}; let typing = ctx.egui_wants_keyboard_input(); + // A form field's editor is open on the page: its text isn't in the document until + // committed. (Not egui's keyboard focus: an Escape in this frame has already cleared that, + // while the field has yet to see the Escape and discard its draft.) + let active = self.active_ids(); + let form_typing = active.and_then(|(i, _)| self.views.get(i)).is_some_and(|v| v.forms.focus.is_some()); let mut specs: Vec<&CommandSpec> = COMMANDS.iter().filter(|c| c.shortcut.is_some()).collect(); specs.sort_by_key(|c| std::cmp::Reverse(c.shortcut.map(|s| s.modifier_count()).unwrap_or(0))); for spec in specs { @@ -522,7 +527,15 @@ impl PdfCraftApp { m |= Modifiers::CTRL; } if ctx.input_mut(|i| i.consume_shortcut(&KeyboardShortcut::new(m, key))) { - self.execute(spec.id); + if form_typing { + // A form field has the keyboard: let it take this frame's typing (and + // Escape) first, so ⌘S saves what's on screen (#166). Runs next frame, for + // this document only. + self.deferred_commands.push((spec.id, active.map(|(_, id)| id))); + ctx.request_repaint(); + } else { + self.execute(spec.id); + } } } } diff --git a/crates/ui-egui/src/create_ui.rs b/crates/ui-egui/src/create_ui.rs index 17c4be2998..48c8a1cbb5 100644 --- a/crates/ui-egui/src/create_ui.rs +++ b/crates/ui-egui/src/create_ui.rs @@ -243,6 +243,10 @@ impl PdfCraftApp { /// Reduce File Size: write a compacted copy with images downsampled (the open document is /// unchanged). pub(crate) fn reduce_file_size(&mut self) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let result = self.session.reduced_bytes(id).map(|(b, _)| (b, String::new())); self.save_optimized(id, "reduced", result); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 26daf6e3d2..51d97cb4c3 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -102,10 +102,13 @@ impl PdfCraftApp { } /// Apply any edit a view queued this frame (organize toolbar, keys). - pub(crate) fn process_pending_edits(&mut self) { - let Some(i) = self.active else { return }; - if let Some(edit) = self.views.get_mut(i).and_then(|v| v.pending_edit.take()) { - self.apply_edit(edit); + /// Returns false when a field refused the typing queued for it (its editor is open again). + pub(crate) fn process_pending_edits(&mut self) -> bool { + let Some(i) = self.active else { return true }; + let applied = self.apply_queued_edit(i); + if !applied { + // Nor may a shortcut pressed with it (⌘S right after Enter) run behind the refusal. + self.deferred_commands.clear(); } match self.views.get_mut(i).and_then(|v| v.pending_action.take()) { Some(crate::canvas::ViewAction::InsertFromFile) => self.insert_from_file_dialog(), @@ -115,11 +118,16 @@ impl PdfCraftApp { Some(crate::canvas::ViewAction::PastePages) => self.paste_pages(), None => {} } + applied } /// Organize ▸ Copy / Cut: remember the selected pages (the document as it is now); Cut also /// deletes them (one page always stays). pub fn copy_pages(&mut self, cut: bool) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((i, id)) = self.active_ids() else { return }; let Some(doc) = self.session.get(id) else { return }; let pages = self.views[i].target_pages(); @@ -154,6 +162,80 @@ impl PdfCraftApp { } } + /// Make the active document say what its form shows (#166): apply an edit still queued on + /// its tab, then commit the text in a field's open editor, which otherwise commits only on + /// Enter, Tab or clicking away (the editor closes, as clicking away would). Run before + /// anything reads the document: Save, Print, Export, Optimize, Sign, Extract, Split, Copy + /// pages and button scripts. + /// + /// Returns false when the field rejects the value (a validation script or format; the user + /// is told why). The editor stays open with the text, and the caller must stop: nothing is + /// saved, printed or exported without it. + pub(crate) fn commit_form_typing(&mut self) -> bool { + let Some((i, id)) = self.active_ids() else { return true }; + if !self.apply_queued_edit(i) { + return false; + } + let Some(draft) = self.views.get(i).and_then(|v| v.forms.focus.clone()) else { return true }; + let Some(edit) = self.session.get(id).and_then(|d| crate::forms_ui::draft_edit(&draft, &d.form)) else { + return true; // nothing typed that the document doesn't already have + }; + if let Some(view) = self.views.get_mut(i) { + view.forms.focus = None; + } + if self.apply_edit(edit) { + return true; + } + if let Some(view) = self.views.get_mut(i) { + let mut draft = draft; + draft.request_focus = true; + view.forms.focus = Some(draft); + } + false + } + + /// Apply the edit queued on tab `i` (the active one). Returns false only when it was a + /// field's typing and the field refused it (#166): the editor reopens with the text, and the + /// user has been told why, so the typing isn't lost and the caller can stop. + 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 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) || !typed { + return true; + } + if let (Some(mut draft), Some(view)) = (committed, self.views.get_mut(i)) { + draft.request_focus = true; + view.forms.focus = Some(draft); + } + false + } + + /// [`Self::commit_form_typing`] for document `id`, which may not be the active tab (a save + /// panel answering later, Close All). If its tab has typing or a queued edit, the tab is + /// brought forward first, so the commit and any field scripts act on that document. + pub(crate) fn commit_typing_in(&mut self, id: pdfcraft_engine::DocId) -> bool { + let Some(i) = self.views.iter().position(|v| v.id == id) else { return true }; + if self.active != Some(i) { + let typing = self.views.get(i).is_some_and(|v| v.pending_edit.is_some() || v.forms.focus.is_some()); + if !typing { + return true; + } + self.active = Some(i); + } + self.commit_form_typing() + } + + /// Whether tab `index` has work that isn't saved: edits, or form typing not committed yet. + pub(crate) fn has_unsaved_work(&self, index: usize) -> bool { + let Some(v) = self.views.get(index) else { return false }; + let Some(doc) = self.session.get(v.id) else { return false }; + doc.dirty || v.pending_edit.is_some() || v.forms.focus.as_ref().is_some_and(|f| crate::forms_ui::draft_edit(f, &doc.form).is_some()) + } + /// Save the active document. Returns `true` if it was written. pub fn save_active(&mut self, target: SaveTarget) -> bool { match self.active { @@ -172,6 +254,12 @@ impl PdfCraftApp { /// [`Self::save_view`], then `after` once the document is written: now, or on a later frame /// when the user had to choose where. `after` never runs when the save fails or is cancelled. pub(crate) fn save_then(&mut self, index: usize, target: SaveTarget, after: impl FnOnce(&mut Self) + Send + 'static) -> bool { + // What's typed in a field is part of what's saved (#166). + if let Some(id) = self.views.get(index).map(|v| v.id) + && !self.commit_typing_in(id) + { + return false; + } let Some(id) = self.views.get(index).map(|v| v.id) else { return false }; let Some(doc) = self.session.get(id) else { return false }; let (name, path) = (doc.name.clone(), doc.path.clone()); @@ -229,6 +317,11 @@ impl PdfCraftApp { /// 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) -> bool { + // Save As answers on a later frame: commit what was typed meanwhile, too (#166), even + // if another tab is active by then. + if !self.commit_typing_in(id) { + return false; + } let Some(name) = self.session.get(id).map(|d| d.name.clone()) else { self.notify_tr("The document was closed before it could be saved."); return false; @@ -264,8 +357,10 @@ impl PdfCraftApp { /// Close a tab, asking first if it has unsaved changes. pub fn request_close_tab(&mut self, index: usize) { - let dirty = self.views.get(index).and_then(|v| self.session.get(v.id)).is_some_and(|d| d.dirty); - if dirty && let Some(id) = self.views.get(index).map(|v| v.id) { + // Text typed into a field counts as an unsaved change. + if self.has_unsaved_work(index) + && let Some(id) = self.views.get(index).map(|v| v.id) + { self.close_request = Some(CloseRequest::Tab(id)); } else { self.close_tab(index); @@ -275,7 +370,7 @@ impl PdfCraftApp { /// File ▸ Close all: clean documents close at once; each one with unsaved changes asks. pub fn close_all(&mut self) { for i in (0..self.views.len()).rev() { - if !self.session.get(self.views[i].id).is_some_and(|d| d.dirty) { + if !self.has_unsaved_work(i) { self.close_tab(i); } } @@ -302,7 +397,7 @@ impl PdfCraftApp { /// The first tab with unsaved changes. pub fn first_dirty(&self) -> Option { - self.views.iter().position(|v| self.session.get(v.id).is_some_and(|d| d.dirty)) + (0..self.views.len()).find(|&i| self.has_unsaved_work(i)) } /// Answer the save prompt: `Some(true)` save, `Some(false)` discard, `None` cancel. @@ -328,6 +423,9 @@ impl PdfCraftApp { Some(false) => self.close_and_continue(ctx, index, req), Some(true) => { let Some(id) = self.views.get(index).map(|v| v.id) else { return }; + // Bring the document forward: what's typed in its form is committed and saved + // with it, and a field that rejects the value is shown where the user can fix it. + self.active = Some(index); // Close only once the save has actually been written, which may be on a later // frame when the user has to choose where. A failed or cancelled save keeps the // document open. @@ -364,6 +462,7 @@ impl PdfCraftApp { if !ctx.input(|i| i.viewport().close_requested()) { return; } + // `first_dirty` counts text typed into a field as an unsaved change. if !self.allow_quit && self.first_dirty().is_some() { ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); self.close_request = Some(CloseRequest::Quit); diff --git a/crates/ui-egui/src/export_ui.rs b/crates/ui-egui/src/export_ui.rs index 684d32fbba..03f322c7b6 100644 --- a/crates/ui-egui/src/export_ui.rs +++ b/crates/ui-egui/src/export_ui.rs @@ -206,6 +206,10 @@ fn run( impl PdfCraftApp { /// Start exporting the active document with the dialog's settings. pub(crate) fn start_export(&mut self, kind: ExportKind) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let Some(doc) = self.session.get(id) else { return }; let src = doc.export_source(); diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index f7cc0714c5..d4c492a1c9 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -286,6 +286,10 @@ impl PdfCraftApp { /// Copy the selected pages (or the current page) into a new unsaved document tab. pub fn extract_selection(&mut self) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((i, id)) = self.active_ids() else { return }; let pages = self.views[i].target_pages(); let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default(); @@ -408,6 +412,10 @@ impl PdfCraftApp { /// Split the active document and write the parts: into a chosen folder (desktop, written on /// a later frame once the user has chosen it) or as downloads (web). pub fn split_active(&mut self, plan: &SplitPlan) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_else(|| "document".into()); let (parts, titles) = match plan { @@ -502,6 +510,10 @@ impl PdfCraftApp { /// Export all comments / form data: the format follows the file name's extension. /// Export a PDF ▸ Word, HTML or RTF: ask where (`save_override` in tests), then write. pub fn export_office_dialog(&mut self, format: pdfcraft_engine::compare::OfficeFormat) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let Some(doc) = self.session.get(id) else { return }; let stem = doc.name.trim_end_matches(".pdf").trim_end_matches(".PDF").to_string(); @@ -593,12 +605,20 @@ impl PdfCraftApp { } pub fn export_data_dialog(&mut self, comments: bool, fields: bool) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let Some(doc) = self.session.get(id) else { return }; let stem = doc.name.trim_end_matches(".pdf").trim_end_matches(".PDF").to_string(); #[cfg(not(target_arch = "wasm32"))] { let write = move |app: &mut Self, path: std::path::PathBuf| { + // Include what was typed while the save panel was open (#166). + if !app.commit_typing_in(id) { + return; + } let ext = path.extension().map(|e| e.to_string_lossy().into_owned()).unwrap_or_default(); let format = pdfcraft_engine::DataFormat::from_extension(&ext).unwrap_or(pdfcraft_engine::DataFormat::Xfdf); match app.session.export_data(id, format, comments, fields) { diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index 79e9660b12..866b58d278 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -34,6 +34,9 @@ pub struct Focus { #[derive(Clone, Debug, Default)] pub struct FormView { pub focus: Option, + /// The draft whose edit is queued (`DocView::pending_edit`): if the field refuses the value, + /// the editor reopens with it rather than losing the typing. + pub(crate) committed: Option, /// A message for the app to show (e.g. "buttons run JavaScript"). pub notice: Option, /// A push button was clicked: (its field name, what it does). @@ -141,16 +144,25 @@ pub(crate) fn page_input( true } -/// The edit for the focused field, if its draft differs from the field's value. -fn commit(view: &mut DocView, form: &[FormField]) { - let Some(focus) = view.forms.focus.take() else { return }; - let Some(f) = form.iter().find(|f| f.name == focus.name) else { return }; +/// The edit the focused field's draft makes, if it differs from the field's value. +pub(crate) fn draft_edit(focus: &Focus, form: &[FormField]) -> Option { + let f = form.iter().find(|f| f.name == focus.name)?; let value = match f.kind { - FormFieldKind::Text if f.value.first().map(String::as_str).unwrap_or("") != focus.text => FieldValue::Text(focus.text), - FormFieldKind::List if f.has(field_flags::MULTI_SELECT) && focus.picked != f.value => FieldValue::Choice(focus.picked), - _ => return, + FormFieldKind::Text if f.value.first().map(String::as_str).unwrap_or("") != focus.text => FieldValue::Text(focus.text.clone()), + FormFieldKind::List if f.has(field_flags::MULTI_SELECT) && focus.picked != f.value => FieldValue::Choice(focus.picked.clone()), + _ => return None, }; - view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value }); + Some(Edit::SetFieldValue { name: f.name.clone(), value }) +} + +/// Close the focused field's editor, queueing the edit for its draft if it differs from the +/// field's value. +pub(crate) fn commit(view: &mut DocView, form: &[FormField]) { + let Some(focus) = view.forms.focus.take() else { return }; + if let Some(edit) = draft_edit(&focus, form) { + view.pending_edit = Some(edit); + view.forms.committed = Some(focus); + } } const MONTHS: [&str; 12] = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"]; diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index eb72fa0550..30121855b3 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -59,6 +59,10 @@ impl PdfCraftApp { /// Run a push button's JavaScript (its Mouse Up action). pub fn run_button_script(&mut self, id: DocId, field: &str, script: &str) { + // A script reads the fields: include what's still being typed in one (#166). + if !self.commit_form_typing() { + return; + } match self.session.run_javascript(id, script, Some(field)) { Ok(o) => { if let Some(i) = self.views.iter().position(|v| v.id == id) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 67afcf8a2c..147bebb527 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -443,6 +443,9 @@ pub struct PdfCraftApp { last_autosave: f64, pending_recovered: Option, allow_quit: bool, + /// Shortcuts pressed while a text field had the keyboard, run on the next frame (see + /// `registry_shortcuts`). + deferred_commands: Vec<(&'static str, Option)>, /// The dialog seen at the last check, and a counter bumped whenever it changes (see /// [`Self::dialog_epoch`]). dialog_seen: Option, @@ -627,6 +630,7 @@ impl PdfCraftApp { last_autosave: 0.0, pending_recovered: None, allow_quit: false, + deferred_commands: Vec::new(), dialog_seen: None, dialog_epoch: 0, ctx: None, @@ -1424,6 +1428,8 @@ impl eframe::App for PdfCraftApp { let now = ctx.input(|i| i.time); self.autosave_tick(now); self.poll_updates(); + // Shortcuts deferred last frame: the text field has taken that frame's typing since. + let deferred = std::mem::take(&mut self.deferred_commands); self.shortcuts(ctx); // Scrolling is transient: never resume after changing tabs, opening a modal/palette, // or returning to a window that lost focus. @@ -1433,7 +1439,16 @@ impl eframe::App for PdfCraftApp { view.auto_scroll.cancel(); } } - self.process_pending_edits(); + // A field that refused its value this frame keeps the shortcut from saving or printing + // behind the user's back; so does a save prompt opened since. + if self.process_pending_edits() && self.close_request.is_none() { + for (id, doc) in deferred { + // Only in the document the shortcut was pressed in. + if self.active_ids().map(|(_, active)| active) == doc { + self.execute(id); + } + } + } self.poll_export(); self.poll_ocr(); self.poll_action(); @@ -1477,6 +1492,8 @@ impl eframe::App for PdfCraftApp { }, ); dialogs::show(self, &ctx); + // Notices too: a refused field value or a failed save must be seen in full screen. + widgets::toast(self, &ctx); return; } chrome::tab_strip(self, ui); diff --git a/crates/ui-egui/src/optimize_ui.rs b/crates/ui-egui/src/optimize_ui.rs index fbb25d0cab..a607cfb12f 100644 --- a/crates/ui-egui/src/optimize_ui.rs +++ b/crates/ui-egui/src/optimize_ui.rs @@ -190,6 +190,10 @@ pub(crate) fn audit_body(ui: &mut egui::Ui, rows: &[pdfcraft_engine::optimize::S impl PdfCraftApp { /// Optimize PDF with the dialog's choices and save the copy. pub fn optimize_with_draft(&mut self) { + // What's typed in a form field is part of the document (#166). + if !self.commit_form_typing() { + return; + } let Some((_, id)) = self.active_ids() else { return }; let d = self.optimize_draft.clone(); let result = self.session.optimized_bytes(id, &d.settings, &d.discard).map(|(b, r)| { diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index b207bace8a..bcc2d9e8c2 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -147,6 +147,10 @@ impl PdfCraftApp { /// a preset path returns `true` once the save picker is showing; the file is written on a /// later frame, when the user has chosen where. pub fn print_now(&mut self) -> bool { + // What's typed in a form field is part of what's printed (#166). + if !self.commit_form_typing() { + return false; + } let Some((_, id)) = self.active_ids() else { return false }; let Some(doc) = self.session.get(id) else { return false }; let labels: Vec = doc.info.pages.iter().map(|p| p.label.clone()).collect(); diff --git a/crates/ui-egui/src/sign_ui.rs b/crates/ui-egui/src/sign_ui.rs index bb3314e8c9..cce6eb76aa 100644 --- a/crates/ui-egui/src/sign_ui.rs +++ b/crates/ui-egui/src/sign_ui.rs @@ -268,6 +268,11 @@ impl PdfCraftApp { /// Sign as …: open the ID, sign, save the signed file (Save As), and show it. fn finish_signing(&mut self) -> Result<(), String> { + // What's typed in a form field is signed with the document (#166). A rejected value keeps + // the dialog open; the notice says why. + if !self.commit_form_typing() { + return Err(String::new()); + } let Some((_, doc_id)) = self.active_ids() else { return Err("no document".into()) }; let d = self.sign_draft.clone().ok_or("nothing to sign")?; let entry = d.selected.and_then(|i| self.digital_ids.get(i)).cloned().ok_or("Choose a digital ID.")?; @@ -296,6 +301,11 @@ impl PdfCraftApp { let name = self.session.get(doc_id).map(|d| d.name.clone()).unwrap_or_default(); let stem = name.trim_end_matches(".pdf").trim_end_matches(".PDF").to_string(); let sign_and_save = move |app: &mut Self, path: PathBuf| -> Result<(), String> { + // Sign what was typed while the save panel was open, too (#166); a refused value + // has said why already. + if !app.commit_typing_in(doc_id) { + return Err(String::new()); + } let signed = app.session.sign(doc_id, &id, opts).map_err(|e| e.to_string())?; crate::editing::write_atomically(&path.to_string_lossy(), signed.as_slice()).map_err(|e| format!("Could not save: {e}"))?; app.session.mark_signed(doc_id, signed, Some(path.to_string_lossy().into_owned())).map_err(|e| e.to_string())?; @@ -316,7 +326,9 @@ impl PdfCraftApp { // meanwhile. Once the picker shows, the dialog closes; an error from here on // arrives as a notice. If no picker could show, the dialog stays open. let asked = self.ask_one(crate::pickers::Ask::Save(dialog), Some(doc_id), move |app, path| { - if let Err(e) = sign_and_save(app, path) { + if let Err(e) = sign_and_save(app, path) + && !e.is_empty() + { app.notify(e); } }); diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index 8b004dd128..1fde4e1eb0 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -144,3 +144,256 @@ fn date_fields_offer_a_calendar() { let (ny, nm) = if m == 12 { (y + 1, 1) } else { (y, m + 1) }; assert_eq!(value(&h, "due"), vec![format!("{nm:02}/15/{ny}")], "picked in the field's format"); } + +/// The value of `name` in a saved PDF, read back by opening it in a fresh app. +fn saved_value(path: &std::path::Path, name: &str) -> Vec { + let mut app = PdfCraftApp::new(); + app.open_bytes("saved.pdf", None, std::fs::read(path).unwrap()).unwrap(); + app.session.get(app.views[0].id).unwrap().form.iter().find(|f| f.name == name).unwrap().value.clone() +} + +fn scratch(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("pdfcraft-forms-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn saving_while_still_typing_in_a_field_saves_the_text() { + // Issue #166: ⌘S with the field still focused saved it blank. + let dir = scratch("save-focused"); + let out = dir.join("saved.pdf"); + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Browser Form 123".into())); + h.run_steps(2); + assert!(h.state().views[0].forms.focus.is_some(), "still typing"); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::S); + h.run_steps(3); + assert_eq!(saved_value(&out, "name"), ["Browser Form 123"], "the saved PDF has the typed text"); + assert_eq!(value(&h, "name"), ["Browser Form 123"]); + let doc = h.state().session.get(h.state().views[0].id).unwrap(); + assert!(!doc.dirty, "and the document is saved, not left with an unsaved change"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn closing_while_still_typing_in_a_field_asks_to_save() { + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Not lost".into())); + h.run_steps(2); + h.state_mut().request_close_tab(0); + assert!(h.state().close_request.is_some(), "typed text is an unsaved change: the user is asked"); + assert_eq!(h.state().views.len(), 1); +} + +#[test] +fn close_all_saves_text_typed_in_a_tab_that_is_no_longer_active() { + let dir = scratch("close-all"); + let out = dir.join("first.pdf"); + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Typed in tab one".into())); + h.run_steps(2); + // Another document becomes active while the first one's field still has its draft. + h.state_mut().open_bytes("other.pdf", None, include_bytes!("data/form.pdf").to_vec()).unwrap(); + h.run_steps(2); + h.state_mut().close_all(); + assert!(h.state().close_request.is_some(), "the first document has an unsaved change"); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + let ctx = h.ctx.clone(); + h.state_mut().resolve_close(&ctx, Some(true)); + assert_eq!(saved_value(&out, "name"), ["Typed in tab one"]); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A key press as one event (kittest's `key_press` runs a frame per queued event). +fn key(key: egui::Key, modifiers: egui::Modifiers) -> egui::Event { + egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers } +} + +/// Deliver `events` together, in one frame. +fn one_frame(h: &mut Harness<'static, PdfCraftApp>, events: Vec) { + h.input_mut().events.extend(events); + h.run_steps(1); +} + +/// Type `text` into field `name`, then press ⌘S in the same frame as `last` (more input). +fn type_then_save_in_one_frame(h: &mut Harness<'static, PdfCraftApp>, name: &str, text: &str, last: egui::Event, out: &std::path::Path) { + click_field(h, name, 0); + h.event(egui::Event::Text(text.into())); + h.run_steps(2); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + one_frame(h, vec![last, key(egui::Key::S, egui::Modifiers::COMMAND)]); + h.run_steps(3); +} + +#[test] +fn typing_and_saving_in_the_same_frame_saves_the_last_keystrokes() { + let dir = scratch("same-frame"); + let out = dir.join("saved.pdf"); + let mut h = harness(); + type_then_save_in_one_frame(&mut h, "name", "Browser", egui::Event::Text(" Form 123".into()), &out); + assert_eq!(saved_value(&out, "name"), ["Browser Form 123"]); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn escape_and_save_in_the_same_frame_discards_the_draft() { + let dir = scratch("escape"); + let out = dir.join("saved.pdf"); + let mut h = harness(); + let before = value(&h, "name"); + type_then_save_in_one_frame(&mut h, "name", "Discard me", key(egui::Key::Escape, egui::Modifiers::NONE), &out); + assert_eq!(value(&h, "name"), before, "Escape cancels typing, even with ⌘S right after it"); + assert!(h.state().views[0].forms.focus.is_none()); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn an_unrelated_command_leaves_the_field_being_typed_in_alone() { + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Still typing".into())); + h.run_steps(2); + assert!(h.state_mut().execute("view.theme.dark")); + h.run_steps(2); + let focus = h.state().views[0].forms.focus.clone().expect("the editor stays open"); + assert_eq!(focus.text, "Still typing"); + assert!(!h.state().session.get(h.state().views[0].id).unwrap().dirty, "nothing was committed"); +} + +#[test] +fn exporting_form_data_while_typing_includes_the_text() { + let dir = scratch("export"); + let out = dir.join("data.xfdf"); + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Exported value".into())); + h.run_steps(2); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.state_mut().export_data_dialog(false, true); + assert!(std::fs::read_to_string(&out).unwrap().contains("Exported value")); + let _ = std::fs::remove_dir_all(&dir); +} + +/// One text field, `qty`, whose validation script refuses values over 100. +fn validated_form() -> Vec { + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (qty) /V (4) /Rect [20 350 280 372] /P 3 0 R /F 4 /AA << /V << /S /JavaScript /JS (if (event.value > 100) { app.alert('At most 100'); event.rc = false; }) >> >> >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +#[test] +fn a_rejected_value_keeps_the_typing_and_stops_the_save() { + let dir = scratch("rejected"); + let out = dir.join("saved.pdf"); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.open_bytes("order.pdf", None, validated_form()).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("zoom", "150").unwrap(); + app + }); + h.run_steps(6); + click_field(&mut h, "qty", 0); + h.event(egui::Event::Text("500".into())); + h.run_steps(2); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::S); + h.run_steps(4); + assert!(!out.exists(), "nothing is saved without the typed value"); + assert_eq!(value(&h, "qty"), ["4"], "the field keeps its valid value"); + let focus = h.state().views[0].forms.focus.clone().expect("the editor stays open"); + assert_eq!(focus.text, "4500", "with the text (typed after the 4), so the user can fix it"); + // And closing asks rather than dropping it. + h.state_mut().request_close_tab(0); + assert!(h.state().close_request.is_some()); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_save_shortcut_pressed_while_typing_saves_only_the_document_it_was_pressed_in() { + let dir = scratch("deferred-doc"); + let out = dir.join("saved.pdf"); + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Tab one".into())); + h.run_steps(2); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + one_frame(&mut h, vec![key(egui::Key::S, egui::Modifiers::COMMAND)]); + assert!(!out.exists(), "⌘S while typing waits a frame for the field"); + // Before the deferred ⌘S runs, another document becomes active. + h.state_mut().open_bytes("other.pdf", None, include_bytes!("data/form.pdf").to_vec()).unwrap(); + h.run_steps(3); + assert!(!out.exists(), "the other document is not saved by a shortcut pressed in the first"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_value_refused_on_enter_keeps_save_in_the_same_frame_from_running() { + let dir = scratch("enter-refused"); + let out = dir.join("saved.pdf"); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.open_bytes("order.pdf", None, validated_form()).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("zoom", "150").unwrap(); + app + }); + h.run_steps(6); + click_field(&mut h, "qty", 0); + h.event(egui::Event::Text("500".into())); + h.run_steps(2); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + one_frame(&mut h, vec![key(egui::Key::Enter, egui::Modifiers::NONE), key(egui::Key::S, egui::Modifiers::COMMAND)]); + h.run_steps(3); + assert_eq!(value(&h, "qty"), ["4"]); + assert!(!out.exists(), "nothing is saved right after the field refused its value"); + // The refused text is back in the field, so a later ⌘S stops again instead of saving 4. + assert_eq!(h.state().views[0].forms.focus.as_ref().map(|f| f.text.as_str()), Some("4500")); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::S); + h.run_steps(4); + assert!(!out.exists()); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn flattening_while_typing_flattens_the_typed_text() { + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Flat Ada".into())); + h.run_steps(2); + assert!(h.state_mut().execute("form.flatten")); + h.run_steps(2); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert!(doc.form.iter().all(|f| f.name != "name"), "the field is flattened"); + let mut r = pdfcraft_render::PageRenderer::new(doc.bytes.clone(), Default::default()); + let text = r + .render(pdfcraft_render::RenderRequest { page: 0, kind: pdfcraft_render::RequestKind::Text, scale: 1.0, ..Default::default() }) + .text + .map(|t| t.plain_text()) + .unwrap_or_default(); + assert!(text.contains("Flat Ada"), "into the page: {text:?}"); +}