From c73eb76e920ac052bb51310aeb6145e8ef6395d9 Mon Sep 17 00:00:00 2001 From: Earl Co Date: Wed, 14 Oct 2026 23:03:26 +0200 Subject: [PATCH 01/34] fix(forms): draw the widget border inside /Tx BMC so Acrobat keeps it on focus changes (#1613) --- crates/forms/src/appearance.rs | 28 ++++++++++++------------- crates/forms/src/tests.rs | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index dc5ffcef19..a9558bbe0c 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -426,20 +426,20 @@ pub fn field_appearance_drawn(doc: &Document, f: &Field, w: &Widget, values: &[S } } } - let mut content = c.into_bytes(); - content.extend( - format!( - "/Tx BMC\nq\n{} {} {} {} re W n\nBT\n/{} {} Tf\n{}\n", - n(bw), - n(bw), - n(width - 2.0 * bw), - n(height - 2.0 * bw), - font_name, - n(size), - da.color - ) - .bytes(), - ); + // Acrobat regenerates only what is inside /Tx BMC when it redraws a field (e.g. on focus + // changes), discarding what came before it (issues #1613, #633). The background and border + // therefore go inside the marked-content block too, so a redraw keeps them with the text. + let mut content = format!( + "/Tx BMC\nq\n{c}{} {} {} {} re W n\nBT\n/{} {} Tf\n{}\n", + n(bw), + n(bw), + n(width - 2.0 * bw), + n(height - 2.0 * bw), + font_name, + n(size), + da.color + ) + .into_bytes(); content.extend(body); content.extend_from_slice(b"ET\nQ\nEMC\n"); let mut fonts = Dict::new(); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 0256bfafb9..23691aa17f 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -277,6 +277,44 @@ fn text_fields_get_new_appearances() { assert!(raw.windows(8).any(|x| x == b"(Z\xfcrich)")); } +/// Acrobat regenerates only the `/Tx BMC` block when it redraws a text field, so background and +/// border must live inside it rather than immediately before it (#1613). +fn marked_content_borders_field(appearance: &str, label: &str) { + let start = appearance.find("/Tx BMC").expect("no /Tx BMC"); + let marked = appearance[start..].split_once("EMC").expect("no matching EMC").0; + assert!(marked.contains("1 1 0.9 rg"), "background inside /Tx BMC: {appearance} {label}"); + assert!(marked.contains("0 0 0 RG\n1 w\n0.5 0.5 199 19 re S"), "border inside /Tx BMC: {appearance} {label}"); +} + +#[test] +fn text_and_date_fields_keep_background_and_border_inside_marked_content() { + let mut doc = one_page(); + add_field(&mut doc, 0, [50.0, 700.0, 250.0, 720.0], &NewField::Text { multiline: false }, Some("name")).unwrap(); + add_field(&mut doc, 0, [50.0, 660.0, 250.0, 680.0], &NewField::Date, Some("due")).unwrap(); + let props = || FieldProps { + look: Some(Look { + border: Some([0.0; 3]), + fill: Some([1.0, 1.0, 0.9]), + width: 1.0, + style: BorderStyle::Solid, + text: [0.0, 0.0, 0.0], + font: FieldFont::Helvetica, + }), + ..FieldProps::default() + }; + set_props(&mut doc, "name", &props()).unwrap(); + set_props(&mut doc, "due", &props()).unwrap(); + set_value(&mut doc, "name", &FieldValue::Text("Ada".into())).unwrap(); + set_value(&mut doc, "due", &FieldValue::Text("10/14/2026".into())).unwrap(); + let name_ap = ap(&doc, &field(&fields(&doc), "name").widgets[0]); + let due_ap = ap(&doc, &field(&fields(&doc), "due").widgets[0]); + marked_content_borders_field(&name_ap, "text field"); + marked_content_borders_field(&due_ap, "date field"); + assert!(name_ap.contains("(Ada) Tj"), "{name_ap}"); + assert!(due_ap.contains("(10/14/2026) Tj"), "{due_ap}"); + assert!(!need_appearances(&doc), "drawable ASCII fields do not ask other viewers to redraw"); +} + #[test] fn existing_field_font_encoding_keeps_italian_punctuation_visible() { // ReportLab-style Helvetica: an indirect encoding dictionary with PDFDoc-like Differences. From 2f208a289b28059660fe88c5404cb0f24ecbda82 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 14 Oct 2026 21:40:11 +0000 Subject: [PATCH 02/34] Keep Auto font size in the field properties Appearance tab Fixes #2250 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/prepare.rs | 5 ++++- crates/ui-egui/tests/prepare.rs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/prepare.rs b/crates/ui-egui/src/prepare.rs index 37747f14c7..d4e20f254c 100644 --- a/crates/ui-egui/src/prepare.rs +++ b/crates/ui-egui/src/prepare.rs @@ -977,7 +977,10 @@ pub(crate) fn body(ui: &mut egui::Ui, d: &mut FieldDraft, t: &crate::theme::Toke if ui.add(widgets::checkbox(&mut auto, tl!("Auto"))).changed() { d.font_size = if auto { 0.0 } else { 12.0 }; } - ui.add_enabled(!auto, egui::DragValue::new(&mut d.font_size).range(2.0..=100.0).speed(0.25).suffix(" pt")); + ui.add_enabled( + !auto, + egui::DragValue::new(&mut d.font_size).range(2.0..=100.0).clamp_existing_to_range(false).speed(0.25).suffix(" pt"), + ); }); ui.end_row(); if let Some(l) = d.look.as_mut() { diff --git a/crates/ui-egui/tests/prepare.rs b/crates/ui-egui/tests/prepare.rs index b23f1a9f8f..ea006911d8 100644 --- a/crates/ui-egui/tests/prepare.rs +++ b/crates/ui-egui/tests/prepare.rs @@ -193,6 +193,20 @@ fn appearance_tab_restyles_the_field() { assert_eq!((l.style, l.font, l.fill), (BorderStyle::Dashed, FieldFont::Courier, Some([1.0, 1.0, 0.8]))); } +#[test] +fn appearance_tab_keeps_auto_font_size() { + let mut h = harness(); + h.state_mut().execute("form.prepare"); + h.run_steps(2); + h.state_mut().open_field_props("city", 0); + h.run_steps(2); + h.get_by_label("Appearance").click(); + h.run_steps(2); + h.state_mut().field_props.as_mut().unwrap().font_size = 0.0; + h.run_steps(3); + assert_eq!(h.state().field_props.as_ref().unwrap().font_size, 0.0, "Auto (0) must survive a frame"); +} + #[test] fn position_tab_rotates_a_field() { let mut h = harness(); From 1c3cb9d14d7b1a7a7b8c3ca1ef3b3fa856ef0cc3 Mon Sep 17 00:00:00 2001 From: fanyzp Date: Fri, 16 Oct 2026 01:15:06 +0800 Subject: [PATCH 03/34] feat(forms): nudge the selected field with the arrow keys in Prepare a form (#2608) --- ROADMAP.md | 1 + crates/ui-egui/src/lib.rs | 7 +++++ crates/ui-egui/src/prepare.rs | 48 ++++++++++++++++++++++++++++++++- crates/ui-egui/tests/prepare.rs | 40 +++++++++++++++++++++++++++ docs/ui-parity.md | 5 ++-- parity/acrobat-features.toml | 4 +-- 6 files changed, 100 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index fe9723fdf6..88bb4bb29e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -156,6 +156,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md | 8 | 1.0 polish: performance, right-to-left, missing languages, keyboard-only (M14) | 55–95 | ## Progress log +- **2026-10-15 (community, M6, #2608):** In Prepare a form the selected field moves with the arrow keys: a point per press, ten with Shift, as the object nudge and dragging it do. Each press is one undo step ("Change field properties"), and the keys no longer turn the page while a field is selected. Estimates unchanged. - **2026-10-13 (community, M3, #1569 review):** Pixel-only Wayland wheel, wheel-tilt and continuous input use standalone moved events, so Ctrl zoom does not remain latched after the modifier is released (#991); vendored winit patch (13), with a policy test. Estimates unchanged. - **2026-10-12 (M6):** XFDF field import and data-file extraction join all text nodes in a value, preserving text after XML comments and processing instructions. Synthetic regressions cover Unicode, CDATA, empty values and PDF save/reopen. Estimates unchanged. - **2026-10-12 (M7, picture right-click menu):** The Edit text & images menu of a picture gains Cut Image, Redact Image (a redaction mark over the picture, applied from Redact a PDF), Create Link… and Recognize Text… (for that page), in all 21 catalogs. The menu was drawn only while the pointer was over the picture, so its items outside the picture (most of a logo's menu, and Delete below a page) could not be clicked; an open menu now stays drawn. Still missing from Acrobat's menu: Arrange (bring to front, send backward), Remove crop, Draw freehand and Edit image using… (opening an external editor). diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d101d812e0..d753a0f733 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2813,6 +2813,13 @@ impl PdfCraftApp { { object_ui::nudge_keys(ctx, &mut self.views[i], doc); } + // A selected field moves with the arrows too (Prepare a form, #2608), before the view + // turns the page with them. + if self.is_preparing() + && let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_modification()) + { + prepare::nudge_keys(ctx, &mut self.views[i], doc); + } canvas::shortcuts(&mut self.views[i], ctx, self.scroll_speed); self.copy_key(i, ctx); self.image_pick_keys(i, ctx); diff --git a/crates/ui-egui/src/prepare.rs b/crates/ui-egui/src/prepare.rs index 386c42e872..28797de559 100644 --- a/crates/ui-egui/src/prepare.rs +++ b/crates/ui-egui/src/prepare.rs @@ -7,7 +7,7 @@ use egui::{Color32, CornerRadius, Pos2, Rect, Stroke, vec2}; use pdfcraft_engine::form_scripts::{CalcOp, Calculate, DATE_PRESETS, Format, TIME_PRESETS, Validate, format_value}; -use pdfcraft_engine::{BorderStyle, Edit, FieldFont, FieldLook, FieldProps, FormField, FormFieldKind, NewField}; +use pdfcraft_engine::{BorderStyle, Document, Edit, FieldFont, FieldLook, FieldProps, FormField, FormFieldKind, NewField}; use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; @@ -565,6 +565,52 @@ pub(crate) fn delete_selected(view: &mut DocView) -> Option { Some(Edit::Batch { label, edits: names.into_iter().map(|name| Edit::DeleteField { name }).collect() }) } +/// The arrow keys move the selected field a point, Shift and an arrow ten points, as in Acrobat, +/// as the object nudge does (`object_ui::nudge_keys`) and as dragging it does (#2608). The keys +/// are consumed before the view turns the page with them; one press is one undo step. +pub(crate) fn nudge_keys(ctx: &egui::Context, view: &mut DocView, doc: &Document) { + if ctx.egui_wants_keyboard_input() || view.prepare.grab.is_some() { + return; + } + let Some((name, wi)) = view.prepare.selected.clone() else { return }; + let Some(w) = doc.form.iter().find(|f| f.name == name).and_then(|f| f.widgets.get(wi)) else { return }; + let Some(page) = w.page else { return }; + let Some(xf) = view.page_xform(page) else { return }; + use egui::{Key, Modifiers}; + let mut step = egui::Vec2::ZERO; + ctx.input_mut(|i| { + for (key, direction) in [ + (Key::ArrowLeft, egui::vec2(-1.0, 0.0)), + (Key::ArrowRight, egui::vec2(1.0, 0.0)), + (Key::ArrowUp, egui::vec2(0.0, -1.0)), + (Key::ArrowDown, egui::vec2(0.0, 1.0)), + ] { + // Shift first: a plain key pattern also matches a Shift press. + if i.consume_key(Modifiers::SHIFT, key) { + step += direction * 10.0; + } else if i.consume_key(Modifiers::NONE, key) { + step += direction; + } + } + }); + if step == egui::Vec2::ZERO { + return; + } + // A point in view space whatever the zoom and whatever way the view is turned; the page's own + // rotation turns it into the user-space move the widget's rect is stored in. + let c = xf.rect.center(); + let unit = |d: egui::Vec2| { + let (a, b) = (xf.screen_to_view(c), xf.screen_to_view(c + d)); + let v = egui::vec2(b.0 - a.0, b.1 - a.1); + if v.length() > 0.0 { v / v.length() } else { egui::Vec2::ZERO } + }; + let delta = unit(egui::vec2(1.0, 0.0)) * step.x + unit(egui::vec2(0.0, 1.0)) * step.y; + let Ok(offset) = doc.object_move_offset(page, [f64::from(delta.x), f64::from(delta.y)]) else { return }; + let r = w.rect; + let rect = [r[0] + offset[0], r[1] + offset[1], r[2] + offset[0], r[3] + offset[1]]; + view.pending_edit = Some(Edit::SetFieldProps { name, props: Box::new(FieldProps { rect: Some((wi, rect)), ..Default::default() }) }); +} + /// Delete removes the selected fields; Escape clears the selection. pub(crate) fn select_all(view: &mut DocView, form: &[FormField]) { let mut selected = diff --git a/crates/ui-egui/tests/prepare.rs b/crates/ui-egui/tests/prepare.rs index eb25e42a03..cce42e337d 100644 --- a/crates/ui-egui/tests/prepare.rs +++ b/crates/ui-egui/tests/prepare.rs @@ -116,6 +116,46 @@ fn placing_moving_editing_and_deleting_a_field() { assert_eq!((r[2] - r[0], r[3] - r[1]), (14.0, 14.0)); } +/// The arrow keys nudge the selected field a point, Shift and an arrow ten points (#2608), as they +/// do for a selected object or image and as dragging it does. One press is one undo step, the page +/// does not turn with them, and the field stays selected for the next press. +#[test] +fn arrow_keys_nudge_the_selected_field_instead_of_turning_the_page() { + let mut h = harness(); + h.state_mut().execute("form.add.text"); + h.run_steps(1); + // A click places a default-size text field and selects it. + let p = at(&h, 40.0, 250.0); + h.hover_at(p); + h.run_steps(1); + h.drag_at(p); + h.run_steps(1); + h.drop_at(p); + h.run_steps(4); + let id = h.state().views[0].id; + let name = h.state().views[0].prepare.selected.clone().expect("the placed field is selected").0; + let near = |a: f64, b: f64| (a - b).abs() < 0.01; + let before = rect_of(&h, &name); + h.key_press(egui::Key::ArrowRight); + h.run_steps(2); + let right = rect_of(&h, &name); + assert!(near(right[0], before[0] + 1.0) && near(right[2], before[2] + 1.0), "{before:?} → {right:?}"); + assert!(near(right[2] - right[0], before[2] - before[0]), "moved, not resized"); + assert_eq!(h.state().session.get(id).unwrap().can_undo(), Some("Change field properties")); + h.key_press_modifiers(egui::Modifiers::SHIFT, egui::Key::ArrowUp); + h.run_steps(2); + let up = rect_of(&h, &name); + assert!(near(up[1], right[1] + 10.0) && near(up[3], right[3] + 10.0), "{right:?} → {up:?}"); + h.key_press(egui::Key::ArrowDown); + h.run_steps(2); + h.key_press(egui::Key::ArrowLeft); + h.run_steps(2); + let back = rect_of(&h, &name); + assert!(near(back[0], up[0] - 1.0) && near(back[1], up[1] - 1.0), "{up:?} → {back:?}"); + assert!(h.state().views[0].prepare.selected.is_some(), "it stays selected for the next nudge"); + assert_eq!(h.state().views[0].current, 0, "the arrows moved the field, not the page"); +} + #[test] fn format_validate_and_calculate_tabs() { use pdfcraft_engine::form_scripts::{Calculate, Format, Validate}; diff --git a/docs/ui-parity.md b/docs/ui-parity.md index a78409fade..93cc83086b 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI and interaction parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (column select #740, multi-object move #844, Shift-⌘-G #810 fixed) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-15 · **Change:** minor (the selected field nudges with the arrow keys in Prepare a form, #2608) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) How PdfCraft's tools, panels, shortcuts and feel compare with Acrobat Pro's current interface (the 2023+ "new Acrobat": All tools pane on the left, quick-action toolbar, Home view). Behaviour is @@ -52,7 +52,7 @@ No dump of Acrobat's menu tree has been diffed against the registry yet (gap 24) | Rulers, grids, snap to grid, guides | yes | no | planned | | Cursor coordinates, line weights | yes | no | planned | | Measuring snap to paths, endpoints, midpoints, intersections | yes | yes | shipped | -| Nudge selected objects with arrow keys (Shift for larger steps) | yes | yes in Edit text: a point, Shift ten points, for selected objects, images and an open paragraph's box (#1520) | shipped | +| Nudge selected objects with arrow keys (Shift for larger steps) | yes | yes in Edit text: a point, Shift ten points, for selected objects, images and an open paragraph's box (#1520); and in Prepare a form, for the selected field (#2608) | shipped | | Single-key tool accelerators (H, V, Z, …) | yes, opt-in | no | planned | | Context menus on pages, thumbnails, comments, fields | yes | thumbnails, comments, fields | partial | | Comment pop-ups as movable windows, connector lines | yes | inline in the panel | planned | @@ -81,5 +81,6 @@ No dump of Acrobat's menu tree has been diffed against the registry yet (gap 24) | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | A selected field nudges with the arrow keys in Prepare a form: a point, Shift ten points, one undo step per press (#2608) | | 2026-10-11 | minor | Column select shipped (#740, PR #765); several objects move together on one page (#844, PR #907); Shift-⌘-G goes back (#810, PR #835) | | 2026-10-10 | major | Created from the command registry, automation tools, the View area of the checklist and open UI issues | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 533013b547..3e5eb9c0bd 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4740,8 +4740,8 @@ milestone = "M6" status = "shipped" commands = ["form.prepare"] tools = ["form_add_field", "form_set_props", "form_delete_field"] -evidence = ["crates/ui-egui/tests/prepare.rs::placing_moving_editing_and_deleting_a_field", "crates/ui-egui/src/prepare.rs::corner_drags_stretch_freely_and_keep_proportions_with_shift", "crates/ui-egui/tests/prepare.rs::deleting_several_selected_fields_and_detecting_fields_from_the_panel", "crates/engine/src/tests.rs::preparing_a_form_adds_renames_and_deletes_fields", "crates/ui-egui/tests/ui_group/bulk_fields.rs::shared_properties_preserve_mixed_values_and_undo_the_whole_change", "crates/ui-egui/tests/ui_group/bulk_fields.rs::select_all_in_prepare_a_form_selects_only_the_current_pages_fields", "crates/ui-egui/tests/ui_group/bulk_fields.rs::dragging_an_empty_page_area_selects_fields_without_moving_them"] -notes = "fields shown with blue fill and name tags; select, move, resize by corners (Shift keeps the field's proportions, #2199), Delete (every selected field, one undo step); field tools return to Select after one field. Shared General, Appearance and applicable Options edits are atomic and opt-in; Ctrl/Command+A selects the current page, rectangle and panel selection are supported. Names, positions, choices/defaults and script tabs remain individual GUI edits" +evidence = ["crates/ui-egui/tests/prepare.rs::placing_moving_editing_and_deleting_a_field", "crates/ui-egui/tests/prepare.rs::arrow_keys_nudge_the_selected_field_instead_of_turning_the_page", "crates/ui-egui/src/prepare.rs::corner_drags_stretch_freely_and_keep_proportions_with_shift", "crates/ui-egui/tests/prepare.rs::deleting_several_selected_fields_and_detecting_fields_from_the_panel", "crates/engine/src/tests.rs::preparing_a_form_adds_renames_and_deletes_fields", "crates/ui-egui/tests/ui_group/bulk_fields.rs::shared_properties_preserve_mixed_values_and_undo_the_whole_change", "crates/ui-egui/tests/ui_group/bulk_fields.rs::select_all_in_prepare_a_form_selects_only_the_current_pages_fields", "crates/ui-egui/tests/ui_group/bulk_fields.rs::dragging_an_empty_page_area_selects_fields_without_moving_them"] +notes = "fields shown with blue fill and name tags; select, move (drag, or the arrow keys: a point, Shift ten points, #2608), resize by corners (Shift keeps the field's proportions, #2199), Delete (every selected field, one undo step); field tools return to Select after one field. Shared General, Appearance and applicable Options edits are atomic and opt-in; Ctrl/Command+A selects the current page, rectangle and panel selection are supported. Names, positions, choices/defaults and script tabs remain individual GUI edits" [[feature]] id = "form.add-text-field" From bf2eb470ed61d1c0b69eb7c1a971138d3fb9d144 Mon Sep 17 00:00:00 2001 From: boneyland <48160772+boneyland@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:59:09 +0800 Subject: [PATCH 04/34] M6: detected form fields get no border or fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare a form ▸ Detect form fields places each field on a line or box the page already draws, but every new widget got a grey /MK /BC and an opaque white /MK /BG, so the fill hid the printed form underneath. Detected fields now clear both in the same undoable step; fields drawn by hand keep the defaults. Refs #294, #2831. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/js.rs | 17 ++++++++++++----- crates/engine/src/tests.rs | 22 ++++++++++++++++++++++ parity/acrobat-features.toml | 4 ++-- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index 8958688d2d..908b969fef 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -343,8 +343,17 @@ impl Session { let names = found.iter().map(|(_, c, _)| c.name.clone()).collect(); let mut edits = Vec::with_capacity(found.len()); for (page, c, turn) in found { - // A turned field keeps its place: the rectangle is already the one on the page. - let props = (turn != 0).then(|| pdfcraft_forms::FieldProps { rotation: Some((0, turn)), rect: Some((0, c.rect)), ..Default::default() }); + // The page already draws the line or box the field sits on, so the field gets no + // border or fill of its own: an opaque fill would hide the printed form (#294, #2831). + let mut props = pdfcraft_forms::FieldProps { + appearance: Some(pdfcraft_forms::LookPatch { border: Some(None), fill: Some(None), ..Default::default() }), + ..Default::default() + }; + if turn != 0 { + // A turned field keeps its place: the rectangle is already the one on the page. + props.rotation = Some((0, turn)); + props.rect = Some((0, c.rect)); + } let tall = if turn % 180 == 0 { c.rect[3] - c.rect[1] } else { c.rect[2] - c.rect[0] }; edits.push(Edit::AddField { page, @@ -355,9 +364,7 @@ impl Session { }, name: Some(c.name.clone()), }); - if let Some(props) = props { - edits.push(Edit::SetFieldProps { name: c.name, props: Box::new(props) }); - } + edits.push(Edit::SetFieldProps { name: c.name, props: Box::new(props) }); } self.apply(id, Edit::Batch { label: "Detect form fields".into(), edits })?; Ok(names) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ec305b1ca5..d9bac106b9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3489,6 +3489,28 @@ fn detecting_fields_on_a_printed_form() { assert!(s.detect_fields(id, &[]).is_empty(), "nothing left to detect"); } +/// #294, #2831: a detected field sits on a line or box the page already draws, so it gets no border +/// and no fill that would cover the printed form. A field drawn by hand keeps the defaults. +#[test] +fn detected_fields_have_no_border_or_fill() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("paper.pdf", None, Arc::new(paper_form()), None).unwrap(); + let added = s.auto_detect_fields(id, &[]).unwrap(); + let d = s.get(id).unwrap(); + let kinds: Vec = d.form.iter().map(|f| f.kind).collect(); + assert!(kinds.contains(&pdfcraft_forms::FieldKind::Text) && kinds.contains(&pdfcraft_forms::FieldKind::CheckBox), "{kinds:?}"); + for name in &added { + let look = d.field_look(name).unwrap(); + assert_eq!((look.border, look.fill), (None, None), "{name}"); + } + assert_eq!(d.can_undo(), Some("Detect form fields"), "still one step"); + + s.apply(id, Edit::AddField { page: 0, rect: [200.0, 10.0, 300.0, 30.0], kind: NewField::Text { multiline: false }, name: Some("drawn".into()) }) + .unwrap(); + let look = s.get(id).unwrap().field_look("drawn").unwrap(); + assert!(look.border.is_some() && look.fill.is_some(), "{look:?}"); +} + /// A printed form, 420 × 300 pt: a title, a label with a rule to write on, a box with a small /// label in its corner, an empty box with its label above, and two check boxes. fn paper_form() -> Vec { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c3d319f94f..c3ef94e68d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5088,8 +5088,8 @@ milestone = "M6" status = "partial" commands = ["form.detect", "form.prepare"] tools = ["form_detect_fields"] -evidence = ["crates/forms/src/tests.rs::detection_finds_blanks_rules_boxes_and_names_them", "crates/engine/src/tests.rs::detecting_fields_on_a_printed_form", "crates/engine/src/tests.rs::detecting_fields_on_a_scanned_form", "crates/forms/src/detect/raster.rs::a_scanned_form_gives_its_rules_boxes_and_check_boxes", "crates/forms/src/detect/raster.rs::noise_and_uneven_paper_do_not_hide_the_lines", "crates/automation/tests/automation.rs::detecting_form_fields_through_tools", "crates/ui-egui/tests/javascript.rs::prepare_a_form_detects_fields_on_a_paper_form", "crates/ui-egui/tests/prepare.rs::deleting_several_selected_fields_and_detecting_fields_from_the_panel"] -notes = "Underscore runs, rules, empty boxes and small squares (check boxes); runs when Prepare a form starts on a document without fields, and from Prepare a form ▸ Detect form fields at any time (existing fields are skipped). Scanned pages (an image covers at least half the page and the content draws no lines) are rendered and their rules, boxes and check boxes read from the pixels; a box with a small printed label gets a field in its empty part. Tested on synthetic scans only: straight, stored sideways with /Rotate 90 (fields get /MK /R so typing reads upright), noisy, and 1 degree askew (fields stay inside their boxes but lose up to the skew in height). Labels come from the text layer or, when the OCR models are installed, from text recognition (Latin without accents, so German labels with umlauts make poorer names); without either, fields get default names (Text1, Check Box1). Recognition runs synchronously, about a second per scanned page. Not yet: comb boxes, dotted leader lines, tables of cells as such, deskewing, radio groups." +evidence = ["crates/forms/src/tests.rs::detection_finds_blanks_rules_boxes_and_names_them", "crates/engine/src/tests.rs::detecting_fields_on_a_printed_form", "crates/engine/src/tests.rs::detected_fields_have_no_border_or_fill", "crates/engine/src/tests.rs::detecting_fields_on_a_scanned_form", "crates/forms/src/detect/raster.rs::a_scanned_form_gives_its_rules_boxes_and_check_boxes", "crates/forms/src/detect/raster.rs::noise_and_uneven_paper_do_not_hide_the_lines", "crates/automation/tests/automation.rs::detecting_form_fields_through_tools", "crates/ui-egui/tests/javascript.rs::prepare_a_form_detects_fields_on_a_paper_form", "crates/ui-egui/tests/prepare.rs::deleting_several_selected_fields_and_detecting_fields_from_the_panel"] +notes = "Underscore runs, rules, empty boxes and small squares (check boxes); runs when Prepare a form starts on a document without fields, and from Prepare a form ▸ Detect form fields at any time (existing fields are skipped). Scanned pages (an image covers at least half the page and the content draws no lines) are rendered and their rules, boxes and check boxes read from the pixels; a box with a small printed label gets a field in its empty part. Detected fields get no border and no fill, since the page already draws the line or box they sit on; fields drawn by hand keep the grey border and white fill. Tested on synthetic scans only: straight, stored sideways with /Rotate 90 (fields get /MK /R so typing reads upright), noisy, and 1 degree askew (fields stay inside their boxes but lose up to the skew in height). Labels come from the text layer or, when the OCR models are installed, from text recognition (Latin without accents, so German labels with umlauts make poorer names); without either, fields get default names (Text1, Check Box1). Recognition runs synchronously, about a second per scanned page. Not yet: comb boxes, dotted leader lines, tables of cells as such, deskewing, radio groups." [[feature]] id = "form.auto-name-fields" From fbd9911d765f62672ef383423addf72db8c271b4 Mon Sep 17 00:00:00 2001 From: Guillem Hermida Date: Fri, 16 Oct 2026 17:48:47 +0200 Subject: [PATCH 05/34] fix(forms): renaming a nested field checks the full name it would take MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming a field changes only its own /T, so MASTER.a renamed to b becomes MASTER.b. set_props compared the bare new partial name with the other fields' fully qualified names: renaming MASTER.a to b succeeded while MASTER.b existed, leaving two fields with one fully qualified name, which are one field with a shared value (ISO 32000-2 §12.7.4.2); and renaming MASTER.a to c was refused because of an unrelated top-level field c. The check now compares the new full name, leaving the field itself out so a kid keeping its own partial name still succeeds, and the error names that full name. Top-level renames are checked as before. --- crates/forms/src/author.rs | 10 +++-- crates/forms/src/tests.rs | 74 ++++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 2 +- 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index bb45f38d9f..c134f5cdcc 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -900,12 +900,14 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result Document { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), // 1 + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), // 2 + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R 8 0 R 9 0 R 11 0 R 12 0 R] >>".into(), // 3 + "<< /Fields [5 0 R 8 0 R 9 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), // 4 + "<< /T (MASTER) /Kids [6 0 R 7 0 R 10 0 R] >>".into(), // 5 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (a) /Parent 5 0 R /V (one) /Rect [50 700 250 720] /P 3 0 R >>".into(), // 6 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (b) /Parent 5 0 R /V (two) /Rect [50 650 250 670] /P 3 0 R >>".into(), // 7 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (c) /V (three) /Rect [50 600 250 620] /P 3 0 R >>".into(), // 8 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (d) /Rect [50 550 250 570] /P 3 0 R >>".into(), // 9 + "<< /FT /Tx /T (e) /Parent 5 0 R /V (four) /Kids [11 0 R 12 0 R] >>".into(), // 10 + "<< /Type /Annot /Subtype /Widget /Parent 10 0 R /Rect [50 500 250 520] /P 3 0 R >>".into(), // 11 + "<< /Type /Annot /Subtype /Widget /Parent 10 0 R /Rect [300 500 500 520] /P 3 0 R >>".into(), // 12 + ]; + document(&objs) +} + +fn rename(doc: &mut Document, name: &str, to: &str) -> Result { + set_props(doc, name, &FieldProps { name: Some(to.into()), ..FieldProps::default() }) +} + +/// The partial name (`/T`) of object `num`. +fn own_name(doc: &Document, num: u32) -> Option { + doc.get(ObjRef::new(num, 0)).as_dict().unwrap().get(b"T").and_then(|t| text_of(&doc.resolve(t))) +} + +#[test] +fn renaming_a_kid_is_checked_against_the_full_name_it_takes() { + let mut doc = dotted_form(); + // MASTER.b is taken, though no field's full name is the bare "b". Two fields of one fully + // qualified name are one field and share its value (ISO 32000-2 §12.7.4.2). + let err = rename(&mut doc, "MASTER.a", "b").unwrap_err(); + assert_eq!(err, FormError::Invalid("a field named \"MASTER.b\" already exists".into())); + assert_eq!(own_name(&doc, 6).as_deref(), Some("a"), "a refused rename changes nothing"); + // The same between a field merged with its widget and a field with widget kids. + assert!(rename(&mut doc, "MASTER.e", "a").is_err()); + assert!(rename(&mut doc, "MASTER.a", "e").is_err()); + // A kid keeping its own name is no clash with itself. + assert_eq!(rename(&mut doc, "MASTER.b", "b").unwrap(), "MASTER.b"); + // The top-level "c" does not take MASTER.c. + assert_eq!(rename(&mut doc, "MASTER.a", "c").unwrap(), "MASTER.c"); + assert_eq!(rename(&mut doc, "MASTER.e", "f").unwrap(), "MASTER.f"); + let doc = reopen(&doc); + let all = fields(&doc); + let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect(); + assert_eq!(names, ["MASTER.c", "MASTER.b", "MASTER.f", "c", "d"]); + for (name, value) in [("MASTER.c", "one"), ("MASTER.b", "two"), ("MASTER.f", "four"), ("c", "three")] { + assert_eq!(field(&all, name).value, [value], "{name} keeps its own value"); + } + // Only the field's own /T changes: the kids stay under MASTER, a merged kid stays its own + // widget, and the widget kids get no /T (which would make them fields). + let widgets = |name: &str| field(&all, name).widgets.iter().map(|w| w.obj.num).collect::>(); + assert_eq!((field(&all, "MASTER.c").obj, widgets("MASTER.c")), (ObjRef::new(6, 0), vec![6])); + assert_eq!((field(&all, "MASTER.f").obj, widgets("MASTER.f")), (ObjRef::new(10, 0), vec![11, 12])); + let partial: Vec> = [5, 6, 10, 11, 12].into_iter().map(|n| own_name(&doc, n)).collect(); + assert_eq!(partial.iter().map(Option::as_deref).collect::>(), [Some("MASTER"), Some("c"), Some("f"), None, None]); +} + +#[test] +fn renaming_a_top_level_field_is_still_checked_against_top_level_names() { + let mut doc = dotted_form(); + let err = rename(&mut doc, "c", "d").unwrap_err(); + assert_eq!(err, FormError::Invalid("a field named \"d\" already exists".into())); + assert_eq!(own_name(&doc, 8).as_deref(), Some("c"), "a refused rename changes nothing"); + // A kid's partial name is no top-level name: MASTER.a leaves "a" free. + assert_eq!(rename(&mut doc, "c", "a").unwrap(), "a"); + let doc = reopen(&doc); + let names: Vec = fields(&doc).into_iter().map(|f| f.name).collect(); + assert_eq!(names, ["MASTER.a", "MASTER.b", "MASTER.e", "a", "d"]); +} + #[test] fn deleting_works_with_a_form_dictionary_inside_the_catalog() { let mut doc = fixture(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c3d319f94f..24e1793fe6 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5040,7 +5040,7 @@ milestone = "M6" status = "shipped" commands = ["form.prepare"] tools = ["form_set_props"] -evidence = ["crates/forms/src/tests.rs::locked_fields_only_take_unlocking", "crates/ui-egui/tests/prepare.rs::preview_fills_the_form_and_locked_fields_keep_their_properties"] +evidence = ["crates/forms/src/tests.rs::locked_fields_only_take_unlocking", "crates/ui-egui/tests/prepare.rs::preview_fills_the_form_and_locked_fields_keep_their_properties", "crates/forms/src/tests.rs::renaming_a_kid_is_checked_against_the_full_name_it_takes", "crates/forms/src/tests.rs::renaming_a_top_level_field_is_still_checked_against_top_level_names"] notes = "rename in Field Properties ▸ General; Locked sets the widgets' annotation Locked flag (/F bit 8) and greys out the properties until unlocked; values can still be filled in" [[feature]] From b2150f3ffd09a0dc6c2d51ba1a4abd39746bac3d Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 19:37:00 +0200 Subject: [PATCH 06/34] Forms: measure a field's own simple font, not Helvetica, when laying out its text (fixes #2894) --- crates/forms/src/appearance.rs | 12 +++++++--- crates/forms/src/tests.rs | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 4ec22f0114..4093742138 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -13,8 +13,9 @@ //! Acrobat keeps a value its field font can't show: the value itself is stored as typed, //! [`field_appearance_drawn`] returns the first such character, and the caller marks the widget //! (`pdfcraft_fonts::UNDRAWN_MARK`) and asks other viewers to redraw the field -//! (`/NeedAppearances`). Widths use the approximate metrics of `pdfcraft-fonts` (Helvetica, or one -//! em per full-width character in composite fonts). +//! (`/NeedAppearances`). Widths are the field font's own (its `/Widths`, or the standard-14 +//! metrics), Helvetica's when Helvetica stands in, and one em per full-width character in +//! composite fonts. //! //! `/MK /R` (0, 90, 180, 270, counterclockwise) lays the content out in the upright box and sets //! the appearance `/Matrix` so that box lands on the widget rectangle. Any other value is 0, and @@ -191,7 +192,12 @@ impl FontText { fn width(&self, s: &str, size: f64) -> f64 { match self { - Self::WinAnsi | Self::Simple(_) => helvetica_width(s, size), + Self::WinAnsi => helvetica_width(s, size), + // The font's `/Widths`, or its standard-14 metrics (Courier, Times) when it has none. + Self::Simple(metrics) => match metrics.encode(&s.replace(['\t', '\r', '\n'], " ")) { + Some(bytes) => metrics.codes(&bytes).iter().map(|&(code, _)| metrics.width(code)).sum::() * size, + None => helvetica_width(s, size), + }, Self::Unicode(_) => cjk_width(s, size), } } diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index ef545cd433..fb7281be6e 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -364,6 +364,47 @@ fn multiline_wrapping_keeps_encodable_text_without_requiring_ascii_space() { assert_eq!(metrics.decode(&lines.concat()), value.replace('\n', "")); } +/// A field drawn in its `/DR` font is laid out with that font's widths, not Helvetica's (#2894): +/// Courier is 0.6 em a character, so 10 pt text holds 94 to a line in a 564 pt box, and a +/// right-aligned value ends at the padding in Courier and in Times. +#[test] +fn reused_standard_fonts_wrap_and_align_with_their_own_widths() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [7 0 R 8 0 R 9 0 R] >>".into(), + "<< /Fields [7 0 R 8 0 R 9 0 R] /DA (/Cour 10 Tf 0 g) /DR << /Font << /Cour 5 0 R /TiRo 6 0 R >> >> >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Courier /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Times-Roman /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (notes) /Rect [21 537 591 722] /Ff 4096 /DA (/Cour 10 Tf 0 g) /MK << /BC [0 0 0] >> /P 3 0 R >>" + .into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (code) /Q 2 /Rect [21 500 221 520] /DA (/Cour 10 Tf 0 g) /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (initials) /Q 2 /Rect [21 470 221 490] /DA (/TiRo 10 Tf 0 g) /P 3 0 R >>".into(), + ]; + let mut doc = document(&objs); + let value = "The quick brown fox jumps over the lazy dog, then it runs into the field and keeps on going well past the right edge of the box. Every line here is plain mixed-case ASCII text in Courier at 10 points."; + set_value(&mut doc, "notes", &FieldValue::Text(value.into())).unwrap(); + set_value(&mut doc, "code", &FieldValue::Text("iiii".into())).unwrap(); + set_value(&mut doc, "initials", &FieldValue::Text("iiii".into())).unwrap(); + let doc = reopen(&doc); + let all = fields(&doc); + let (_, lines) = appearance_text(&doc, &field(&all, "notes").widgets[0], b"Cour"); + let lengths: Vec = lines.iter().map(Vec::len).collect(); + assert!(lengths.iter().all(|&l| l <= 94), "every line fits the box in Courier: {lengths:?}"); + assert!(lengths[0] > 85, "and the first is not wrapped early: {lengths:?}"); + // Where each right-aligned value starts: the box is 200 pt with 2 pt padding, so `iiii` ends + // at 198 and starts 24 pt (Courier) or 11.12 pt (Times) before it, against 8.88 in Helvetica. + let start = |name: &str| { + let w = &field(&all, name).widgets[0]; + let n = doc.get(w.obj).as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().reference(b"N").unwrap(); + let Object::Stream(s) = &*doc.get(n) else { panic!() }; + let tm = pdfcraft_content::parse(&s.decoded().unwrap()).ops.into_iter().find(|op| op.is("Tm")).unwrap(); + tm.operands[4].as_f64().unwrap() + }; + assert!((start("code") - 174.0).abs() < 1e-3, "Courier: {}", start("code")); + assert!((start("initials") - 186.88).abs() < 1e-3, "Times: {}", start("initials")); +} + #[test] fn japanese_choices_use_the_unicode_cid_font() { // A choice field whose /DA names a non-embedded CID font with a predefined Unicode CMap From 0b77f4cb771ba903233d9f8d92f5980545dd1ee8 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:21:22 +0800 Subject: [PATCH 07/34] fix(ui): Fill & Sign font size resizes the selected text (#1274) The Font size control on the Fill & Sign bar only set the size of text typed afterwards; text already placed and picked up kept its size, so the only way to resize it was to type it again. With a placed Fill & Sign text or date selected, a size typed into the control (or the end of a drag) now applies to it through Edit::FontSizeAnnotation, one undo step per change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz --- crates/ui-egui/src/canvas.rs | 19 ++++++++++- crates/ui-egui/tests/fill_sign.rs | 54 +++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 0acab64a08..f39fbc24b7 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -4582,12 +4582,29 @@ fn quick_bar(app: &mut PdfCraftApp, area: Rect, ui: &mut egui::Ui) { }); if matches!(current_fill, Some(crate::fill_sign::FillTool::Text | crate::fill_sign::FillTool::Date)) { ui.label(tl!("Font size:")); - ui.add( + let size = ui.add( egui::DragValue::new(&mut app.fill_text_size) .range(crate::fill_sign::MIN_TEXT_SIZE..=crate::fill_sign::MAX_TEXT_SIZE) .speed(0.25) .suffix(" pt"), ); + // #1274: the size also applies to the Fill & Sign text that is selected, + // once per change (a typed value, or the end of a drag), so a drag adds + // one undo step and not one per frame. + if ((size.changed() && !size.dragged()) || size.drag_stopped()) + && let Some((index, id)) = app.active_ids() + && let Some((page, annotation)) = app.views.get(index).and_then(|view| view.comments.selected) + && app.session.get(id).is_some_and(|doc| { + doc.allows_annotation() + && doc + .info + .annotations + .iter() + .any(|a| a.page == page && a.index == annotation && a.fill_sign && a.subtype == "FreeText" && !a.locked) + }) + { + app.apply_edit(Edit::FontSizeAnnotation { page, index: annotation, size: app.fill_text_size }); + } } if let QuickTool::Comment(tool) = app.quick_tool { let (r, _) = ui.allocate_exact_size(vec2(32.0, 9.0), Sense::hover()); diff --git a/crates/ui-egui/tests/fill_sign.rs b/crates/ui-egui/tests/fill_sign.rs index 27fd6dd8a5..8f4019fe69 100644 --- a/crates/ui-egui/tests/fill_sign.rs +++ b/crates/ui-egui/tests/fill_sign.rs @@ -168,6 +168,60 @@ fn fill_text_size_controls_new_text_and_survives_settings_restore() { assert_eq!(restored.fill_text_size, 24.0, "invalid settings do not change the size"); } +/// #1274: with a placed Fill & Sign text selected, the Font size control resizes that text, +/// not only text typed afterwards; with nothing selected it changes nothing on the page. +#[test] +fn fill_text_size_resizes_the_selected_text() { + let mut h = harness(); + h.state_mut().execute("sign.fill.text"); + click(&mut h, 50.0, 250.0); + h.event(egui::Event::Text("Resize me".into())); + h.run_steps(1); + h.key_press(egui::Key::Enter); + h.run_steps(3); + let height = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + let a = s.session.get(s.views[0].id).unwrap().info.annotations.iter().find(|a| a.contents.as_deref() == Some("Resize me")).cloned().unwrap(); + a.rect[3] - a.rect[1] + }; + let before = height(&h); + let set_size = |h: &mut Harness<'static, PdfCraftApp>, value: &str| { + h.query_all_by_role(egui::accesskit::Role::SpinButton) + .find(|n| n.value().is_some_and(|v| v.ends_with(" pt"))) + .expect("the Font size control") + .click(); + h.run_steps(2); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::A); + h.event(egui::Event::Text(value.into())); + h.run_steps(1); + h.key_press(egui::Key::Enter); + h.run_steps(4); + }; + // Nothing selected: only the size for new text changes. + h.state_mut().views[0].comments.selected = None; + set_size(&mut h, "18"); + assert_eq!(h.state().fill_text_size, 18.0); + assert_eq!(height(&h), before, "an unselected placed text keeps its size"); + // The placed text picked up with the Add text tool, then resized. + let annotation = { + let s = h.state(); + s.session + .get(s.views[0].id) + .unwrap() + .info + .annotations + .iter() + .find(|a| a.contents.as_deref() == Some("Resize me")) + .map(|a| (a.page, a.index)) + .unwrap() + }; + h.state_mut().views[0].comments.selected = Some(annotation); + set_size(&mut h, "36"); + assert_eq!(h.state().fill_text_size, 36.0); + let after = height(&h); + assert!(after > before * 2.0, "36 pt text has a taller box than 10 pt text: {before} -> {after}"); +} + #[test] fn the_type_box_grows_as_text_is_typed() { let mut h = harness(); From a61bd92fdd43bc6d0dbc273b283197f25090450a Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Sun, 18 Oct 2026 13:21:34 +0200 Subject: [PATCH 08/34] Fonts: measure a remapped standard-14 code by the glyph it draws A standard-14 font with no /Widths was measured by code in the WinAnsi-indexed table, so a /Differences remap took the width of the glyph WinAnsi gives that code: Helvetica with [65 /W 87 /A] laid out "AAAA" four W's wide. The code is now looked up through the font's encoding first; /Widths still go by code, and Symbol, ZapfDingbats and glyphs WinAnsi has no code for keep the code itself. --- crates/fonts/src/pdf.rs | 44 ++++++++++++++++++++++++++++++++++++++- crates/forms/src/tests.rs | 35 +++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 0cc456cba6..674cfa6c71 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -766,7 +766,7 @@ impl Metrics { match self.std14 { // A font with no /Widths is one of the standard 14; use its published metrics. // Codes the font's encoding leaves undefined fall back to /MissingWidth. - Some(f) => match u8::try_from(code).map(|c| f.width(c)).unwrap_or(0.0) { + Some(f) => match u8::try_from(self.std14_code(f, code)).map(|c| f.width(c)).unwrap_or(0.0) { 0.0 => self.default * self.scale, w => w / 1000.0, }, @@ -774,6 +774,17 @@ impl Metrics { } } + /// Where the standard-14 table, indexed by WinAnsi, holds the glyph `code` draws in this font: + /// its encoding (base and `/Differences`) names the glyph, so a remapped code is measured as + /// the glyph it now shows (#2894). Symbol and ZapfDingbats are indexed by their own encodings, + /// and a glyph WinAnsi has no code for keeps the code itself. + fn std14_code(&self, face: crate::Std14, code: u32) -> u32 { + if matches!(face, crate::Std14::Symbol | crate::Std14::ZapfDingbats) { + return code; + } + self.unicode.encoding.get(&code).and_then(|t| only_char(t)).and_then(crate::win_ansi_byte).map_or(code, u32::from) + } + /// Word spacing applies to the single-byte code 32 (§9.3.3). pub fn is_space(&self, code: u32, len: usize) -> bool { code == 32 && len == 1 @@ -1325,6 +1336,37 @@ mod tests { assert_eq!(m.encode("Ω").as_deref(), Some(&b"B"[..])); } + /// A standard-14 font with no `/Widths` is measured by the glyph each code draws, not by the + /// glyph WinAnsi gives that code (#2894); `/Widths`, when present, still go by code. + #[test] + fn standard_fonts_without_widths_measure_the_glyph_their_differences_name() { + let mut doc = Document::new_empty(); + let mut enc = Dict::new(); + enc.set(b"BaseEncoding".to_vec(), Object::name("WinAnsiEncoding")); + let diffs = + vec![Object::Int(65), Object::name("W"), Object::Int(87), Object::name("A"), Object::name("uni03A9"), Object::name("not.a.glyph")]; + enc.set(b"Differences".to_vec(), Object::Array(diffs)); + let f = font( + &mut doc, + vec![("Subtype", Object::name("Type1")), ("BaseFont", Object::name("Helvetica")), ("Encoding", Object::Dict(enc.clone()))], + ); + let m = Metrics::from_dict(&doc, &f); + assert_eq!((m.width(65), m.width(87)), (0.944, 0.667), "A and W swap their widths"); + // Omega has no WinAnsi code and the last name reads as nothing: both keep their code's width. + assert_eq!((m.width(88), m.width(89)), (0.667, 0.667)); + let f = font( + &mut doc, + vec![ + ("Subtype", Object::name("Type1")), + ("BaseFont", Object::name("Helvetica")), + ("FirstChar", Object::Int(87)), + ("Widths", Object::Array(vec![Object::Int(500)])), + ("Encoding", Object::Dict(enc)), + ], + ); + assert_eq!(Metrics::from_dict(&doc, &f).width(87), 0.5, "/Widths go by code"); + } + #[test] fn subset_fonts_only_encode_the_glyphs_they_have() { let mut doc = Document::new_empty(); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fb7281be6e..071c62170d 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -405,6 +405,41 @@ fn reused_standard_fonts_wrap_and_align_with_their_own_widths() { assert!((start("initials") - 186.88).abs() < 1e-3, "Times: {}", start("initials")); } +/// A standard-14 font whose `/Differences` swap A and W draws `AAAA` with code 87, so it is +/// measured as four A's (0.667 em), not as the W that code has in WinAnsi. The same remap with +/// `/Widths` takes the width the font gives code 87, here 0.5 em. +#[test] +fn remapped_standard_fonts_are_measured_by_the_glyph_each_code_draws() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [7 0 R 8 0 R] >>".into(), + "<< /Fields [7 0 R 8 0 R] /DA (/Dif 10 Tf 0 g) /DR << /Font << /Dif 5 0 R /DifW 6 0 R >> >> >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding << /BaseEncoding /WinAnsiEncoding /Differences [65 /W 87 /A] >> >>" + .into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /FirstChar 87 /LastChar 87 /Widths [500] /Encoding << /BaseEncoding /WinAnsiEncoding /Differences [65 /W 87 /A] >> >>" + .into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (std) /Q 2 /Rect [21 500 221 520] /DA (/Dif 10 Tf 0 g) /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (widths) /Q 2 /Rect [21 470 221 490] /DA (/DifW 10 Tf 0 g) /P 3 0 R >>".into(), + ]; + let mut doc = document(&objs); + set_value(&mut doc, "std", &FieldValue::Text("AAAA".into())).unwrap(); + set_value(&mut doc, "widths", &FieldValue::Text("AAAA".into())).unwrap(); + let doc = reopen(&doc); + let all = fields(&doc); + assert_eq!(appearance_text(&doc, &field(&all, "std").widgets[0], b"Dif").1, vec![b"WWWW".to_vec()], "A is drawn by code 87"); + let start = |name: &str| { + let w = &field(&all, name).widgets[0]; + let n = doc.get(w.obj).as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().reference(b"N").unwrap(); + let Object::Stream(s) = &*doc.get(n) else { panic!() }; + let tm = pdfcraft_content::parse(&s.decoded().unwrap()).ops.into_iter().find(|op| op.is("Tm")).unwrap(); + tm.operands[4].as_f64().unwrap() + }; + // The text ends at 198 (200 pt box, 2 pt padding): 26.68 pt of A's, or 20 pt by /Widths. + assert!((start("std") - 171.32).abs() < 1e-3, "no /Widths: {}", start("std")); + assert!((start("widths") - 178.0).abs() < 1e-3, "/Widths: {}", start("widths")); +} + #[test] fn japanese_choices_use_the_unicode_cid_font() { // A choice field whose /DA names a non-embedded CID font with a predefined Unicode CMap From d7f8ba8b57541552d1337c82867996af7a30367c Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 20:13:56 +0200 Subject: [PATCH 09/34] fix(forms): Tab and Space reach check boxes and radio buttons, and a click focuses them (fixes #2223) --- crates/ui-egui/src/canvas.rs | 8 +- crates/ui-egui/src/forms_ui.rs | 137 +++++++++++++++++++++++++---- crates/ui-egui/tests/forms.rs | 155 +++++++++++++++++++++++++++++++-- parity/acrobat-features.toml | 4 +- 4 files changed, 274 insertions(+), 30 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 00ff0eb3a5..bab4d5d476 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2206,9 +2206,13 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s view.find_step(true); } } - if ctx.egui_wants_keyboard_input() { + // A focused check box or radio button holds egui's keyboard focus only for Space, and a + // radio button for the arrows that move within its group: the other page keys still work. + let form_box = view.forms.keys.filter(|(id, _)| ctx.memory(|m| m.focused()) == Some(*id)); + if ctx.egui_wants_keyboard_input() && form_box.is_none() { return; } + let arrows = !form_box.is_some_and(|(_, radio)| radio); // Rotation first: egui matches ⌘+ loosely with respect to Shift. if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::Plus)) || pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::Equals)) @@ -2255,7 +2259,7 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s if pressed(cmd(Key::CloseBracket)) { view.view_history(true); } - let key = |k| ctx.input(|i| i.key_pressed(k)); + let key = |k| (arrows || !matches!(k, Key::ArrowLeft | Key::ArrowRight | Key::ArrowUp | Key::ArrowDown)) && ctx.input(|i| i.key_pressed(k)); if key(Key::Escape) { if view.camera.is_some() || view.camera_done.is_some() { view.pending_action = Some(ViewAction::CancelCamera); diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index 8020e868b2..bf2edf5d70 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -1,10 +1,10 @@ //! Filling in forms on the page (execution plan M6; Acrobat fills fields in every viewing mode). //! //! Text fields open an in-place editor over the widget (Enter or clicking away commits, Escape -//! cancels, Tab moves to the next field). Check boxes and radio buttons toggle on click. List -//! boxes are directly selectable in-place on the page (clicking picks an option; Ctrl-click -//! toggles on multi-select lists). Combo boxes open a list of their options. Every change is one -//! undoable engine edit. +//! cancels, Tab moves to the next field). Check boxes and radio buttons toggle on click, or on +//! Space once Tab or a click has focused them. List boxes are directly selectable in-place on the +//! page (clicking picks an option; Ctrl-click toggles on multi-select lists). Combo boxes open a +//! list of their options. Every change is one undoable engine edit. use std::collections::HashSet; @@ -57,6 +57,13 @@ pub struct FormView { pub notice: Option, /// A push button was clicked: (its field name, what it does). pub button: Option<(String, pdfcraft_engine::form_scripts::ButtonAction)>, + /// The focused check box or radio button was clicked. It toggles once no other edit is + /// queued: the text the click committed goes first, and the toggle is dropped if that text + /// is refused and its field reopens. + pub(crate) toggle: bool, + /// The widget holding egui's keyboard focus for the focused check box or radio button, and + /// whether it takes the arrows (radio buttons). The page keys still work around it. + pub(crate) keys: Option<(egui::Id, bool)>, } /// A form message for the app to show, with document data kept separate so the visible @@ -93,6 +100,29 @@ fn open_focus(f: &FormField, widget: usize) -> Focus { } } +/// Select a radio button, if it isn't selected already. +fn select_edit(f: &FormField, w: &FormWidget) -> Option { + (w.on_state.as_ref() != f.value.first()).then(|| Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::Radio(w.on_state.clone()) }) +} + +/// What clicking a check box or radio button does. +fn toggle_edit(f: &FormField, w: &FormWidget) -> Option { + let value = match f.kind { + FormFieldKind::CheckBox => FieldValue::Check(f.value.is_empty()), + FormFieldKind::Radio => { + let on = w.on_state.clone(); + // Clicking the selected button again turns it off unless the group forbids it. + let value = if f.value.first() == on.as_ref() && !f.has(field_flags::NO_TOGGLE_TO_OFF) { None } else { on }; + if value.as_ref() == f.value.first() { + return None; + } + FieldValue::Radio(value) + } + _ => return None, + }; + Some(Edit::SetFieldValue { name: f.name.clone(), value }) +} + /// Where the pointer is relative to one page's form widgets. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum FieldPointer { @@ -159,18 +189,14 @@ pub(crate) fn page_input( None => view.forms.notice = Some(FormNotice::NoAction(f.name.clone())), }, FormFieldKind::Signature => view.sign.field = Some(f.name.clone()), - FormFieldKind::CheckBox => { - view.forms.focus = None; - view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::Check(f.value.is_empty()) }); - } - FormFieldKind::Radio => { - view.forms.focus = None; - let on = w.on_state.clone(); - // Clicking the selected button again turns it off unless the group forbids it. - let value = if f.value.first() == on.as_ref() && !f.has(field_flags::NO_TOGGLE_TO_OFF) { None } else { on }; - if value.as_ref() != f.value.first() { - view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::Radio(value) }); + FormFieldKind::CheckBox | FormFieldKind::Radio => { + // The box takes the focus, as in Acrobat, so Tab goes on from it (#2223). The overlay + // toggles it, after the edit for any text this commits. + if view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) { + commit(view, form); + view.forms.focus = Some(open_focus(f, wi)); } + view.forms.toggle = true; } // The press already committed the previous field and opened this one (above), unless // press and release came in one frame. Committing again would re-run a validation that @@ -468,6 +494,8 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f if !ctx.input(|i| i.pointer.primary_down()) { view.forms.pressed = false; } + let toggle = std::mem::take(&mut view.forms.toggle); + view.forms.keys = None; let focus = view.forms.focus.clone()?; let Some(f) = form.iter().find(|f| f.name == focus.name) else { view.forms.focus = None; @@ -475,7 +503,15 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f }; let w = f.widgets.get(focus.widget)?; let page = w.page?; - let xf = view.page_xform(page)?; + let Some(xf) = view.page_xform(page) else { + // A check box or radio button scrolled out of view lets go (it has no draft to keep): + // taking the keyboard back when its page returns would turn scrolling arrows into + // choices, or pull the keyboard from wherever the user went meanwhile. + if matches!(f.kind, FormFieldKind::CheckBox | FormFieldKind::Radio) { + view.forms.focus = None; + } + return None; + }; let rect = widget_rect(&xf, info, page, w.rect); let zoom = xf.rect.width() / xf.pw.max(1.0); let t = Tokens::get(ctx); @@ -628,6 +664,69 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f close = true; } } + FormFieldKind::CheckBox | FormFieldKind::Radio => { + // Nothing is drawn over the box, but a widget there holds egui's keyboard focus, so + // Space, Tab and the arrows come here instead of turning or scrolling the page. + let id = egui::Id::new(("form-field", view.id.0, &f.name, focus.widget)); + let mut has_focus = false; + // Not interactable, so clicks still reach the page under it. + let area = egui::Area::new(egui::Id::new(("form-toggle", view.id.0))).order(egui::Order::Foreground).interactable(false); + area.fixed_pos(rect.min).show(ctx, |ui| { + let r = ui.interact(rect, id, egui::Sense::focusable_noninteractive()); + if let Some(fx) = view.forms.focus.as_mut() + && std::mem::take(&mut fx.request_focus) + { + r.request_focus(); + } + let filter = egui::EventFilter { tab: true, horizontal_arrows: true, vertical_arrows: true, escape: false }; + ui.memory_mut(|m| m.set_focus_lock_filter(id, filter)); + has_focus = r.has_focus(); + }); + view.forms.keys = Some((id, f.kind == FormFieldKind::Radio)); + let (esc, tab, shift) = ctx.input(|i| (i.key_pressed(egui::Key::Escape), i.key_pressed(egui::Key::Tab), i.modifiers.shift)); + let space = has_focus && ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Space)); + let arrow = has_focus && f.kind == FormFieldKind::Radio; + let step = ctx.input(|i| { + if !arrow { + None + } else if i.key_pressed(egui::Key::ArrowRight) || i.key_pressed(egui::Key::ArrowDown) { + Some(true) + } else if i.key_pressed(egui::Key::ArrowLeft) || i.key_pressed(egui::Key::ArrowUp) { + Some(false) + } else { + None + } + }); + let clicked_away = ctx.input(|i| i.pointer.any_click() && !i.pointer.hover_pos().is_some_and(|p| rect.contains(p))); + if esc || clicked_away { + close = true; + } else if tab { + next = Some(!shift); + } else if toggle && view.pending_edit.is_some() { + view.forms.toggle = true; + } else if toggle { + edit = toggle_edit(f, w); + } else if space { + // Space toggles a check box and selects a radio button, as in browsers: it never + // turns the selected button off. + edit = if f.kind == FormFieldKind::Radio { select_edit(f, w) } else { toggle_edit(f, w) }; + } else if let Some(forward) = step { + // The arrows move to the group's next or previous button and select it, wrapping. + let stops: Vec = f.widgets.iter().enumerate().filter(|(_, w)| w.page.is_some() && !w.hidden).map(|(i, _)| i).collect(); + if let Some(pos) = stops.iter().position(|&i| i == focus.widget) { + let k = if forward { (pos + 1) % stops.len() } else { (pos + stops.len() - 1) % stops.len() }; + if let Some((wi, target)) = stops.get(k).and_then(|&wi| Some((wi, f.widgets.get(wi)?))) { + view.forms.focus = Some(open_focus(f, wi)); + edit = select_edit(f, target); + if let Some(p) = target.page + && view.page_screen_rect(p).is_none() + { + view.go_to_page(p); + } + } + } + } + } _ => close = true, } if close { @@ -636,11 +735,11 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f if let Some(forward) = next { commit(view, form); let pending = view.pending_edit.take(); - // The next widget that takes typing or a choice, in the document's tab order (each - // page's /Tabs: rows, columns or structure), wrapping. + // The next field that can be filled in, check boxes and radio buttons included (#2223), in + // the document's tab order (each page's /Tabs: rows, columns or structure), wrapping. let mut order: Vec<(usize, &FormField, usize)> = form .iter() - .filter(|x| fillable(x) && matches!(x.kind, FormFieldKind::Text | FormFieldKind::Combo | FormFieldKind::List)) + .filter(|x| fillable(x)) .flat_map(|x| x.widgets.iter().enumerate().filter(|(_, w)| w.page.is_some() && !w.hidden).map(move |(wi, w)| (w.tab, x, wi))) .collect(); order.sort_by_key(|o| o.0); diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index 96a0a6a11e..fa9732c481 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -54,12 +54,8 @@ fn typing_into_a_text_field_and_tabbing_on() { h.key_press(egui::Key::Tab); h.run_steps(4); assert_eq!(value(&h, "name"), ["Ada Lovelace"]); - assert_eq!( - h.state().views[0].forms.focus.as_ref().map(|f| f.name.as_str()), - Some("country"), - "Tab moves to the next field that takes typing or a choice" - ); - // The country list is open; Escape closes it. Escape also abandons a draft. + assert_eq!(h.state().views[0].forms.focus.as_ref().map(|f| f.name.as_str()), Some("agree"), "Tab moves to the next field"); + // Escape leaves the check box. It also abandons a draft. h.key_press(egui::Key::Escape); h.run_steps(2); click_field(&mut h, "city", 0); @@ -96,6 +92,144 @@ fn check_boxes_radios_and_choices() { assert_eq!(value(&h, "country"), ["fr"]); } +fn focused(h: &Harness<'static, PdfCraftApp>) -> Option<(String, usize)> { + h.state().views[0].forms.focus.as_ref().map(|f| (f.name.clone(), f.widget)) +} + +/// A text field, a check box, another text field and a pair of radio buttons, one under the +/// other, then two empty pages. +fn check_box_harness() -> Harness<'static, PdfCraftApp> { + let bytes = pdf_from_objects(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 0 R 6 0 R 10 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >> >>", + "<< /Type /Pages /Kids [3 0 R 9 0 R 13 0 R] /Count 3 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R 11 0 R 12 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (first) /Rect [20 350 280 372] /P 3 0 R /F 4 >>", + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (agree) /V /Off /AS /Off /Rect [20 310 36 326] /P 3 0 R /F 4 /AP << /N << /Yes 8 0 R /Off 8 0 R >> >> >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (last) /Rect [20 270 280 292] /P 3 0 R /F 4 >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + "<< /Type /XObject /Subtype /Form /BBox [0 0 16 16] /Length 0 >>\nstream\n\nendstream", + "<< /Type /Page /Parent 2 0 R >>", + "<< /FT /Btn /T (pick) /Ff 32768 /V /Off /Kids [11 0 R 12 0 R] >>", + "<< /Type /Annot /Subtype /Widget /Parent 10 0 R /AS /Off /Rect [20 230 36 246] /P 3 0 R /F 4 /AP << /N << /A 8 0 R /Off 8 0 R >> >> >>", + "<< /Type /Annot /Subtype /Widget /Parent 10 0 R /AS /Off /Rect [60 230 76 246] /P 3 0 R /F 4 /AP << /N << /B 8 0 R /Off 8 0 R >> >> >>", + "<< /Type /Page /Parent 2 0 R >>", + ]); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("agree.pdf", None, bytes).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("zoom", "150").unwrap(); + app + }); + h.run_steps(6); + h +} + +#[test] +fn tab_and_space_fill_in_a_check_box_between_text_fields() { + // Issue #2223: Tab went from `first` straight to `last`, and Space could not tick `agree`. + let mut h = check_box_harness(); + click_field(&mut h, "first", 0); + h.event(egui::Event::Text("Ada".into())); + h.run_steps(1); + h.key_press(egui::Key::Tab); + h.run_steps(3); + assert_eq!(value(&h, "first"), ["Ada"]); + assert_eq!(focused(&h), Some(("agree".into(), 0)), "Tab stops on the check box"); + assert!(h.ctx.egui_wants_keyboard_input(), "the check box has the keyboard, not the page"); + h.key_press(egui::Key::Space); + h.run_steps(3); + assert_eq!(value(&h, "agree"), ["Yes"], "Space ticks it"); + assert_eq!(focused(&h), Some(("agree".into(), 0)), "and it keeps the focus"); + h.key_press(egui::Key::Space); + h.run_steps(3); + assert!(value(&h, "agree").is_empty(), "Space again clears it"); + h.key_press(egui::Key::Tab); + h.run_steps(3); + assert_eq!(focused(&h), Some(("last".into(), 0))); + h.key_press_modifiers(egui::Modifiers::SHIFT, egui::Key::Tab); + h.run_steps(3); + assert_eq!(focused(&h), Some(("agree".into(), 0)), "Shift+Tab comes back to it"); +} + +#[test] +fn clicking_a_check_box_while_typing_keeps_the_text_and_focuses_the_box() { + // Issue #2223: the click ticked the box but left no field focused, so Tab started over. + let mut h = check_box_harness(); + click_field(&mut h, "first", 0); + h.event(egui::Event::Text("Ada".into())); + h.run_steps(1); + click_field(&mut h, "agree", 0); + assert_eq!(value(&h, "agree"), ["Yes"]); + assert_eq!(value(&h, "first"), ["Ada"], "the text typed before the click is kept"); + assert_eq!(focused(&h), Some(("agree".into(), 0)), "the click focuses the check box"); + h.key_press(egui::Key::Tab); + h.run_steps(3); + assert_eq!(focused(&h), Some(("last".into(), 0)), "so Tab goes on from there"); +} + +#[test] +fn page_keys_still_work_with_a_check_box_or_radio_button_focused() { + // The widget that takes Space for a focused box must not swallow the page keys. + let mut h = check_box_harness(); + click_field(&mut h, "agree", 0); + assert_eq!(focused(&h), Some(("agree".into(), 0))); + h.key_press(egui::Key::End); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 2, "End goes to the last page"); + assert!(h.state().views[0].page_screen_rect(0).is_none(), "where the check box is out of view"); + // Scrolled away, the box lets go: back on its page, Space no longer toggles it. + h.key_press(egui::Key::Home); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 0); + assert_eq!(focused(&h), None, "the box let go of the focus"); + h.key_press(egui::Key::Space); + h.run_steps(3); + assert_eq!(value(&h, "agree"), ["Yes"]); + // A radio button keeps the arrows for its group, so → selects B instead of turning the page. + click_field(&mut h, "pick", 0); + h.key_press(egui::Key::ArrowRight); + h.run_steps(3); + assert_eq!(value(&h, "pick"), ["B"]); + assert_eq!(h.state().views[0].current, 0, "the arrow stayed in the group"); + // Once scrolled away, an arrow scrolls again instead of changing the choice. + h.key_press(egui::Key::End); + h.run_steps(4); + h.key_press(egui::Key::Home); + h.run_steps(4); + h.key_press(egui::Key::ArrowUp); + h.run_steps(3); + assert_eq!((focused(&h), value(&h, "pick")), (None, vec!["B".to_string()])); +} + +#[test] +fn radio_buttons_take_tab_space_and_arrows() { + let mut h = harness(); + click_field(&mut h, "agree", 0); + h.key_press(egui::Key::Tab); + h.run_steps(3); + assert_eq!(focused(&h), Some(("size".into(), 0))); + h.key_press(egui::Key::Space); + h.run_steps(3); + assert_eq!(value(&h, "size"), ["S"], "Space selects the focused button"); + h.key_press(egui::Key::Space); + h.run_steps(3); + assert_eq!(value(&h, "size"), ["S"], "and doesn't turn it off again"); + h.key_press(egui::Key::ArrowRight); + h.run_steps(3); + assert_eq!(focused(&h), Some(("size".into(), 1)), "an arrow moves within the group"); + assert_eq!(value(&h, "size"), ["L"], "and selects the button it lands on"); + h.key_press(egui::Key::ArrowLeft); + h.run_steps(3); + assert_eq!((focused(&h), value(&h, "size")), (Some(("size".into(), 0)), vec!["S".to_string()])); + h.key_press(egui::Key::Tab); + h.run_steps(3); + h.key_press(egui::Key::Tab); + h.run_steps(3); + assert_eq!(focused(&h), Some(("country".into(), 0)), "past the group's second button"); +} + #[test] fn tabbing_into_a_filled_field_selects_it_so_typing_replaces() { let mut h = harness(); @@ -108,10 +242,17 @@ fn tabbing_into_a_filled_field_selects_it_so_typing_replaces() { h.key_press(egui::Key::Enter); h.run_steps(3); assert_eq!(value(&h, "city"), ["Paris"]); - // name → (Tab) country → (Tab) city, whose text is selected on entry. + // name → (Tab) agree → (Tab) size, both buttons → (Tab) country → (Tab) city, whose text is + // selected on entry. click_field(&mut h, "name", 0); h.key_press(egui::Key::Tab); h.run_steps(3); + assert_eq!(focused(&h), Some(("agree".into(), 0))); + for _ in 0..3 { + h.key_press(egui::Key::Tab); + h.run_steps(3); + } + assert_eq!(focused(&h), Some(("country".into(), 0))); h.key_press(egui::Key::Tab); h.run_steps(4); assert_eq!(h.state().views[0].forms.focus.as_ref().map(|f| f.name.as_str()), Some("city")); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index d8fbe77ddf..db514b6da4 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4428,8 +4428,8 @@ milestone = "M6" status = "shipped" commands = ["form.fields"] tools = ["form_fill"] -evidence = ["crates/ui-egui/tests/forms.rs::check_boxes_radios_and_choices", "crates/forms/src/tests.rs::check_boxes_and_radios_switch_states"] -notes = "NoToggleToOff honoured; widgets without appearances get our own" +evidence = ["crates/ui-egui/tests/forms.rs::check_boxes_radios_and_choices", "crates/forms/src/tests.rs::check_boxes_and_radios_switch_states", "crates/ui-egui/tests/forms.rs::tab_and_space_fill_in_a_check_box_between_text_fields", "crates/ui-egui/tests/forms.rs::clicking_a_check_box_while_typing_keeps_the_text_and_focuses_the_box", "crates/ui-egui/tests/forms.rs::radio_buttons_take_tab_space_and_arrows"] +notes = "NoToggleToOff honoured; widgets without appearances get our own. Tab and Shift+Tab stop on check boxes and on each radio button, Space toggles the focused one, and the arrows move within a radio group (#2223); a click focuses the box too" [[feature]] id = "form.fill-choice" From b661ba4584e510f9077852c8af609322ad7cdf35 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:53:42 +0800 Subject: [PATCH 10/34] fix(forms): draw fields the file leaves to the viewer, for display only (#3470) A form with /NeedAppearances true (ISO 32000-2 Table 224: the writer "has not provided appearance streams for all visible widget annotations") showed its text fields empty: PdfCraft only read the flag when writing, and the display copy that draws missing appearances skips widgets. Filled forms from tools that leave drawing to the viewer looked blank, while Acrobat, Firefox and MuPDF show the values. pdfcraft_forms::with_field_appearances returns a copy with text and choice widgets drawn from their values (every one when the flag is set, otherwise those with no /AP /N), with the generator a typed value uses. The engine's display copy (page view, page images, Print as Image, CLI render) applies it after the comment appearances. The document itself is not changed, so saving writes the file as it was. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/engine/src/lib.rs | 19 +++++++---- crates/engine/src/tests.rs | 60 +++++++++++++++++++++++++++++++++++ crates/forms/README.md | 4 +++ crates/forms/src/lib.rs | 28 +++++++++++++++++ crates/forms/src/tests.rs | 61 ++++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +-- 6 files changed, 168 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8f39248abd..3caa0c828b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -876,12 +876,13 @@ fn is_placeholder_title(title: &str) -> bool { address || t == "untitled" } -/// The working file plus appearances for comments that have none. Signature artwork -/// is preserved; validation badges belong to the viewer and never enter these bytes, -/// which image/text exports and Print as Image also consume. +/// The working file plus appearances for comments that have none, and for form fields the file +/// leaves to the viewer (#3470). Signature artwork is preserved; validation badges belong to the +/// viewer and never enter these bytes, which image/text exports and Print as Image also consume. fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { let Some(e) = editor else { return bytes.clone() }; - let Some(display) = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances) else { + let comments = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances); + let Some(display) = with_field_appearances(comments, &e.cos) else { return bytes.clone(); }; match write_incremental(&display, &SaveOptions::default()) { @@ -893,6 +894,12 @@ fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc, doc: &pdfcraft_cos::Document) -> Option { + pdfcraft_forms::with_field_appearances(comments.as_ref().unwrap_or(doc)).or(comments) +} + /// The reader shows the same pages in `a` and `b`: count, displayed size and rotation. fn same_pages(a: &[pdfcraft_render::PageInfo], b: &[pdfcraft_render::PageInfo]) -> bool { a.len() == b.len() @@ -900,14 +907,14 @@ fn same_pages(a: &[pdfcraft_render::PageInfo], b: &[pdfcraft_render::PageInfo]) } /// The copy PDFCraft displays when opening `bytes`: with appearances drawn, in memory only, for -/// comments that have none. Render paths outside the app, such as the CLI, use this so their page +/// comments that have none and form fields the file leaves to the viewer. Render paths outside the app, such as the CLI, use this so their page /// images agree with the app and independent viewers. It never changes the source document, and an /// unparseable file, a write failure or a parser panic falls back to the original bytes. pub fn display_bytes_for(bytes: Arc>, password: Option<&str>) -> Arc> { guard(|| { pdfcraft_cos::Document::open_with_password(bytes.clone(), password) .ok() - .and_then(|cos| pdfcraft_annot::with_missing_appearances(&cos)) + .and_then(|cos| with_field_appearances(pdfcraft_annot::with_missing_appearances(&cos), &cos)) .and_then(|display| write_incremental(&display, &SaveOptions::default()).ok()) .map(Arc::new) }) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f125c41dac..d199eeb700 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6106,3 +6106,63 @@ fn undo_never_makes_an_unsaved_document_clean() { s.undo(new).unwrap(); assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); } + +/// #3470: a filled form whose writer left the appearances to the viewer (`/NeedAppearances`): +/// field A has an empty appearance, B none. The page as displayed (and printed) shows both +/// values, and a comment without an appearance beside them still gets its own; the file itself +/// is not changed, so saving writes it as it was. +#[test] +fn fields_left_to_the_viewer_are_displayed_from_their_values() { + let pdf = |need: bool| { + let flag = if need { "/NeedAppearances true " } else { "" }; + format!( + "%PDF-1.7\n\ + 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm 5 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 400 200] /Annots [6 0 R 7 0 R 10 0 R] >> endobj\n\ + 10 0 obj << /Type /Annot /Subtype /Square /Rect [360 20 390 50] /C [1 0 0] /BS << /W 4 >> >> endobj\n\ + 5 0 obj << /Fields [6 0 R 7 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 8 0 R >> >> >> endobj\n\ + 6 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (a) /V (MMMMMMMM) /DA (/Helv 12 Tf 0 g) /Rect [50 120 350 140] /F 4 /P 3 0 R /AP << /N 9 0 R >> >> endobj\n\ + 7 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (b) /V (MMMMMMMM) /DA (/Helv 12 Tf 0 g) /Rect [50 60 350 80] /F 4 /P 3 0 R >> endobj\n\ + 8 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> endobj\n\ + 9 0 obj << /Type /XObject /Subtype /Form /BBox [0 0 300 20] /Length 4 >> stream\nq\nQ\nendstream endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF" + ) + .into_bytes() + }; + let red = |s: &Session, id: DocId| { + let doc = s.get(id).unwrap(); + let mut r = pdfcraft_render::PageRenderer::new(doc.display.clone(), doc.config.clone()); + let page = r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }); + page.rgba.as_chunks::<4>().0.iter().filter(|p| p[0] > 200 && p[1] < 80 && p[2] < 80).count() + }; + // Dark pixels inside a field's box (user-space rectangle, page drawn at scale 1). + let ink = |s: &Session, id: DocId, y0: u32, y1: u32| { + let doc = s.get(id).unwrap(); + let mut r = pdfcraft_render::PageRenderer::new(doc.display.clone(), doc.config.clone()); + let page = r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }); + let (px, _) = page.rgba.as_chunks::<4>(); + (200 - y1..200 - y0) + .flat_map(|y| (55..345).map(move |x| (x, y))) + .filter(|&(x, y)| px.get((y * page.width + x) as usize).is_some_and(|p| p[0] < 128)) + .count() + }; + let mut s = Session::new(); + let id = s.open("form.pdf", None, Arc::new(pdf(true)), None).unwrap(); + let (a, b) = (ink(&s, id, 122, 138), ink(&s, id, 62, 78)); + assert!(a > 100 && b > 100, "both values are shown: a {a}, b {b}"); + assert!(red(&s, id) > 100, "the comment is drawn too"); + // The file is saved as it was: B still has no appearance, A its empty one. + let saved = pdfcraft_cos::Document::open(s.save_bytes(id).unwrap()).unwrap(); + let fields = pdfcraft_forms::fields(&saved); + let widget = |name: &str| saved.get(fields.iter().find(|f| f.name == name).unwrap().widgets[0].obj); + assert!(widget("b").as_dict().is_some_and(|d| !d.contains(b"AP")), "B has no appearance"); + let n = widget("a").as_dict().and_then(|d| d.get(b"AP").and_then(|ap| saved.resolve(ap).as_dict().and_then(|ap| ap.reference(b"N")))).unwrap(); + let pdfcraft_cos::Object::Stream(n) = &*saved.get(n) else { panic!("A's appearance is a stream") }; + assert_eq!(n.decoded().unwrap().trim_ascii_end(), b"q\nQ", "A keeps its empty appearance"); + + // Without the flag, the empty appearance is the file's own, and only B is drawn. + let id = s.open("form.pdf", None, Arc::new(pdf(false)), None).unwrap(); + let (a, b) = (ink(&s, id, 122, 138), ink(&s, id, 62, 78)); + assert!(a == 0 && b > 100, "a {a}, b {b}"); +} diff --git a/crates/forms/README.md b/crates/forms/README.md index 66fb919fe7..a05b69b945 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -42,6 +42,10 @@ delete_field(&mut doc, "email")?; that have them. Not in a document that declares PDF/A, which forbids that flag (`declares_pdfa`). `undrawn_fields` lists the marked fields; the engine reports each edit that draws one, and flattening refuses to make that `?` permanent. +- Displaying a form that leaves its appearances to the viewer (`/NeedAppearances`, or a text or + choice widget with no `/AP /N`): `with_field_appearances` returns a copy with those widgets + drawn from their values, as typing them would draw them; the engine shows and prints that copy + and saves the file as it was (#3470). - Check boxes and radio buttons switch `/V` and each widget's `/AS`. Widgets with no appearances get PdfCraft's own (a drawn check mark or dot; no symbol font needed). - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 1dfa767262..8d78fb8993 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1232,6 +1232,34 @@ pub fn need_appearances(doc: &Document) -> bool { acroform(doc).is_some_and(|af| matches!(af.get(b"NeedAppearances").map(|o| doc.resolve(o)).as_deref(), Some(Object::Bool(true)))) } +/// A copy of `doc` in which text and choice fields are drawn from their values, for displaying +/// the document (`None` when no widget needs it): every such widget when the form sets +/// `/NeedAppearances` (ISO 32000-2 Table 224: the writer left the appearances to the viewer), +/// and otherwise each widget without a normal appearance (`/AP /N`). Drawn as typing the value +/// would draw it (#3470). `doc` is not changed: saving writes the file as it was. +pub fn with_field_appearances(doc: &Document) -> Option { + let all = need_appearances(doc); + let mut copy: Option = None; + for f in fields(doc).iter().filter(|f| matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List)) { + for w in &f.widgets { + let widget = doc.get(w.obj); + let drawn = widget.as_dict().and_then(|d| d.get(b"AP")).is_some_and(|ap| doc.resolve(ap).as_dict().is_some_and(|ap| ap.contains(b"N"))); + if drawn && !all { + continue; + } + let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + let target = copy.get_or_insert_with(|| doc.clone()); + let ap = target.add(Object::Stream(stream)); + let mut fresh = Dict::new(); + fresh.set(b"N".to_vec(), Object::Ref(ap)); + let apd = merged_appearance(target, w.obj, &fresh); + // Fails only when the widget isn't a dictionary; that widget then keeps its own look. + target.update_dict(w.obj, |d| d.set(b"AP".to_vec(), Object::Dict(apd))).ok(); + } + } + copy +} + /// Whether the document's XMP metadata declares PDF/A conformance (`pdfaid:part`). pub fn declares_pdfa(doc: &Document) -> bool { let Some(root) = doc.root() else { return false }; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd8..987a74f2c6 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2277,3 +2277,64 @@ fn document_scripts_read_each_name_tree_node_once() { ]); assert_eq!(document_scripts_named(&doc), [("init".to_string(), "var ready = true;".to_string())]); } + +/// #3470: a form whose writer left the appearances to the viewer. Field A has an empty one, +/// B none, C its own (`(old C)`), D is a combo without one, E a check box with states, F a list +/// box without one, and G only a down appearance (`/D`, no `/N`). +fn left_to_the_viewer(need_appearances: bool) -> Document { + let flag = if need_appearances { "/NeedAppearances true " } else { "" }; + document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R 8 0 R 9 0 R 10 0 R 13 0 R 14 0 R] >>".into(), + format!("<< /Fields [6 0 R 7 0 R 8 0 R 9 0 R 10 0 R 13 0 R 14 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>"), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (a) /V (Value A) /DA (/Helv 12 Tf 0 g) /Rect [50 700 350 720] /P 3 0 R /AP << /N 11 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (b) /V (Value B) /DA (/Helv 12 Tf 0 g) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (c) /V (Value C) /DA (/Helv 12 Tf 0 g) /Rect [50 600 350 620] /P 3 0 R /AP << /N 12 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Ch /Ff 131072 /T (d) /Opt [(Red) (Blue)] /V (Blue) /DA (/Helv 12 Tf 0 g) /Rect [50 550 350 570] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (e) /V /Yes /AS /Yes /Rect [400 700 415 715] /P 3 0 R /AP << /N << /Yes 11 0 R /Off 11 0 R >> >> >>".into(), + "<< /Length 4 >>\nstream\nq\nQ\nendstream".into(), + "<< /Length 18 >>\nstream\nBT (old C) Tj ET\nendstream".into(), + "<< /Type /Annot /Subtype /Widget /FT /Ch /T (f) /Opt [(Green) (Grey)] /V (Green) /DA (/Helv 12 Tf 0 g) /Rect [50 450 350 520] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (g) /V (Value G) /DA (/Helv 12 Tf 0 g) /Rect [50 400 350 420] /P 3 0 R /AP << /D 11 0 R >> >>".into(), + ]) +} + +/// #3470: with `/NeedAppearances`, every text and choice widget of the display copy is drawn +/// from its value, its own appearance or not; buttons keep their states, and the document +/// itself is untouched. +#[test] +fn need_appearances_draws_every_text_and_choice_field_in_the_display_copy() { + let doc = left_to_the_viewer(true); + let copy = with_field_appearances(&doc).expect("fields to draw"); + let all = fields(©); + for (name, value) in + [("a", "(Value A) Tj"), ("b", "(Value B) Tj"), ("c", "(Value C) Tj"), ("d", "(Blue) Tj"), ("f", "(Green) Tj"), ("g", "(Value G) Tj")] + { + let shown = ap(©, &field(&all, name).widgets[0]); + assert!(shown.contains(value), "{name}: {shown}"); + } + let e = |d: &Document| d.get(field(&fields(d), "e").widgets[0].obj).as_dict().and_then(|w| w.get(b"AP").cloned()); + assert_eq!(e(©), e(&doc), "the check box keeps its states"); + let original = fields(&doc); + assert_eq!(ap(&doc, &field(&original, "a").widgets[0]).trim_end(), "q\nQ", "the document keeps its own appearances"); + assert!(doc.get(field(&original, "b").widgets[0].obj).as_dict().is_some_and(|w| !w.contains(b"AP"))); +} + +/// #3470: without `/NeedAppearances`, only widgets with no normal appearance are drawn; an +/// appearance the file has, even an empty one, is the file's. A form with all of them needs no +/// copy. +#[test] +fn without_need_appearances_only_missing_appearances_are_drawn() { + let doc = left_to_the_viewer(false); + let copy = with_field_appearances(&doc).expect("b, d, f and g have none"); + let all = fields(©); + assert_eq!(ap(©, &field(&all, "a").widgets[0]).trim_end(), "q\nQ"); + assert!(ap(©, &field(&all, "b").widgets[0]).contains("(Value B) Tj")); + assert_eq!(ap(©, &field(&all, "c").widgets[0]).trim_end(), "BT (old C) Tj ET"); + assert!(ap(©, &field(&all, "d").widgets[0]).contains("(Blue) Tj")); + assert!(ap(©, &field(&all, "f").widgets[0]).contains("(Green) Tj")); + assert!(ap(©, &field(&all, "g").widgets[0]).contains("(Value G) Tj"), "a down appearance alone is no normal one"); + assert!(with_field_appearances(©).is_none(), "every widget has one now"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index e19efb0a09..35812170b3 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -806,8 +806,8 @@ milestone = "M2" status = "shipped" commands = [] tools = [] -evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source"] -notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered yet" +evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy"] +notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only. Not cached: a form with 1000 such fields adds about 80 ms to each refresh (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered yet" [[feature]] id = "view.render-error-isolation" From da351cda8f0bef0265da4bedf7d4144cb8020cc3 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:26:31 +0800 Subject: [PATCH 11/34] fix(forms): cache field appearances drawn for display, redraw only what changed (#3470) Review follow-up (CodeRabbit on #3475). A form edit refreshes the display copy on every keystroke, and with /NeedAppearances that redrew every field of the form: 68 ms per edit on a 1000-field form against 7 ms on main. FieldAppearanceCache keeps each widget's drawing with the objects it was drawn from: the widget and its field ancestors, the form dictionary and the fonts of its /DR, each with the objects its entries refer to. An entry is used only while each of them is still the same object (Arc::ptr_eq); the document replaces an object it edits, so a changed input is a miss, never a stale hit. The engine keeps one per editor. A field edit on that form now takes 13 ms. The repair note on a failed display write now names field appearances too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/engine/src/lib.rs | 10 ++- crates/engine/src/tests.rs | 5 ++ crates/forms/README.md | 4 +- crates/forms/src/lib.rs | 127 ++++++++++++++++++++++++++++++++++- crates/forms/src/tests.rs | 107 +++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 6 files changed, 251 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3caa0c828b..af80271b93 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -316,6 +316,10 @@ struct Editor { /// runs (content edits leave every annotation dictionary alone). Cleared on any refresh /// that may change annotations; see [`AppearanceCache`]. appearances: AppearanceCache, + /// Display appearances drawn for form fields the file leaves to the viewer (#3470), reused + /// for each widget while the objects it was drawn from are unchanged; see + /// [`pdfcraft_forms::FieldAppearanceCache`]. + field_appearances: pdfcraft_forms::FieldAppearanceCache, /// The text blocks last computed for one page, reused while the document is unchanged. text_blocks: TextBlocksCache, } @@ -882,13 +886,14 @@ fn is_placeholder_title(title: &str) -> bool { fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { let Some(e) = editor else { return bytes.clone() }; let comments = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances); - let Some(display) = with_field_appearances(comments, &e.cos) else { + let fields = pdfcraft_forms::with_field_appearances_cached(comments.as_ref().unwrap_or(&e.cos), &mut e.field_appearances); + let Some(display) = fields.or(comments) else { return bytes.clone(); }; match write_incremental(&display, &SaveOptions::default()) { Ok(written) => Arc::new(written), Err(err) => { - e.cos.note_repair(format!("missing comment appearances could not be drawn ({err})")); + e.cos.note_repair(format!("missing comment or field appearances could not be drawn ({err})")); bytes.clone() } } @@ -3576,6 +3581,7 @@ impl Session { redo: Vec::new(), keys, appearances: AppearanceCache::default(), + field_appearances: pdfcraft_forms::FieldAppearanceCache::default(), text_blocks: TextBlocksCache::default(), }), None, diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d199eeb700..46dbe4ce36 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6160,6 +6160,11 @@ fn fields_left_to_the_viewer_are_displayed_from_their_values() { let n = widget("a").as_dict().and_then(|d| d.get(b"AP").and_then(|ap| saved.resolve(ap).as_dict().and_then(|ap| ap.reference(b"N")))).unwrap(); let pdfcraft_cos::Object::Stream(n) = &*saved.get(n) else { panic!("A's appearance is a stream") }; assert_eq!(n.decoded().unwrap().trim_ascii_end(), b"q\nQ", "A keeps its empty appearance"); + // Typing in one field redraws that field only, not the whole form, on each keystroke. + let builds = |s: &Session| s.get(id).unwrap().editor.as_ref().unwrap().field_appearances.builds(); + let before = builds(&s); + s.apply(id, Edit::SetFieldValue { name: "a".into(), value: pdfcraft_forms::FieldValue::Text("Typed".into()) }).unwrap(); + assert_eq!(builds(&s), before + 1); // Without the flag, the empty appearance is the file's own, and only B is drawn. let id = s.open("form.pdf", None, Arc::new(pdf(false)), None).unwrap(); diff --git a/crates/forms/README.md b/crates/forms/README.md index a05b69b945..96f1f01f91 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -45,7 +45,9 @@ delete_field(&mut doc, "email")?; - Displaying a form that leaves its appearances to the viewer (`/NeedAppearances`, or a text or choice widget with no `/AP /N`): `with_field_appearances` returns a copy with those widgets drawn from their values, as typing them would draw them; the engine shows and prints that copy - and saves the file as it was (#3470). + and saves the file as it was (#3470). `with_field_appearances_cached` keeps each widget's + drawing while the objects it was drawn from are the same (`FieldAppearanceCache`), so typing in + one field redraws that field, not the form. - Check boxes and radio buttons switch `/V` and each widget's `/AS`. Widgets with no appearances get PdfCraft's own (a drawn check mark or dot; no symbol font needed). - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 8d78fb8993..8a27d3ebd2 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1238,6 +1238,12 @@ pub fn need_appearances(doc: &Document) -> bool { /// and otherwise each widget without a normal appearance (`/AP /N`). Drawn as typing the value /// would draw it (#3470). `doc` is not changed: saving writes the file as it was. pub fn with_field_appearances(doc: &Document) -> Option { + with_field_appearances_cached(doc, &mut FieldAppearanceCache::default()) +} + +/// [`with_field_appearances`], reusing the appearances `cache` drew for widgets whose inputs are +/// the same objects as then (see [`FieldAppearanceCache`]). +pub fn with_field_appearances_cached(doc: &Document, cache: &mut FieldAppearanceCache) -> Option { let all = need_appearances(doc); let mut copy: Option = None; for f in fields(doc).iter().filter(|f| matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List)) { @@ -1247,7 +1253,15 @@ pub fn with_field_appearances(doc: &Document) -> Option { if drawn && !all { continue; } - let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + let inputs = appearance_inputs(doc, w); + let stream = match cache.get(w.obj, &inputs) { + Some(stream) => stream, + None => { + let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + cache.insert(w.obj, inputs, &stream); + stream + } + }; let target = copy.get_or_insert_with(|| doc.clone()); let ap = target.add(Object::Stream(stream)); let mut fresh = Dict::new(); @@ -1260,6 +1274,117 @@ pub fn with_field_appearances(doc: &Document) -> Option { copy } +/// The objects a widget's drawn appearance is built from, as the document holds them: the widget +/// and the fields above it (value, `/DA`, `/Q`, flags, options, `/MK`, formatting actions), the +/// form dictionary (`/DA`) and the fonts of its `/DR`, each with the objects its entries refer to +/// (an indirect `/MK` or `/V`). Not the page: a field's rotation is its own `/MK /R`. +fn appearance_inputs(doc: &Document, w: &Widget) -> Vec> { + // The objects a dictionary's entries refer to. The tree links and the drawn appearance itself + // are not inputs, and `/Kids` and `/Fields` can be long. + let referred = |d: &Dict, inputs: &mut Vec>| { + for (k, v) in d.iter() { + if !matches!(&k[..], b"Kids" | b"Fields" | b"P" | b"Parent" | b"AP") + && let Some(r) = v.as_ref() + { + inputs.push(doc.get(r)); + } + } + }; + let mut inputs = Vec::new(); + let mut seen = std::collections::HashSet::new(); + let mut node = Some(w.obj); + // The /Parent chain; the seen-set and the cap end a cycle or an absurd depth. + while let Some(r) = node.filter(|r| seen.insert(*r) && seen.len() <= 64) { + let obj = doc.get(r); + if let Some(d) = obj.as_dict() { + node = d.reference(b"Parent"); + referred(d, &mut inputs); + } else { + node = None; + } + inputs.push(obj); + } + let Some(root) = doc.root() else { return inputs }; + let catalog = doc.get(root); + let form = match catalog.as_dict().and_then(|c| c.reference(b"AcroForm")) { + Some(r) => doc.get(r), + None => catalog, + }; + if let Some(f) = form.as_dict() { + referred(f, &mut inputs); + // The fonts of /DR. An indirect /DR or /Font is listed through the entries that refer to + // it; a direct one is part of the object that holds it. `held` only finds them: a copy + // of a direct object is a new one each time, never an input. + let held = |o: &Object| o.as_ref().map_or_else(|| std::sync::Arc::new(o.clone()), |r| doc.get(r)); + if let Some(dr) = f.get(b"DR").map(held) + && let Some(dr) = dr.as_dict() + { + referred(dr, &mut inputs); + if let Some(font) = dr.get(b"Font").map(held) + && let Some(fonts) = font.as_dict() + { + referred(fonts, &mut inputs); + } + } + } + inputs.push(form); + inputs +} + +/// Field appearances drawn for the display copy ([`with_field_appearances_cached`]), so a refresh +/// redraws only the widgets whose inputs changed: a form that leaves every appearance to the +/// viewer otherwise redrew all of its fields on each keystroke in any one of them (#3470). An +/// entry holds the input objects it was drawn from and is used only while each is still the same +/// object (`Arc::ptr_eq`); the document replaces an object it edits, so a changed input is a +/// miss, never a stale hit. +#[derive(Clone, Debug, Default)] +pub struct FieldAppearanceCache { + entries: std::collections::HashMap>, pdfcraft_cos::Stream)>, + bytes: usize, + hits: u64, + builds: u64, +} + +impl FieldAppearanceCache { + /// Cached appearance data restarts past this instead of growing without bound. + const MAX_BYTES: usize = 64 << 20; + + /// Forget every entry. + pub fn clear(&mut self) { + *self = Self::default(); + } + + fn get(&mut self, w: ObjRef, inputs: &[std::sync::Arc]) -> Option { + let (from, stream) = self.entries.get(&w)?; + if from.len() != inputs.len() || !from.iter().zip(inputs).all(|(a, b)| std::sync::Arc::ptr_eq(a, b)) { + return None; + } + self.hits += 1; + Some(stream.clone()) + } + + fn insert(&mut self, w: ObjRef, inputs: Vec>, stream: &pdfcraft_cos::Stream) { + self.builds += 1; + if self.bytes.saturating_add(stream.raw.len()) > Self::MAX_BYTES { + self.clear(); + } + self.bytes = self.bytes.saturating_add(stream.raw.len()); + if let Some((_, old)) = self.entries.insert(w, (inputs, stream.clone())) { + self.bytes = self.bytes.saturating_sub(old.raw.len()); + } + } + + /// Cache hits so far (reused appearances); for tests. + pub fn hits(&self) -> u64 { + self.hits + } + + /// Appearances drawn so far; for tests. + pub fn builds(&self) -> u64 { + self.builds + } +} + /// Whether the document's XMP metadata declares PDF/A conformance (`pdfaid:part`). pub fn declares_pdfa(doc: &Document) -> bool { let Some(root) = doc.root() else { return false }; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 987a74f2c6..7a4bd86951 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2338,3 +2338,110 @@ fn without_need_appearances_only_missing_appearances_are_drawn() { assert!(ap(©, &field(&all, "g").widgets[0]).contains("(Value G) Tj"), "a down appearance alone is no normal one"); assert!(with_field_appearances(©).is_none(), "every widget has one now"); } + +/// #3470: refreshing the display copy redraws only the widgets whose inputs changed. Typing in +/// one field of a form that leaves every appearance to the viewer otherwise redrew all of them. +#[test] +fn the_field_appearance_cache_redraws_only_what_changed() { + let mut doc = left_to_the_viewer(true); + let mut cache = FieldAppearanceCache::default(); + with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + let drawn = cache.builds(); + assert_eq!(drawn, 6, "a, b, c, d, f and g"); + with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + assert_eq!((cache.builds(), cache.hits()), (drawn, drawn), "nothing changed, nothing redrawn"); + + set_value(&mut doc, "b", &FieldValue::Text("Typed".into())).unwrap(); + let copy = with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + assert_eq!(cache.builds(), drawn + 1, "only b is redrawn"); + assert!(ap(©, &field(&fields(©), "b").widgets[0]).contains("(Typed) Tj"), "and shows its new value"); + assert!(ap(©, &field(&fields(©), "a").widgets[0]).contains("(Value A) Tj"), "a reuses its drawing"); +} + +/// #3470: each kind of input a field's drawing depends on invalidates its cached appearance when +/// it changes, wherever it lives: the parent field's value, the form's `/DA`, an indirect `/MK` +/// and a `/DR` font. Field H is a parent with one kid widget; I is a plain field. +#[test] +fn the_field_appearance_cache_sees_every_input() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R] >>".into(), + "<< /Fields [5 0 R 7 0 R] /NeedAppearances true /DA (/Helv 12 Tf 0 g) /DR << /Font << /Helv 8 0 R >> >> >>".into(), + "<< /FT /Tx /T (h) /V (Kid value) /Kids [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /Rect [50 700 350 720] /P 3 0 R /MK 9 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (i) /V (Plain) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /BG [1 1 1] >>".into(), + ]); + let r = |n: u32| pdfcraft_cos::ObjRef::new(n, 0); + let mut cache = FieldAppearanceCache::default(); + let mut redraw = |doc: &Document| { + let before = cache.builds(); + let copy = with_field_appearances_cached(doc, &mut cache).expect("fields to draw"); + (cache.builds() - before, copy) + }; + assert_eq!(redraw(&doc).0, 2); + assert_eq!(redraw(&doc).0, 0, "unchanged"); + + doc.update_dict(r(5), |d| d.set(b"V".to_vec(), Object::String(PdfString::text("New value")))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 1, "the parent's value: only its kid"); + assert!(ap(©, &field(&fields(©), "h").widgets[0]).contains("(New value) Tj")); + + doc.update_dict(r(9), |d| d.set(b"BG".to_vec(), Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)]))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 1, "the kid's indirect /MK"); + assert!(ap(©, &field(&fields(©), "h").widgets[0]).contains("1 0 0 rg"), "the new background"); + + doc.update_dict(r(4), |d| d.set(b"DA".to_vec(), Object::String(PdfString::text("/Helv 9 Tf 0 g")))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 2, "the form's /DA: both fields"); + assert!(ap(©, &field(&fields(©), "i").widgets[0]).contains("/Helv 9 Tf")); + + doc.update_dict(r(8), |d| d.set(b"BaseFont".to_vec(), Object::Name(b"Courier".to_vec()))).unwrap(); + assert_eq!(redraw(&doc).0, 2, "a /DR font: both fields"); +} + +/// #3470: an indirect `/DR` and an indirect `/Font` inside it are inputs too, with a font written +/// directly into the `/Font` dictionary, and so is the form's `/DA` when it is an indirect string. +#[test] +fn the_field_appearance_cache_sees_indirect_resources() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R] >>".into(), + "<< /Fields [5 0 R] /NeedAppearances true /DA 8 0 R /DR 6 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (i) /V (Plain) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Font 7 0 R >>".into(), + "<< /Helv << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> >>".into(), + "(/Helv 12 Tf 0 g)".into(), + ]); + let r = |n: u32| pdfcraft_cos::ObjRef::new(n, 0); + let mut cache = FieldAppearanceCache::default(); + let mut redraw = |doc: &Document| { + let before = cache.builds(); + with_field_appearances_cached(doc, &mut cache).expect("a field to draw"); + cache.builds() - before + }; + assert_eq!(redraw(&doc), 1); + assert_eq!(redraw(&doc), 0); + let courier = || { + let mut f = Dict::new(); + f.set(b"Type".to_vec(), Object::Name(b"Font".to_vec())); + f.set(b"Subtype".to_vec(), Object::Name(b"Type1".to_vec())); + f.set(b"BaseFont".to_vec(), Object::Name(b"Courier".to_vec())); + Object::Dict(f) + }; + doc.update_dict(r(7), |d| d.set(b"Helv".to_vec(), courier())).unwrap(); + assert_eq!(redraw(&doc), 1, "the indirect /Font dictionary"); + doc.update_dict(r(6), |d| { + let mut fonts = Dict::new(); + fonts.set(b"Helv".to_vec(), courier()); + d.set(b"Font".to_vec(), Object::Dict(fonts)); + }) + .unwrap(); + assert_eq!(redraw(&doc), 1, "the indirect /DR"); + doc.set(r(8), Object::String(PdfString::text("/Helv 9 Tf 0 g"))); + assert_eq!(redraw(&doc), 1, "the form's indirect /DA"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 35812170b3..8720180037 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -806,8 +806,8 @@ milestone = "M2" status = "shipped" commands = [] tools = [] -evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy"] -notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only. Not cached: a form with 1000 such fields adds about 80 ms to each refresh (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered yet" +evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy", "crates/forms/src/tests.rs::the_field_appearance_cache_sees_every_input"] +notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only and cached per widget (a field edit on a 1000-field /NeedAppearances form takes ~13 ms against ~7 ms on a form that needs none) (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered yet" [[feature]] id = "view.render-error-isolation" From 6b4e396af376895770dce04ee8654514a6887d22 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:34:27 -0400 Subject: [PATCH 12/34] M5: a push button's caption uses the font, size and color of its /DA Button appearances always drew the caption in regular Helvetica, sized from the button's height, whatever the field's /DA asked for: DA Form 31's 'Attachments Menu' (9 pt bold) came out at 4 pt. The caption now takes the /DA's font from the form's /DR, its size (an automatic size still fits the button) and its color, as text fields already do. --- crates/forms/src/appearance.rs | 20 ++++++++++++++++++++ crates/forms/src/author.rs | 29 ++++++++++++----------------- crates/forms/src/tests.rs | 24 ++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 4ec22f0114..502bd9dbfa 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -220,6 +220,26 @@ fn dr_font(doc: &Document, name: &str) -> Option<(Object, FontText)> { Some((entry, FontText::Simple(Box::new(Metrics::from_dict(doc, fd))))) } +/// A push button's caption, centered in a `width` × `height` box, in the font, size and color its +/// `/DA` asks for: the font from the form's `/DR`, else Helvetica, and the size `auto` when the +/// `/DA` leaves it to the viewer (0). Returns the content operators, the font resource (name and +/// object), and the first character drawn as `?` because the font can't show it. +pub(crate) fn button_caption(doc: &Document, da: &str, caption: &str, width: f64, height: f64, auto: f64) -> (Vec, String, Object, Option) { + let da = parse_da(da); + let (name, font, enc) = match dr_font(doc, &da.font).filter(|(_, enc)| enc.can_encode(caption)) { + Some((o, enc)) => (da.font.clone(), o, enc), + None => ("Helv".to_string(), helvetica(), FontText::WinAnsi), + }; + let missing = if enc == FontText::WinAnsi { pdfcraft_fonts::first_undrawable(caption) } else { None }; + let size = if da.size > 0.0 { da.size } else { auto }; + let tw = enc.width(caption, size); + let mut out = + format!("BT /{name} {size:.2} Tf {} 1 0 0 1 {:.3} {:.3} Tm ", da.color, (width - tw) / 2.0, (height - size * 0.7) / 2.0).into_bytes(); + out.extend(literal(&enc.encode(caption))); + out.extend_from_slice(b" Tj ET\n"); + (out, name, font, missing) +} + fn helvetica() -> Object { let mut f = Dict::new(); f.set(b"Type".to_vec(), Object::name("Font")); diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index 521d1f5278..40c94bec08 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -7,7 +7,6 @@ //! immediately so every viewer shows the empty field. use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, Stream}; -use pdfcraft_fonts::{helvetica_width, literal, win_ansi}; use crate::{Field, FieldKind, FormError, Widget, appearance, fields, flags, page_refs}; @@ -706,7 +705,7 @@ fn redraw_widgets(doc: &mut Document, name: &str, only: Option) -> Result appearance::check_box_states(doc, w, f.kind, &on) } FieldKind::PushButton => { - let (s, m) = button_appearance(doc, w); + let (s, m) = button_appearance(doc, w, &f.da); missing = m; let r = doc.add(Object::Stream(s)); let mut d = Dict::new(); @@ -767,7 +766,9 @@ fn form_stream(place: crate::appearance::Placement, content: Vec, resources: /// A push button: background, border, its icon (`/MK /I`, scaled to fit and centred) and its /// centred caption (unless the layout is icon only, `/TP 1`). The caption is drawn in Helvetica, /// with the first character it can't draw (drawn as `?`), for the caller to mark and report. -fn button_appearance(doc: &Document, w: &Widget) -> (Stream, Option) { +/// A push button's appearance: its icon and caption, the caption in the font its `/DA` asks for +/// (the widget's, else `field_da`, the field's own or inherited one). +fn button_appearance(doc: &Document, w: &Widget, field_da: &str) -> (Stream, Option) { let (mut c, place) = frame_only(doc, w); let (width, height) = (place.layout_w, place.layout_h); let wobj = doc.get(w.obj); @@ -792,21 +793,15 @@ fn button_appearance(doc: &Document, w: &Widget) -> (Stream, Option) { } } let mut missing = None; - if !icon_only && !caption.is_empty() { - missing = pdfcraft_fonts::first_undrawable(&caption); - let size = ((height - 4.0) * 0.6).clamp(4.0, 14.0); - let tw = helvetica_width(&caption, size); - content.extend(format!("BT /Helv {size:.2} Tf 0 g 1 0 0 1 {:.3} {:.3} Tm ", (width - tw) / 2.0, (height - size * 0.7) / 2.0).bytes()); - content.extend(literal(&win_ansi(&caption))); - content.extend_from_slice(b" Tj ET\n"); - } - let mut font = Dict::new(); - font.set(b"Type".to_vec(), Object::name("Font")); - font.set(b"Subtype".to_vec(), Object::name("Type1")); - font.set(b"BaseFont".to_vec(), Object::name("Helvetica")); - font.set(b"Encoding".to_vec(), Object::name("WinAnsiEncoding")); let mut fonts = Dict::new(); - fonts.set(b"Helv".to_vec(), Object::Dict(font)); + if !icon_only && !caption.is_empty() { + let da = wobj.as_dict().and_then(|d| d.get(b"DA")).and_then(|o| doc.resolve(o).as_string().map(|s| s.to_text())); + let auto = ((height - 4.0) * 0.6).clamp(4.0, 14.0); + let (ops, name, font, m) = appearance::button_caption(doc, da.as_deref().unwrap_or(field_da), &caption, width, height, auto); + content.extend(ops); + fonts.set(name.into_bytes(), font); + missing = m; + } let mut res = Dict::new(); res.set(b"Font".to_vec(), Object::Dict(fonts)); if !xobjects.is_empty() { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd8..0942a52e6c 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -87,6 +87,30 @@ fn ap(doc: &Document, w: &Widget) -> String { String::from_utf8_lossy(&s.decoded().unwrap()).into_owned() } +#[test] +fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [7 0 R 8 0 R] >>".into(), + "<< /Fields [7 0 R 8 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R /HeBo 6 0 R >> >> >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica-Bold /Encoding /WinAnsiEncoding >>".into(), + // Its /DA asks for 9pt Helvetica Bold in blue. + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (menu) /Ff 65536 /DA (/HeBo 9 Tf 0 0 1 rg) /MK << /CA (Menu) >> /Rect [50 700 140 711] /P 3 0 R >>".into(), + // An automatic size (0): sized to the button, as before. + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (auto) /Ff 65536 /MK << /CA (Auto) >> /Rect [50 600 140 630] /P 3 0 R >>".into(), + ]; + let mut doc = document(&objs); + redraw_field(&mut doc, "menu").unwrap(); + redraw_field(&mut doc, "auto").unwrap(); + let all = fields(&doc); + let menu = ap(&doc, &field(&all, "menu").widgets[0]); + assert!(menu.contains("/HeBo 9.00 Tf") && menu.contains("0 0 1 rg"), "{menu}"); + let auto = ap(&doc, &field(&all, "auto").widgets[0]); + assert!(auto.contains("/Helv 14.00 Tf") && auto.contains(" 0 g"), "{auto}"); +} + #[test] fn button_caption_preserves_other_widgets_and_shared_dictionaries() { for indirect_caption in [false, true] { From c0250b5956616518cf02b80d6da83dd2a1a13e79 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:40:30 -0400 Subject: [PATCH 13/34] M5: form text is measured in its own font, not always Helvetica Appearance text in a simple font was measured with Helvetica's widths, whatever the font: a bold or Times caption or value came out narrower than drawn, so centered and right-aligned text slid right of where it belongs. Text is now measured with the font's /Widths, or a standard font's published widths. --- crates/forms/src/appearance.rs | 6 ++++++ crates/forms/src/tests.rs | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 502bd9dbfa..57dd0d4795 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -191,6 +191,12 @@ impl FontText { fn width(&self, s: &str, size: f64) -> f64 { match self { + // A simple font's own widths (its /Widths, or a standard font's published ones: a + // bold face is wider than Helvetica), one byte per code. + Self::Simple(m) if !m.composite => match m.encode(&s.replace('\t', " ")) { + Some(codes) => codes.iter().map(|&c| m.width(u32::from(c))).sum::() * size, + None => helvetica_width(s, size), + }, Self::WinAnsi | Self::Simple(_) => helvetica_width(s, size), Self::Unicode(_) => cjk_width(s, size), } diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 0942a52e6c..668db63328 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -109,6 +109,10 @@ fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { assert!(menu.contains("/HeBo 9.00 Tf") && menu.contains("0 0 1 rg"), "{menu}"); let auto = ap(&doc, &field(&all, "auto").widgets[0]); assert!(auto.contains("/Helv 14.00 Tf") && auto.contains(" 0 g"), "{auto}"); + // Centered by the bold face's own widths (wider than regular Helvetica's). + let bold: f64 = "Menu".bytes().map(|c| pdfcraft_fonts::Std14::HelveticaBold.width(c)).sum::() / 1000.0 * 9.0; + let x = (90.0 - bold) / 2.0; + assert!(menu.contains(&format!("1 0 0 1 {x:.3} ")), "centered at {x:.3}: {menu}"); } #[test] From 2556a3acb25f0ad6fcaa2181c6649e9291a8c625 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:55:25 -0400 Subject: [PATCH 14/34] M6: the parity notes say button labels follow their /DA --- parity/acrobat-features.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..64208a0c89 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4920,8 +4920,8 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props"] -evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/ui-egui/tests/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically"] -notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen; icon-only layout and existing Helvetica limits retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" +evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically"] +notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by the font's own widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" [[feature]] id = "form.props-actions" From 24a45db0459c785e157c89400e565e0f0db3a8cf Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:59:50 -0400 Subject: [PATCH 15/34] M5: leave measuring form text in its own font to #2900 --- crates/forms/src/appearance.rs | 6 ------ crates/forms/src/tests.rs | 4 ---- parity/acrobat-features.toml | 2 +- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 57dd0d4795..502bd9dbfa 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -191,12 +191,6 @@ impl FontText { fn width(&self, s: &str, size: f64) -> f64 { match self { - // A simple font's own widths (its /Widths, or a standard font's published ones: a - // bold face is wider than Helvetica), one byte per code. - Self::Simple(m) if !m.composite => match m.encode(&s.replace('\t', " ")) { - Some(codes) => codes.iter().map(|&c| m.width(u32::from(c))).sum::() * size, - None => helvetica_width(s, size), - }, Self::WinAnsi | Self::Simple(_) => helvetica_width(s, size), Self::Unicode(_) => cjk_width(s, size), } diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 668db63328..0942a52e6c 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -109,10 +109,6 @@ fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { assert!(menu.contains("/HeBo 9.00 Tf") && menu.contains("0 0 1 rg"), "{menu}"); let auto = ap(&doc, &field(&all, "auto").widgets[0]); assert!(auto.contains("/Helv 14.00 Tf") && auto.contains(" 0 g"), "{auto}"); - // Centered by the bold face's own widths (wider than regular Helvetica's). - let bold: f64 = "Menu".bytes().map(|c| pdfcraft_fonts::Std14::HelveticaBold.width(c)).sum::() / 1000.0 * 9.0; - let x = (90.0 - bold) / 2.0; - assert!(menu.contains(&format!("1 0 0 1 {x:.3} ")), "centered at {x:.3}: {menu}"); } #[test] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 64208a0c89..af2a04bbaa 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4921,7 +4921,7 @@ status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props"] evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically"] -notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by the font's own widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" +notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by Helvetica's widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" [[feature]] id = "form.props-actions" From 0ca7fb9be96dfacbf358e84d54358f1cf8b57cb0 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:25:19 -0400 Subject: [PATCH 16/34] M5: a button caption's characters its font's encoding can't hold are reported --- crates/forms/src/appearance.rs | 2 +- crates/forms/src/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 502bd9dbfa..35bb9146ac 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -230,7 +230,7 @@ pub(crate) fn button_caption(doc: &Document, da: &str, caption: &str, width: f64 Some((o, enc)) => (da.font.clone(), o, enc), None => ("Helv".to_string(), helvetica(), FontText::WinAnsi), }; - let missing = if enc == FontText::WinAnsi { pdfcraft_fonts::first_undrawable(caption) } else { None }; + let missing = enc.first_unencodable(caption); let size = if da.size > 0.0 { da.size } else { auto }; let tw = enc.width(caption, size); let mut out = diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 0942a52e6c..8bc81851ab 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -111,6 +111,26 @@ fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { assert!(auto.contains("/Helv 14.00 Tf") && auto.contains(" 0 g"), "{auto}"); } +#[test] +fn a_button_caption_its_fonts_encoding_cant_hold_is_reported() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [7 0 R] >>".into(), + "<< /Fields [7 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R /CN 6 0 R >> >> >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Font /Subtype /Type0 /BaseFont /STSong-Light /Encoding /UniGB-UCS2-H /DescendantFonts [<< /Type /Font /Subtype /CIDFontType0 /BaseFont /STSong-Light /CIDSystemInfo << /Registry (Adobe) /Ordering (GB1) /Supplement 4 >> >>] >>".into(), + // A UCS-2 CMap holds 签 but not a character beyond U+FFFF, which is drawn as `?`. + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (go) /Ff 65536 /DA (/CN 9 Tf 0 g) /MK << /CA > >> /Rect [50 700 140 711] /P 3 0 R >>".into(), + ]; + let mut doc = document(&objs); + redraw_field(&mut doc, "go").unwrap(); + assert!(undrawn_fields(&doc).is_empty(), "签 is drawn"); + set_props(&mut doc, "go", &FieldProps { caption: Some((0, "签\u{2000B}".into())), ..Default::default() }).unwrap(); + assert!(ap(&doc, &field(&fields(&doc), "go").widgets[0]).contains("/CN 9.00 Tf")); + assert_eq!(undrawn_fields(&doc), [("go".to_string(), '\u{2000B}')]); +} + #[test] fn button_caption_preserves_other_widgets_and_shared_dictionaries() { for indirect_caption in [false, true] { From 280f2439ec34dd82f6feb8416a2fc0898c14261f Mon Sep 17 00:00:00 2001 From: vangel76 Date: Wed, 14 Oct 2026 23:03:14 +0200 Subject: [PATCH 17/34] Preferences: field highlights on by default, with a toggle in Appearance (#2273) New settings start with Highlight fields on. A stored choice is kept; a missing or malformed value falls back to on. Preferences > Appearance gets the checkbox. Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/appearance.rs | 9 ++++++++ crates/ui-egui/src/canvas.rs | 4 ++-- crates/ui-egui/tests/view.rs | 38 +++++++++++++++++++++++++------- parity/acrobat-features.toml | 4 ++-- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/appearance.rs b/crates/ui-egui/src/appearance.rs index 0d607cc910..536d8e4d97 100644 --- a/crates/ui-egui/src/appearance.rs +++ b/crates/ui-egui/src/appearance.rs @@ -139,6 +139,9 @@ pub fn preferences_section(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) app.execute(mode_command(ThemePreference::Dark)); } }); + ui.add_space(4.0); + // Acrobat's "Highlight existing fields": shared with the toggle in the form bar. + ui.add(crate::widgets::checkbox(&mut app.view_defaults.highlight_fields, tl!("Highlight fields"))); } #[cfg(test)] @@ -176,6 +179,12 @@ mod tests { h.render().unwrap().save(path).unwrap(); } + // The field-highlight preference is on for new settings, and the box turns it off. + assert!(h.state().view_defaults.highlight_fields); + h.get_by_label("Highlight fields").click(); + h.run(); + assert!(!h.state().view_defaults.highlight_fields); + let combo = egui::accesskit::Role::ComboBox; h.get_by_role(combo).click(); h.run(); diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eb9244a284..bd408f4017 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -174,14 +174,14 @@ pub struct ViewDefaults { /// The zoom with [`Fit::None`] (1.0 = 100%). pub zoom: f32, /// Highlight existing fields: a preference in Acrobat, so it carries over to the next - /// document and the next session. + /// document and the next session. On for new settings; a stored choice wins. pub highlight_fields: bool, } impl Default for ViewDefaults { fn default() -> Self { // Single page, scrolling continuously at fit width: the earlier "continuous" default. - Self { layout: PageLayout::Single, continuous: true, fit: Fit::Width, zoom: 1.0, highlight_fields: false } + Self { layout: PageLayout::Single, continuous: true, fit: Fit::Width, zoom: 1.0, highlight_fields: true } } } diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 9cf99021a3..713702b139 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -1059,6 +1059,8 @@ fn highlight_fields_tints_the_field_area() { let _gpu = gpu(); let mut h = form_harness(); h.state_mut().set_option("panel", "none").unwrap(); + // Highlighting is on by default; start from plain fields. + h.state_mut().set_option("fields", "off").unwrap(); for _ in 0..100 { h.run_steps(2); if !h.state().render_pending() { @@ -1136,27 +1138,47 @@ fn required_radio_buttons_get_a_round_red_border() { assert!(!stroke(at(50.3, 339.7)), "the widget's corner, outside the circle, is not: {:?}", at(50.3, 339.7)); } -/// Acrobat's Highlight existing fields is a preference, not a per-document choice: turned on -/// once, the next document (and the next session) opens with fields highlighted. +/// Acrobat's Highlight existing fields is a preference, not a per-document choice: turned off +/// once, the next document (and the next session) opens with fields plain. New settings start +/// with it on. #[test] -fn field_highlighting_is_remembered() { +fn field_highlighting_is_on_by_default_and_remembered() { use egui_kittest::kittest::Queryable; let mut h = form_harness(); h.state_mut().set_option("panel", "none").unwrap(); h.run_steps(4); - assert!(!h.state().views[0].highlight_fields, "off by default"); - h.get_by_label("Highlight fields").click(); + assert!(h.state().views[0].highlight_fields, "on by default"); + h.get_by_label("Hide field highlights").click(); h.run_steps(4); - assert!(h.state().views[0].highlight_fields); + assert!(!h.state().views[0].highlight_fields); h.state_mut().open_bytes("radios.pdf", None, RADIOS.to_vec()).expect("opens"); h.run_steps(4); - assert!(h.state().views[1].highlight_fields, "the next document opens highlighted"); + assert!(!h.state().views[1].highlight_fields, "the next document opens plain"); let saved = h.state().persist(); let mut next = PdfCraftApp::new(); next.set_option("language", "en").unwrap(); next.restore(&saved); next.open_bytes("form.pdf", None, FORM.to_vec()).expect("opens"); - assert!(next.views[0].highlight_fields, "and so does the next session"); + assert!(!next.views[0].highlight_fields, "and so does the next session"); +} + +/// Settings saved before the default changed keep their stored choice; a missing or malformed +/// value falls back to the default instead of failing. +#[test] +fn stored_field_highlight_choice_wins_over_the_default() { + for (stored, expected) in [ + (r#"{"highlight_fields": false}"#, false), + (r#"{"highlight_fields": true}"#, true), + (r#"{}"#, true), + (r#"{"highlight_fields": "no"}"#, true), + (r#"{"highlight_fields": null}"#, true), + ] { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.restore(stored); + app.open_bytes("form.pdf", None, FORM.to_vec()).expect("opens"); + assert_eq!(app.views[0].highlight_fields, expected, "{stored}"); + } } #[test] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..7d95c7098f 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4392,8 +4392,8 @@ milestone = "M3" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/view.rs::highlight_fields_tints_the_field_area", "crates/ui-egui/tests/view.rs::required_radio_buttons_get_a_round_red_border", "crates/ui-egui/tests/view.rs::field_highlighting_is_remembered", "crates/ui-egui/tests/editing.rs::a_secured_form_shows_its_form_notice_and_its_security"] -notes = "offered on secured forms too; remembered for the next document and session; required fields get a red border (round for radio buttons); the tint is drawn over the field, where Acrobat paints the highlight colour behind it" +evidence = ["crates/ui-egui/tests/view.rs::highlight_fields_tints_the_field_area", "crates/ui-egui/tests/view.rs::required_radio_buttons_get_a_round_red_border", "crates/ui-egui/tests/view.rs::field_highlighting_is_on_by_default_and_remembered", "crates/ui-egui/tests/view.rs::stored_field_highlight_choice_wins_over_the_default", "crates/ui-egui/tests/editing.rs::a_secured_form_shows_its_form_notice_and_its_security"] +notes = "offered on secured forms too; on by default for new settings (a stored choice is kept) and remembered for the next document and session; toggled from the form bar and from Preferences > Appearance, where it sets the default for documents opened afterwards; required fields get a red border (round for radio buttons); the tint is drawn over the field, where Acrobat paints the highlight colour behind it" [[feature]] id = "form.field-appearance-display" From f295be1e057a84d9273a523ce78b86aabed034c6 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Thu, 15 Oct 2026 23:29:41 +0200 Subject: [PATCH 18/34] Tests: field highlights are on by default, so the toggle reads "Hide field highlights" (#2273) Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/tests/ui_group/animate.rs | 3 ++- crates/ui-egui/tests/ui_group/xfa.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/tests/ui_group/animate.rs b/crates/ui-egui/tests/ui_group/animate.rs index 20008edd60..d01bf6de19 100644 --- a/crates/ui-egui/tests/ui_group/animate.rs +++ b/crates/ui-egui/tests/ui_group/animate.rs @@ -67,6 +67,7 @@ fn push_buttons_alone_show_no_form_bar_and_a_screen_annotation_is_not_a_comment( } assert!(h.query_by_label_contains("interactive form fields").is_none(), "nothing to fill in, so no form bar"); assert!(h.query_by_label("Highlight fields").is_none()); + assert!(h.query_by_label("Hide field highlights").is_none()); } #[test] @@ -75,5 +76,5 @@ fn a_fillable_field_beside_push_buttons_still_shows_the_form_bar() { let text = "<< /Type /Annot /Subtype /Widget /FT /Tx /T (name) /Rect [20 160 200 180] /P 3 0 R >>"; let h = open(pdf(&[&button, text, SCREEN])); h.get_by_label_contains("This document contains 2 interactive form fields."); - h.get_by_label("Highlight fields"); + h.get_by_label("Hide field highlights"); } diff --git a/crates/ui-egui/tests/ui_group/xfa.rs b/crates/ui-egui/tests/ui_group/xfa.rs index 6565a9816f..ee44ec8230 100644 --- a/crates/ui-egui/tests/ui_group/xfa.rs +++ b/crates/ui-egui/tests/ui_group/xfa.rs @@ -58,8 +58,8 @@ fn a_dynamic_xfa_form_is_laid_out_and_filled() { let doc = s.session.get(s.views[0].id).unwrap(); assert_eq!(doc.info.pages.len(), 2); assert!(doc.form.iter().any(|f| f.name == "familyName")); - // The notice offers to highlight the fields, like any form. - h.get_by_label("Highlight fields"); + // The notice offers the highlight toggle, like any form; highlights are on by default. + h.get_by_label("Hide field highlights"); } #[test] @@ -87,8 +87,8 @@ fn a_static_xfa_form_is_filled_from_its_data_and_says_so() { assert_eq!(doc.form.iter().find(|f| f.name == "form1[0].page1[0].name[0]").unwrap().value, vec!["Ada".to_string()]); assert_eq!(doc.form.iter().find(|f| f.name == "form1[0].page1[0].agree[0]").unwrap().value, vec!["1".to_string()]); assert!(!doc.dirty); - // The fields can still be highlighted from the notice. - h.get_by_label("Highlight fields"); + // The highlight toggle (on by default, so it reads "Hide") is still in the notice. + h.get_by_label("Hide field highlights"); } #[test] From 7504799e059c579d3def2a9b6076f57c4cd405dd Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 20 Oct 2026 01:11:01 +0200 Subject: [PATCH 19/34] M6: align secured-form highlight tests after rebase (#2273) --- ROADMAP.md | 2 ++ crates/ui-egui/tests/editing.rs | 2 +- crates/ui-egui/tests/ui_group/xfa.rs | 2 +- docs/roadmap.md | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..7b6650e829 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -172,6 +172,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-20 (M6, #2273 / PR #2279):** Rebased the field-highlight default and Appearance preference onto main, retaining secured-form parity evidence and updating secured AcroForm/XFA toggle expectations. Build and test validation delegated to GitHub CI. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index d41041e52e..a72ac29bef 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -2165,7 +2165,7 @@ fn a_secured_form_shows_its_form_notice_and_its_security() { }); h.run_steps(4); h.get_by_label_contains("interactive form fields. This document is secured. Some changes are restricted by its security settings."); - h.get_by_label("Highlight fields"); + h.get_by_label("Hide field highlights"); h.get_by_label("Security settings"); h.get_by_label("Unlock…"); } diff --git a/crates/ui-egui/tests/ui_group/xfa.rs b/crates/ui-egui/tests/ui_group/xfa.rs index ee44ec8230..e66658b0e1 100644 --- a/crates/ui-egui/tests/ui_group/xfa.rs +++ b/crates/ui-egui/tests/ui_group/xfa.rs @@ -228,6 +228,6 @@ fn a_secured_dynamic_xfa_form_shows_its_layout_and_its_security() { .unwrap(); let h = open(pdfcraft_cos::write_full(&doc, &Default::default()).unwrap()); h.get_by_label_contains("laid out from its template: 2 pages, 11 fields. This document is secured."); - h.get_by_label("Highlight fields"); + h.get_by_label("Hide field highlights"); h.get_by_label("Security settings"); } diff --git a/docs/roadmap.md b/docs/roadmap.md index 5d73511e9c..b0513ce402 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -133,7 +133,7 @@ total matches the parity estimate. | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Missing: page transitions, recovery on the web M4: Insert-from-file preserves source bookmarks; integration tests retained with current main. | | M5 | Comments (all annotation types, XFDF) | 120–200 | 88% | 12–25 | Done: notes, highlight/underline/strikeout/squiggly, text boxes, ink, lines, arrows, rectangles, ovals, stamps (dynamic, Sign Here, Standard Business), with appearance streams; replies, status, checkmarks, locking, move/resize/restyle/delete; properties dialog; panel filter and sort; hide all; summaries (comments only); XFDF/FDF import and export (partial imports preserve existing reply and review-status links); flatten; Fill & Sign with adjustable text/date size; agent tools.
Done since: stamps placed on a rotated page read upright (#1398); polygons, connected lines, clouds, callouts, inserted text, colour and checkmark filters; image signature pointer previews, selection handles and live image movement/resizing with proportional corners and independent edge handles. Missing: custom stamps, replace-text proposals, summary layouts with the page | | M5 | Comments (all annotation types, XFDF) | 120–200 | 88% | 12–25 | Done: notes, highlight/underline/strikeout/squiggly, text boxes, ink, lines, arrows, rectangles, ovals, stamps (dynamic, Sign Here, Standard Business), with appearance streams; replies, status, checkmarks, locking, move/resize/restyle/delete; properties dialog; panel filter and sort; hide all; summaries (comments only); XFDF/FDF import and export (partial imports preserve existing reply and review-status links; nested FDF export uses lazy array owners and annotation positions, with retained entries budgeted before cloning under copied-graph limits and source `/Page` and `/Pages` dictionaries replaced by null at every copy depth); flatten; Fill & Sign with adjustable text/date size; agent tools.
Done since: polygons, connected lines, clouds, callouts, inserted text, colour and checkmark filters; image signature pointer previews, selection handles and live image movement/resizing with proportional corners and independent edge handles. A refused restyle or resize of a text box another program drew leaves that comment unchanged (#1327). Make Current Properties Default on an arrow updates arrows, not plain lines (#1302). Flattening keeps a visible comment it cannot draw (#1317). Missing: custom stamps, replace-text proposals, summary layouts with the page | -| M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Done since: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. (#2892). Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | +| M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Done since: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. (#2892). Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. PR #2279 refreshed on main: field highlighting defaults on, stored choices are retained; secured-form toggle expectations updated. CI validation pending; estimates unchanged. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | From 39d0b953ca83c00aab3a75640b215be5097ee3f3 Mon Sep 17 00:00:00 2001 From: djmushin <31074036+djmushin@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:20:51 -0400 Subject: [PATCH 20/34] M6: treat imported MaxLen zero as unlimited --- crates/forms/README.md | 2 + crates/forms/src/lib.rs | 5 +- crates/forms/src/tests.rs | 16 ++++ docs/form-maxlen-compatibility.md | 117 ++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 docs/form-maxlen-compatibility.md diff --git a/crates/forms/README.md b/crates/forms/README.md index 66fb919fe7..559219527e 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -44,6 +44,8 @@ delete_field(&mut doc, "email")?; flattening refuses to make that `?` permanent. - Check boxes and radio buttons switch `/V` and each widget's `/AS`. Widgets with no appearances get PdfCraft's own (a drawn check mark or dot; no symbol font needed). +- Imported `/MaxLen 0` is treated as unlimited (also clearing an inherited limit), while the + original entry is preserved. Positive limits still apply. - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a failed call changes nothing. - `FieldProps::appearance` applies a `LookPatch` independently to each widget. `None` keeps diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 1dfa767262..18bef8fb80 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -754,7 +754,10 @@ fn walk( inh.q = Some(q); } if let Some(m) = d.get(b"MaxLen").and_then(|o| doc.resolve(o).as_int()) { - inh.max_len = usize::try_from(m).ok(); + // Compatibility: some writers emit zero for an unlimited text field. Keep /MaxLen + // untouched in the object graph, but expose only positive limits to both UI and API. + // An explicit zero also clears an inherited limit rather than inheriting it again. + inh.max_len = usize::try_from(m).ok().filter(|limit| *limit > 0); } let kids: Vec = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd8..d5b82341f8 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2239,6 +2239,22 @@ fn page_shapes_closes_only_last_stroked_subpath_but_all_filled_subpaths() { assert!(filled.rules.is_empty()); } +#[test] +fn zero_max_len_is_unlimited_and_overrides_inherited_limit() { + let mut doc = fixture(); + let city = fields(&doc).into_iter().find(|f| f.name == "address.city").unwrap(); + let parent = doc.get(city.obj).as_dict().unwrap().get(b"Parent").unwrap().as_ref().unwrap(); + doc.update_dict(parent, |d| d.set(b"MaxLen".to_vec(), Object::Int(3))).unwrap(); + assert_eq!(fields(&doc).into_iter().find(|f| f.name == "address.city").unwrap().max_len, Some(3)); + doc.update_dict(city.obj, |d| d.set(b"MaxLen".to_vec(), Object::Int(0))).unwrap(); + assert_eq!(fields(&doc).into_iter().find(|f| f.name == "address.city").unwrap().max_len, None); + set_value(&mut doc, "address.city", &FieldValue::Text("Absalom".into())).unwrap(); + let again = reopen(&doc); + assert_eq!(field(&fields(&again), "address.city").value, ["Absalom"]); + assert_eq!(again.get(city.obj).as_dict().unwrap().int(b"MaxLen"), Some(0), "preserve the original entry"); + assert!(set_value(&mut doc, "zip", &FieldValue::Text("123456".into())).is_err(), "positive limits still apply"); +} + #[test] fn auto_text_size_fits_single_and_multiline_text() { use crate::appearance::auto_text_size; diff --git a/docs/form-maxlen-compatibility.md b/docs/form-maxlen-compatibility.md new file mode 100644 index 0000000000..adcc02ce6d --- /dev/null +++ b/docs/form-maxlen-compatibility.md @@ -0,0 +1,117 @@ +# AcroForm `/MaxLen 0` compatibility + +Status: implemented; proposed as a separate compatibility PR. Last updated: 2026-10-19. + +- `crates/forms/src/lib.rs::walk` reads inherited `/MaxLen`; the UI and value validation both + consume `Field::max_len`. + +The parser exposed `/MaxLen 0` as `Some(0)`, so both editor and API blocked input. It now +exposes only positive limits; zero clears an inherited limit. The original object entry is +preserved through save/reopen. This is an interoperability interpretation for zero, **not +a claim that ISO explicitly defines zero as unlimited**. ISO 32000-1 Table 229 defines an +optional inheritable maximum length without defining a special zero meaning: +[public specification](https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf). +Positive limits remain enforced; no field names or character-sheet rules enter product code. + +Regression: `crates/forms/src/tests.rs::zero_max_len_is_unlimited_and_overrides_inherited_limit`. +The forms README and `form.fill-text` parity entry document this behavior. + +Historical validation recorded before this review: + +- Forms unit suite: all 68 tests passed. The new zero-limit regression was first run against + the old parser and failed (`Some(0)` instead of `None`), then passed with the compatibility + interpretation. Its positive-limit control and preserved raw entry also passed. + +The user reports successful Class text input on the integrated release using their Pathfinder +character sheet. This is a manual functional result, without timings; the private document +was not used for automated regression tests in this review. + +## Independent branch validation + +Validated on upstream `a12fe8f4116984ebbc870eb514346ea922233316` with an isolated +Cargo target directory and no performance changes: + +- The focused regression failed with the original parser (`Some(0)` rather than `None`), + then passed after restoring the compatibility fix. +- `cargo test -p pdfcraft-forms -j 2`: 81 unit tests passed; doc tests passed. +- `cargo clippy -p pdfcraft-forms -j 2 --all-targets -- -D warnings`: passed. +- `cargo check --target wasm32-unknown-unknown -p pdfcraft-forms -j 2`: passed. +- `cargo fmt --check`, `git diff --check`, and `cargo xtask layers`, `assets`, `parity`: passed. + +These are local checks, not hosted CI. No private PDF was used. + +## Rebase validation + +Rebased onto freshly fetched upstream main +`932bf9c6bfdd63e2a21bb831696a3447b9def9cf` on 2026-10-18. The only conflict was +in the M6 roadmap row; upstream CSV/XML updates were retained alongside the compatibility +note. The implementation and original regression are unchanged from the independent branch. + +Validation used a fresh Cargo target directory, two build jobs, and locked, offline builds: + +- The original regression failed with the upstream parser (`Some(0)` instead of `None`), + then passed with the compatibility fix restored. +- Forms: 83 unit tests passed. Engine: 282 unit and 9 integration tests passed. + JavaScript: 47 unit tests passed. Doc tests passed. The timing probe and two private-file + tests remained ignored. +- The automation `javascript_through_tools` regression passed. +- Scoped Clippy for forms, engine and JavaScript, with `--all-targets -- -D warnings`, passed. +- WASM checks for those three crates passed. +- Rust formatting, whitespace, layering, assets and parity checks passed. +- A freshly built CLI filled synthetic fields with direct and inherited `/MaxLen 0`, rendered + their values, saved/reopened them and accepted longer text after reopening. Its positive-limit + control rejected six characters for `/MaxLen 5`; the synthetic input file stayed unchanged. + +No performance optimizations, JavaScript snapshot/diff changes or profiling instrumentation +are included. Existing executables, backups and private PDFs were not modified. + +## PR #3323 refresh + +Revalidated against freshly fetched upstream main +`4db6ac4ae2448df6389d8bb0bc54db6ba175721c` on 2026-10-18. Conflicts in both +roadmaps were resolved by keeping upstream's button-label and content-editing notes alongside +the `/MaxLen 0` documentation. The original compatibility implementation and regression are +preserved; no performance changes were added to the PR. + +Forms: 85 unit tests passed. Engine: 283 unit and 15 integration tests passed. JavaScript: +48 unit tests passed. The automation JavaScript regression, scoped Clippy with warnings denied, +WASM, formatting, whitespace, layering, assets and parity checks all passed. The synthetic CLI +fill/render/save/reopen and positive-limit control passed. The original regression again failed +with the old parser and passed with the fix restored. Private-file tests remained ignored. + +## 2026-10-19 revalidation + +Rebased onto freshly fetched upstream main +`459379ef9b127648cc30c0b2fe2b20c2b2375f04`. Only `ROADMAP.md` required manual +conflict resolution; all upstream entries and the compatibility note were retained. +The implementation, original regression and forms README remain byte-for-byte identical +to the previously reviewed compatibility head. No performance changes were introduced. + +Validation used a fresh isolated build directory, two jobs, and locked offline builds: + +- The original regression failed with the old parser and passed with the fix restored. +- Forms: 93 unit tests passed. Engine: 283 unit and 15 integration tests passed. + JavaScript: 48 tests passed. Doc tests passed; the timing probe and two private-file + tests remained ignored. +- Automation JavaScript, scoped all-targets Clippy with warnings denied, WASM, formatting, + whitespace, layering, assets and parity passed. +- Synthetic CLI fill/render/save/reopen checks passed for direct zero and zero overriding + an inherited positive limit. Longer text after reopening was accepted. The `/MaxLen 5` + control rejected six characters, and the synthetic input stayed unchanged. + +These are local checks, not hosted CI. Existing executables, backups and private PDFs +were not modified; only the compatibility worktree was changed. + +## Revision history + +- 2026-10-19: refresh PR #3323 onto upstream `459379ef`, retain all upstream roadmap entries + and independently repeat regression, native/WASM, repository and CLI validation. + +- 2026-10-18: refresh PR #3323 onto upstream `4db6ac4a`, retaining both roadmap updates and + independently revalidating compatibility and repository checks. + +- 2026-10-18: rebase onto fetched upstream `932bf9c6`; preserve upstream roadmap updates and + independently revalidate the regression, forms/engine/JavaScript suites and quality gates. + +- 2026-10-18: isolate the compatibility change on current upstream main; verify the + regression before and after the fix, the forms suite and scoped quality gates. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..e06a5c7f1d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4416,8 +4416,8 @@ milestone = "M6" status = "shipped" commands = ["form.fields"] tools = ["form_fill", "form_fields"] -evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/ui-egui/tests/forms.rs::auto_size_fields_shrink_and_wrap_typed_text_to_fit_their_box", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes", "crates/automation/tests/automation.rs::orphan_form_fields_through_tools", "crates/forms/src/tests.rs::fields_sharing_a_name_take_the_same_value"] -notes = "in-place editor; Enter or clicking away commits, Escape cancels; auto-size (0 Tf) fields shrink, and multiline ones wrap, the text being typed to fit the box, on screen as on the page (the screen measures in the interface font, so its size can be a little smaller than the page's Helvetica fit); fixed sizes are kept and clip, as in Acrobat; no format/keystroke JavaScript yet (M6.4)" +evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/ui-egui/tests/forms.rs::auto_size_fields_shrink_and_wrap_typed_text_to_fit_their_box", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes", "crates/automation/tests/automation.rs::orphan_form_fields_through_tools", "crates/forms/src/tests.rs::fields_sharing_a_name_take_the_same_value", "crates/forms/src/tests.rs::zero_max_len_is_unlimited_and_overrides_inherited_limit"] +notes = "in-place editor; Enter or clicking away commits, Escape cancels; auto-size (0 Tf) fields shrink, and multiline ones wrap, the text being typed to fit the box, on screen as on the page (the screen measures in the interface font, so its size can be a little smaller than the page's Helvetica fit); fixed sizes are kept and clip, as in Acrobat; no format/keystroke JavaScript yet (M6.4); imported MaxLen 0 permits text input and overrides inherited limits; positive limits remain enforced." [[feature]] id = "form.fill-checkbox-radio" From 0d0cce1bb03b1f49c40169540e079856c8e74c82 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:13:05 -0400 Subject: [PATCH 21/34] Forms: Highlight fields marks only the fields that can be filled --- crates/ui-egui/src/canvas.rs | 5 +++-- crates/ui-egui/tests/view.rs | 29 +++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 ++-- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eb9244a284..8980a5b7e1 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3461,9 +3461,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } // Form-field highlight (Acrobat's "Highlight existing fields"); required fields get a - // red border. Radio buttons are round, and so is theirs (as in Acrobat). + // red border. Radio buttons are round, and so is theirs (as in Acrobat). Only fields + // that can be filled are marked: a read-only one is left as it is drawn. if view.highlight_fields { - for f in form.iter() { + for f in form.iter().filter(|f| !f.has(pdfcraft_engine::field_flags::READ_ONLY)) { let required = f.has(pdfcraft_engine::field_flags::REQUIRED); let round = f.kind == pdfcraft_engine::FormFieldKind::Radio; for w in f.widgets.iter().filter(|w| w.page == Some(i) && !w.hidden) { diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 9cf99021a3..12a740f77a 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -1083,6 +1083,35 @@ fn highlight_fields_tints_the_field_area() { assert_ne!(before, after, "the field area is tinted when highlighting is on"); } +/// A text field that can be filled and one that is read-only (`/Ff` 1). +const LOCKED: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 0 R] >> >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R] >> endobj +4 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (open) /Rect [50 300 250 330] /P 3 0 R >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (locked) /Ff 1 /Rect [50 200 250 230] /P 3 0 R >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// Highlight fields marks the fields that can be filled: a read-only one (a form's locked +/// box, or one its scripts have not opened yet) is left as it is drawn, as in Acrobat. +#[test] +fn highlight_fields_leaves_read_only_fields_alone() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("locked.pdf", None, LOCKED.to_vec()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app.set_option("panel", "none").unwrap(); + app.set_option("fields", "on").unwrap(); + app + }); + h.run_steps(6); + let tint = egui::Color32::from_rgba_unmultiplied(0x6E, 0x8E, 0xF5, 48); + let tinted = h.output().shapes.iter().filter(|c| matches!(&c.shape, egui::Shape::Rect(r) if r.fill == tint)).count(); + assert_eq!(tinted, 1, "only the field that can be filled is tinted"); +} + /// A required radio group (`/Ff` 32768 radio + 2 required) with two 40 pt round buttons. const RADIOS: &[u8] = b"%PDF-1.7 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >> endobj diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..470be4e464 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4392,8 +4392,8 @@ milestone = "M3" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/view.rs::highlight_fields_tints_the_field_area", "crates/ui-egui/tests/view.rs::required_radio_buttons_get_a_round_red_border", "crates/ui-egui/tests/view.rs::field_highlighting_is_remembered", "crates/ui-egui/tests/editing.rs::a_secured_form_shows_its_form_notice_and_its_security"] -notes = "offered on secured forms too; remembered for the next document and session; required fields get a red border (round for radio buttons); the tint is drawn over the field, where Acrobat paints the highlight colour behind it" +evidence = ["crates/ui-egui/tests/view.rs::highlight_fields_tints_the_field_area", "crates/ui-egui/tests/view.rs::required_radio_buttons_get_a_round_red_border", "crates/ui-egui/tests/view.rs::field_highlighting_is_remembered", "crates/ui-egui/tests/view.rs::highlight_fields_leaves_read_only_fields_alone", "crates/ui-egui/tests/editing.rs::a_secured_form_shows_its_form_notice_and_its_security"] +notes = "only fields that can be filled are marked (a read-only field is left as drawn, so a form's locked boxes don't look fillable); offered on secured forms too; remembered for the next document and session; required fields get a red border (round for radio buttons); the tint is drawn over the field, where Acrobat paints the highlight colour behind it" [[feature]] id = "form.field-appearance-display" From 465bd0a71737b8d8101cb9bca34c1040317ea9a6 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:41:47 -0400 Subject: [PATCH 22/34] M5: doc comments for the functions this change touches --- crates/forms/src/author.rs | 3 +++ crates/forms/src/tests.rs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index 40c94bec08..3f05aec3ea 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -683,6 +683,9 @@ pub fn redraw_field(doc: &mut Document, name: &str) -> Result<(), FormError> { redraw_widgets(doc, name, None) } +/// `redraw_field` for the widget at index `only` alone (so a caption-only edit leaves the field's +/// other widgets' appearances as they are), or for every widget when `only` is `None`. Fails with +/// `FormError::NoSuchField` when no field is named `name`. fn redraw_widgets(doc: &mut Document, name: &str, only: Option) -> Result<(), FormError> { let f = fields(doc).into_iter().find(|f| f.name == name).ok_or_else(|| FormError::NoSuchField(name.into()))?; let mut undrawn = false; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 8bc81851ab..40d44677ff 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -87,6 +87,8 @@ fn ap(doc: &Document, w: &Widget) -> String { String::from_utf8_lossy(&s.decoded().unwrap()).into_owned() } +/// A push button's caption is drawn in the font, size and color its `/DA` asks for; with an +/// automatic size (0) it is sized to fit the button, in the form's default font and color. #[test] fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { let objs: Vec = vec![ @@ -111,6 +113,9 @@ fn a_button_caption_is_drawn_in_its_fields_font_size_and_color() { assert!(auto.contains("/Helv 14.00 Tf") && auto.contains(" 0 g"), "{auto}"); } +/// A button caption in a font whose UCS-2 CMap holds 签 is not reported; once `set_props` adds a +/// character beyond U+FFFF, the caption is still drawn in that font and `undrawn_fields` reports +/// the character. #[test] fn a_button_caption_its_fonts_encoding_cant_hold_is_reported() { let objs: Vec = vec![ From 77eb328c834fb68ff7e6208bcfa496450142145f Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:42:37 -0400 Subject: [PATCH 23/34] Forms: doc comments for the functions this change touches --- crates/ui-egui/src/canvas.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 8980a5b7e1..eaaf016246 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2365,6 +2365,8 @@ fn page_scroll( ui.scope_builder(egui::UiBuilder::new().id(id), |ui| scroll.show_viewport(ui, draw)).inner } +/// Draws the document tab at `index`: its notice bars, then its pages (or the Organize grid) in a +/// scroll area with this frame's zoom, scrolling, tool input, overlays and render requests. pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); // Keyboard scrolling applies to this frame's page view only, never later. From 0a77db066c4aaabf1fb4a113777f3500169fad95 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:20:02 -0400 Subject: [PATCH 24/34] Forms: a radio group's read-only button is left unhighlighted on its own --- crates/forms/src/author.rs | 1 + crates/forms/src/lib.rs | 5 +++++ crates/ui-egui/src/canvas.rs | 9 +++++---- crates/ui-egui/tests/view.rs | 31 +++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index 521d1f5278..83a3fb9fda 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -659,6 +659,7 @@ pub fn add_field(doc: &mut Document, page: usize, rect: [f64; 4], kind: &NewFiel tab: usize::MAX, locked: false, hidden: false, + read_only: false, rotation: 0, caption: None, list: None, diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 1dfa767262..8ea7b2744e 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -101,6 +101,9 @@ pub struct Widget { pub locked: bool, /// The widget's Hidden or NoView flag (`/F` bit 2 or 6): it isn't shown and takes no input. pub hidden: bool, + /// Read-only: the field is, or the widget's own `/Ff` says so (a radio button of a group can + /// be locked on its own, as an XFA exclusion group's layout writes it). + pub read_only: bool, /// `/MK /R` as 0, 90, 180 or 270 degrees counterclockwise. Anything else is stored as 0. pub rotation: i64, /// Push buttons: the normal label (`/MK /CA`), separate from the field's name. @@ -788,6 +791,7 @@ fn walk( let wd = wo.as_dict()?; let rect = nums(doc, wd.get(b"Rect")).filter(|r| r.len() == 4).unwrap_or_else(|| vec![0.0; 4]); let annot_flags = wd.get(b"F").and_then(|f| doc.resolve(f).as_int()).unwrap_or(0); + let own_ff = if w == r { 0 } else { field_flags(doc, wd).unwrap_or(0) }; let on_state = wd .get(b"AP") .map(|ap| doc.resolve(ap)) @@ -803,6 +807,7 @@ fn walk( tab: usize::MAX, locked: annot_flags & 128 != 0, hidden: annot_flags & (2 | 32) != 0, + read_only: (ff | own_ff) & flags::READ_ONLY != 0, rotation: appearance::mk_rotation(doc, wd), caption: (kind == FieldKind::PushButton) .then(|| { diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eaaf016246..be1f284833 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3463,13 +3463,14 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } // Form-field highlight (Acrobat's "Highlight existing fields"); required fields get a - // red border. Radio buttons are round, and so is theirs (as in Acrobat). Only fields - // that can be filled are marked: a read-only one is left as it is drawn. + // red border. Radio buttons are round, and so is theirs (as in Acrobat). Only what can + // be filled is marked: a read-only widget (its field's, or a radio button's own) is + // left as it is drawn. if view.highlight_fields { - for f in form.iter().filter(|f| !f.has(pdfcraft_engine::field_flags::READ_ONLY)) { + for f in form.iter() { let required = f.has(pdfcraft_engine::field_flags::REQUIRED); let round = f.kind == pdfcraft_engine::FormFieldKind::Radio; - for w in f.widgets.iter().filter(|w| w.page == Some(i) && !w.hidden) { + for w in f.widgets.iter().filter(|w| w.page == Some(i) && !w.hidden && !w.read_only) { let r = w.rect; let sr = xf.user_rect(info, i, [r[0] as f32, r[1] as f32, r[2] as f32, r[3] as f32]); let tint = Color32::from_rgba_unmultiplied(0x6E, 0x8E, 0xF5, 48); diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 12a740f77a..c3f15c4a42 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -1112,6 +1112,37 @@ fn highlight_fields_leaves_read_only_fields_alone() { assert_eq!(tinted, 1, "only the field that can be filled is tinted"); } +/// A radio group whose second button is locked on its own widget (`/Ff` 1 there), as the +/// layout of an XFA exclusion group with one read-only button writes it. +const LOCKED_RADIO: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >> endobj +4 0 obj << /FT /Btn /Ff 49152 /T (choice) /V /Off /Kids [5 0 R 6 0 R] >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Rect [50 300 90 340] /AS /Off /P 3 0 R >> endobj +6 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Ff 1 /Rect [150 300 190 340] /AS /Off /P 3 0 R >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// A read-only button of a radio group is left untinted while the group's other button, which +/// can be chosen, is tinted. +#[test] +fn highlight_fields_leaves_a_groups_read_only_button_alone() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("radio.pdf", None, LOCKED_RADIO.to_vec()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app.set_option("panel", "none").unwrap(); + app.set_option("fields", "on").unwrap(); + app + }); + h.run_steps(6); + let tint = egui::Color32::from_rgba_unmultiplied(0x6E, 0x8E, 0xF5, 48); + let tinted = h.output().shapes.iter().filter(|c| matches!(&c.shape, egui::Shape::Circle(r) if r.fill == tint)).count(); + assert_eq!(tinted, 1, "only the button that can be chosen is tinted"); +} + /// A required radio group (`/Ff` 32768 radio + 2 required) with two 40 pt round buttons. const RADIOS: &[u8] = b"%PDF-1.7 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >> endobj From 56b316a7f4c5c53c4bd0546428251737cdc6bb50 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:35:54 -0400 Subject: [PATCH 25/34] Forms: a radio group's read-only button can't be chosen by a click --- crates/forms/src/lib.rs | 2 ++ crates/ui-egui/src/forms_ui.rs | 5 +++-- crates/ui-egui/tests/view.rs | 38 ++++++++++++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 8ea7b2744e..627e16c124 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -721,6 +721,8 @@ pub fn set_tab_order(doc: &mut Document, pages: &[usize], order: TabOrder) -> Re Ok(()) } +/// Read the field at `r` and the fields under it into `out`, with what they inherit from +/// `parent`: kids with a name are fields of their own, kids without are this field's widgets. fn walk( doc: &Document, r: ObjRef, diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index 612b985e65..8269a8a55d 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -135,7 +135,8 @@ pub(crate) fn page_input( // Empty signature fields are signed by clicking them (Use a certificate); push buttons run // their action. let pressable = f.kind == FormFieldKind::PushButton && !f.read_only() && f.button.is_some(); - let usable = allowed && (fillable(f) || pressable || f.kind == FormFieldKind::Signature); + // A radio button of a group can be read-only on its own. + let usable = allowed && !w.read_only && (fillable(f) || pressable || f.kind == FormFieldKind::Signature); ui.ctx().set_cursor_icon(match (usable, f.kind) { (false, _) => egui::CursorIcon::NotAllowed, (true, FormFieldKind::Text) => egui::CursorIcon::Text, @@ -157,7 +158,7 @@ pub(crate) fn page_input( return FieldPointer::Owned; } match f.kind { - _ if f.read_only() => view.forms.notice = Some(FormNotice::ReadOnly(f.name.clone())), + _ if f.read_only() || w.read_only => view.forms.notice = Some(FormNotice::ReadOnly(f.name.clone())), FormFieldKind::PushButton => match &f.button { Some(a) => view.forms.button = Some((f.name.clone(), a.clone())), None => view.forms.notice = Some(FormNotice::NoAction(f.name.clone())), diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index c3f15c4a42..4be92ae192 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -1119,8 +1119,8 @@ const LOCKED_RADIO: &[u8] = b"%PDF-1.7 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >> endobj 3 0 obj << /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >> endobj 4 0 obj << /FT /Btn /Ff 49152 /T (choice) /V /Off /Kids [5 0 R 6 0 R] >> endobj -5 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Rect [50 300 90 340] /AS /Off /P 3 0 R >> endobj -6 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Ff 1 /Rect [150 300 190 340] /AS /Off /P 3 0 R >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Rect [50 300 90 340] /AS /Off /AP << /N << /one << >> /Off << >> >> >> /P 3 0 R >> endobj +6 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Ff 1 /Rect [150 300 190 340] /AS /Off /AP << /N << /two << >> /Off << >> >> >> /P 3 0 R >> endobj trailer << /Root 1 0 R >> %%EOF"; @@ -1143,6 +1143,40 @@ fn highlight_fields_leaves_a_groups_read_only_button_alone() { assert_eq!(tinted, 1, "only the button that can be chosen is tinted"); } +/// A radio group's read-only button can't be chosen by a click, while the group's other button +/// can. +#[test] +fn a_groups_read_only_button_is_not_chosen_by_a_click() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("radio.pdf", None, LOCKED_RADIO.to_vec()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app.set_option("panel", "none").unwrap(); + app + }); + h.run_steps(6); + let r = rect(&h, 0).expect("page 1"); + // The buttons' centers on the 300×400 pt page (y up). + let at = |x: f32, y: f32| egui::pos2(r.min.x + r.width() * (x / 300.0), r.min.y + r.height() * (1.0 - y / 400.0)); + let click = |h: &mut Harness<'static, PdfCraftApp>, pos: egui::Pos2| { + h.event(egui::Event::PointerMoved(pos)); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(4); + }; + let value = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().form.iter().find(|f| f.name == "choice").map(|f| f.value.clone()).unwrap_or_default() + }; + click(&mut h, at(70.0, 320.0)); + assert_eq!(value(&h), ["one"], "the button that can be chosen is"); + click(&mut h, at(170.0, 320.0)); + assert_eq!(value(&h), ["one"], "the read-only button leaves the choice as it was"); +} + /// A required radio group (`/Ff` 32768 radio + 2 required) with two 40 pt round buttons. const RADIOS: &[u8] = b"%PDF-1.7 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >> endobj From 25eb568839f97bc50c97297f9f474b1e3c3a2d22 Mon Sep 17 00:00:00 2001 From: Ahmad Mujahid Date: Tue, 20 Oct 2026 13:30:02 +0400 Subject: [PATCH 26/34] fix(forms): preserve content stream comment boundaries (#3502) --- ROADMAP.md | 4 ++ crates/automation/tests/automation.rs | 59 +++++++++++++++++++++++++++ crates/forms/src/detect.rs | 9 ++-- crates/forms/src/tests.rs | 32 +++++++++++++++ docs/roadmap.md | 4 +- 5 files changed, 102 insertions(+), 6 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..ad251d21fd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away +> **Last reviewed:** 2026-10-20 - **Last updated:** 2026-10-20 - **Change:** minor (M6, #3502: form detection uses the existing content-stream boundary policy; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) @@ -172,6 +173,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-20 (M6, #3502):** Form detection preserves comments across content streams and does not add fields over rectangles hidden by those comments. Synthetic tests cover visible controls, graphics state, and saved field inventories. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. @@ -552,6 +555,7 @@ Newest first: the date, what moved, and the new overall estimate. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | M6, #3502: form detection preserves comments across content streams; estimates unchanged. | | 2026-10-18 | minor | M5, PR #1971: nested source `/Page` and `/Pages` dictionaries export as null without traversing page content or the page tree. Synthetic saved/reopened three-page regression retains unrelated metadata and aliases. FDF import semantics, copied-graph limits and estimates unchanged. | | 2026-10-15 | minor | M5: budget retained FDF entries before cloning; discovery keeps lazy array owners and annotation positions, with alias/cache-eviction stress fixtures. Limits cover the copied graph; source parsing/page discovery retain existing COS/parser limits, with no total source-memory bound. FDF import semantics and estimates unchanged. | | 2026-10-15 | minor | M7: the Edit text caret is as tall as the row it is on in a region with several sizes; estimates unchanged. | diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index cbf40f1962..4068e6b8d6 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -5126,6 +5126,65 @@ fn detecting_form_fields_through_tools() { assert_eq!(f["fields"][1]["value"], "Lisbon", "{f}"); } +#[test] +fn detecting_form_fields_preserves_content_piece_comments() { + for visible in [false, true] { + let first = format!("BT /F1 18 Tf 40 250 Td (FIRST) Tj ET % trailing comment{}", if visible { "\n" } else { "" }); + let last = b"40 120 300 30 re S\nBT /F1 18 Tf 40 50 Td (LAST) Tj ET\n"; + let mut source = pdfcraft_cos::Document::open(std::sync::Arc::new(fixture(1))).unwrap(); + let page = pdfcraft_model::pages(&source)[0].obj; + let contents = [first.as_bytes(), last] + .iter() + .map(|bytes| pdfcraft_cos::Object::Ref(source.add(pdfcraft_cos::Object::Stream(pdfcraft_cos::Stream::flate(Default::default(), bytes))))) + .collect(); + source + .update_dict(page, |d| { + d.set(b"Contents".to_vec(), pdfcraft_cos::Object::Array(contents)); + d.set(b"MediaBox".to_vec(), pdfcraft_cos::Object::Array(vec![0.into(), 0.into(), 400.into(), 300.into()])); + }) + .unwrap(); + let dir = workdir(if visible { "detect-visible-piece" } else { "detect-hidden-piece" }); + std::fs::write(dir.join("pieces.pdf"), pdfcraft_cos::write_full(&source, &Default::default()).unwrap()).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path": "pieces.pdf"}))["doc"].as_u64().unwrap(); + let candidates = if visible { json!([{"page": 1, "kind": "text", "name": "Text1", "rect": [41.0, 121.0, 339.0, 149.0]}]) } else { json!([]) }; + let inventory = if visible { + json!([{"name": "Text1", "type": "text", "value": null, "page": 1, "rect": [41.0, 151.0, 339.0, 179.0], "rotation": 0, "read_only": false, "required": false, "multiline": false}]) + } else { + json!([]) + }; + let check_text = |a: &mut Automation, doc| { + let text = page_text(a, doc).join(" "); + assert_eq!(text.split_whitespace().collect::>(), ["FIRST", "LAST"]); + }; + let check_render = |a: &mut Automation, doc| { + let (width, height, pixels) = a.render_pixels(doc, 1, 1.0).unwrap(); + assert_eq!((width, height), (400, 300)); + let region: Vec<_> = (145..185).flat_map(|y| (35..345).map(move |x| (y * 400 + x) * 4)).collect(); + if visible { + assert!(region.iter().any(|&i| pixels[i..i + 3].iter().all(|&v| v < 128)), "the visible box has ink"); + } else { + assert!(region.iter().all(|&i| pixels[i..i + 3] == [255, 255, 255]), "the hidden box region stays white"); + } + }; + check_text(&mut a, doc); + check_render(&mut a, doc); + assert_eq!(ok(&mut a, "form_fields", json!({"doc": doc}))["fields"], json!([])); + assert_eq!(ok(&mut a, "form_detect_fields", json!({"doc": doc, "add": false}))["fields"], candidates); + assert_eq!(ok(&mut a, "form_fields", json!({"doc": doc}))["fields"], json!([])); + assert_eq!(ok(&mut a, "form_detect_fields", json!({"doc": doc}))["fields"], candidates); + assert_eq!(ok(&mut a, "form_fields", json!({"doc": doc}))["fields"], inventory); + assert_eq!(ok(&mut a, "form_detect_fields", json!({"doc": doc}))["fields"], json!([])); + ok(&mut a, "doc_save", json!({"doc": doc, "path": "saved.pdf"})); + let mut reopened = auto(&dir); + let again = ok(&mut reopened, "doc_open", json!({"path": "saved.pdf"}))["doc"].as_u64().unwrap(); + assert_eq!(ok(&mut reopened, "form_fields", json!({"doc": again}))["fields"], inventory); + check_text(&mut reopened, again); + check_render(&mut reopened, again); + std::fs::remove_dir_all(dir).unwrap(); + } +} + #[test] fn detecting_comb_boxes_through_tools() { let mut body = String::from("BT /F1 10 Tf 40 105 Td (Postcode) Tj ET\n"); diff --git a/crates/forms/src/detect.rs b/crates/forms/src/detect.rs index d5bf073b1e..8c980c09f2 100644 --- a/crates/forms/src/detect.rs +++ b/crates/forms/src/detect.rs @@ -5,7 +5,7 @@ //! [`detect`] is a pure function over the page's words and drawn shapes; [`page_shapes`] reads //! the shapes from a page's content, [`raster::shapes`] from a picture of the page (a scan). -use pdfcraft_content::{Matrix, parse}; +use pdfcraft_content::{Matrix, Pieces}; use pdfcraft_cos::{Document, Object}; pub mod raster; @@ -136,11 +136,10 @@ pub fn page_shapes(doc: &Document, page: usize) -> Shapes { }, None => Vec::new(), }; - let mut data = Vec::new(); + let mut pieces = Vec::new(); for o in list { if let Object::Stream(s) = &*doc.resolve(&o) { - data.extend(s.decoded().unwrap_or_default()); - data.push(b'\n'); + pieces.push(s.decoded().unwrap_or_default()); } } let mut ctm = Matrix::IDENTITY; @@ -149,7 +148,7 @@ pub fn page_shapes(doc: &Document, page: usize) -> Shapes { let mut subpaths: Vec = Vec::new(); let mut path = DetectionPath::default(); let mut cur: Option<[f64; 2]> = None; - for op in parse(&data).ops { + for op in Pieces::join(&pieces).parse().ops { match op.op.as_slice() { b"q" => stack.push(ctm), b"Q" => ctm = stack.pop().unwrap_or(ctm), diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd8..ad01140099 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -1575,6 +1575,38 @@ fn page_shapes_reads_boxes_and_rules() { assert_eq!(sh.rules, [[40.0, 100.0, 240.0, 100.0], [60.0, 600.5, 260.0, 600.5]]); } +#[test] +fn page_shapes_preserves_content_piece_boundaries() { + use crate::detect::{Candidate, Kind, Shapes, detect, page_shapes}; + let read = |pieces: &[&[u8]]| { + let mut doc = one_page(); + let page = pdfcraft_model::pages(&doc)[0].obj; + let contents = + pieces.iter().map(|bytes| Object::Ref(doc.add(Object::Stream(pdfcraft_cos::Stream::flate(Default::default(), bytes))))).collect(); + doc.update_dict(page, |d| d.set(b"Contents".to_vec(), Object::Array(contents))).unwrap(); + page_shapes(&doc, 0) + }; + let empty = Shapes::default(); + for pieces in [vec![b"% trailing".as_slice(), b"40 120 300 30 re S\n"], vec![b"% trailing".as_slice(), b"", b"", b"40 120 300 30 re S\n"]] { + let shapes = read(&pieces); + assert_eq!(shapes, empty, "an unterminated comment hides the next piece's first line"); + assert_eq!(detect(&[], &shapes, &[], &[]), Vec::::new()); + } + let visible = Shapes { boxes: vec![[40.0, 120.0, 340.0, 150.0]], rules: vec![] }; + for ending in [b"% trailing\n".as_slice(), b"% trailing\r", b"% trailing\r\n"] { + let shapes = read(&[ending, b"40 120 300 30 re S\n"]); + assert_eq!(shapes, visible); + assert_eq!(detect(&[], &shapes, &[], &[]), vec![Candidate { kind: Kind::Text, rect: [41.0, 121.0, 339.0, 149.0], name: "Text1".into() }]); + } + assert_eq!(read(&[b"40 120", b"", b"300 30", b"re", b"S"]), visible); + assert_eq!( + read(&[b"q 2 0 0 2 0 0 cm 10 10", b"60 20 re", b"S Q 10 20 m 100 20 l S"]), + Shapes { boxes: vec![[20.0, 20.0, 140.0, 60.0]], rules: vec![[10.0, 20.0, 100.0, 20.0]] } + ); + assert_eq!(read(&[]), empty); + assert_eq!(read(&[b""]), empty); +} + fn close(a: f64, b: f64) -> bool { (a - b).abs() < 0.05 } diff --git a/docs/roadmap.md b/docs/roadmap.md index 5d73511e9c..fd61c7e02d 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -2,6 +2,7 @@ > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-20 - **Last updated:** 2026-10-20 - **Change:** minor (M6, #3502: form detection uses the existing content-stream boundary policy; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-18 · **Change:** minor (M4, #1919: repeated page indices accumulate rotations; M4: indirect page rotations refreshed upstream; M13: CLI check visits each directory once through aliases and cycles; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -133,7 +134,7 @@ total matches the parity estimate. | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Missing: page transitions, recovery on the web M4: Insert-from-file preserves source bookmarks; integration tests retained with current main. | | M5 | Comments (all annotation types, XFDF) | 120–200 | 88% | 12–25 | Done: notes, highlight/underline/strikeout/squiggly, text boxes, ink, lines, arrows, rectangles, ovals, stamps (dynamic, Sign Here, Standard Business), with appearance streams; replies, status, checkmarks, locking, move/resize/restyle/delete; properties dialog; panel filter and sort; hide all; summaries (comments only); XFDF/FDF import and export (partial imports preserve existing reply and review-status links); flatten; Fill & Sign with adjustable text/date size; agent tools.
Done since: stamps placed on a rotated page read upright (#1398); polygons, connected lines, clouds, callouts, inserted text, colour and checkmark filters; image signature pointer previews, selection handles and live image movement/resizing with proportional corners and independent edge handles. Missing: custom stamps, replace-text proposals, summary layouts with the page | | M5 | Comments (all annotation types, XFDF) | 120–200 | 88% | 12–25 | Done: notes, highlight/underline/strikeout/squiggly, text boxes, ink, lines, arrows, rectangles, ovals, stamps (dynamic, Sign Here, Standard Business), with appearance streams; replies, status, checkmarks, locking, move/resize/restyle/delete; properties dialog; panel filter and sort; hide all; summaries (comments only); XFDF/FDF import and export (partial imports preserve existing reply and review-status links; nested FDF export uses lazy array owners and annotation positions, with retained entries budgeted before cloning under copied-graph limits and source `/Page` and `/Pages` dictionaries replaced by null at every copy depth); flatten; Fill & Sign with adjustable text/date size; agent tools.
Done since: polygons, connected lines, clouds, callouts, inserted text, colour and checkmark filters; image signature pointer previews, selection handles and live image movement/resizing with proportional corners and independent edge handles. A refused restyle or resize of a text box another program drew leaves that comment unchanged (#1327). Make Current Properties Default on an arrow updates arrows, not plain lines (#1302). Flattening keeps a visible comment it cannot draw (#1317). Missing: custom stamps, replace-text proposals, summary layouts with the page | -| M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Done since: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. (#2892). Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | +| M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Done since: Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. (#2892). Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values.
Done since: Form detection preserves comments across content streams and does not add fields over rectangles hidden by those comments. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Refreshed against upstream `99941daa` (2026-10-15).
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | | M6 | Forms + JavaScript | 160–320 | 50% | 40–75 | Done: filling all field types with regenerated appearances; Prepare a form (every field type, multi-select/shared properties, move/resize/delete, Field Properties General/Appearance/Position/Options/Format/Validate/Calculate); Acrobat's AF format/keystroke/validate/calculate functions and simplified field notation run natively in Acrobat's event order; tab order; push-button actions; flatten; agent tools. Form data exchange (FDF, XFDF, XML, CSV, text) done; XFDF values retain text across XML comments and processing instructions. XML/XFDF exports preserve tabs and line breaks in names and comment metadata. Plain AcroForm XML imports retain all direct text around comments and processing instructions, including CDATA and character references. CSV export and spreadsheet merging quote tabs, and import distinguishes quoted tabs from TSV delimiters without changing field identities.
Done since: JavaScript engine (boa, sandboxed) running custom keystroke/validate/calculate/format and button scripts, document JavaScripts, console. Missing: Actions tab, wider object model (annotations, layers, dialogs), XFA
Done since: Multi-select choice fields write ascending `/I` option indices while preserving `/V` selection order; synthetic forms/engine regressions cover reverse selection and save/reopen. CSV export preserves carriage returns in names and values. | @@ -200,6 +201,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | M6, #3502: form detection preserves comments across content streams; estimates unchanged. | | 2026-10-18 | minor | M5, PR #1971: nested source `/Page` and `/Pages` dictionaries export as null without traversing page content or the page tree. Synthetic saved/reopened three-page regression retains unrelated metadata and aliases. FDF import semantics, copied-graph limits and estimates unchanged. | | 2026-10-15 | minor | M5: budget retained FDF entries before cloning; discovery keeps lazy array owners and annotation positions, with alias/cache-eviction stress fixtures. Limits cover the copied graph; source parsing/page discovery retain existing COS/parser limits, with no total source-memory bound. FDF import semantics and estimates unchanged. | | 2026-10-15 | minor | M7: Edit text caret height follows its row, with a kittest regression; estimates unchanged | From ad453129f0a43ce282d841223491cf7476135fb1 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:23:23 -0400 Subject: [PATCH 27/34] M5: doc comments for the functions around this change --- crates/forms/src/appearance.rs | 2 ++ crates/forms/src/tests.rs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/crates/forms/src/appearance.rs b/crates/forms/src/appearance.rs index 35bb9146ac..6d7275470c 100644 --- a/crates/forms/src/appearance.rs +++ b/crates/forms/src/appearance.rs @@ -240,6 +240,8 @@ pub(crate) fn button_caption(doc: &Document, da: &str, caption: &str, width: f64 (out, name, font, missing) } +/// A non-embedded Helvetica font dictionary in WinAnsi encoding, the fallback when the form's +/// `/DR` has no font that can show the text. fn helvetica() -> Object { let mut f = Dict::new(); f.set(b"Type".to_vec(), Object::name("Font")); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 40d44677ff..41aa52a53e 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -80,6 +80,7 @@ fn field<'a>(all: &'a [Field], name: &str) -> &'a Field { all.iter().find(|f| f.name == name).unwrap_or_else(|| panic!("no field {name}")) } +/// The decoded text of widget `w`'s normal (`/N`) appearance stream. fn ap(doc: &Document, w: &Widget) -> String { let wd = doc.get(w.obj).as_dict().cloned().unwrap(); let n = wd.get(b"AP").unwrap().as_dict().unwrap().reference(b"N").expect("normal appearance stream"); @@ -136,6 +137,9 @@ fn a_button_caption_its_fonts_encoding_cant_hold_is_reported() { assert_eq!(undrawn_fields(&doc), [("go".to_string(), '\u{2000B}')]); } +/// Changing one widget's caption on a push button with two widgets leaves the other widget, the +/// shared `/MK` and the parent's action untouched, keeps the other `/MK` and `/AP` entries, and +/// drops the stale rollover and down appearances, with a direct or indirect caption and either save. #[test] fn button_caption_preserves_other_widgets_and_shared_dictionaries() { for indirect_caption in [false, true] { From 4125bddbae5bd52cc8aecf8e8ad19f7d959e046f Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:30:45 +0200 Subject: [PATCH 28/34] Parse a bare trailing "a" as AM in AcroForm date/time parsing Found while testing crates/forms/src/af.rs:638-642 and 750-755 (parse_date) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/forms/src/af.rs | 3 ++- crates/forms/tests/af_date_parse.rs | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 crates/forms/tests/af_date_parse.rs diff --git a/crates/forms/src/af.rs b/crates/forms/src/af.rs index c5b81719ac..d95a41ba3c 100644 --- a/crates/forms/src/af.rs +++ b/crates/forms/src/af.rs @@ -639,7 +639,8 @@ pub fn parse_date(text: &str, fmt: &str) -> Option { // A bare trailing "p" ("5:30p") only counts when it follows a digit, not a word like "Sep". let bare_p = lower.strip_suffix('p').is_some_and(|rest| rest.trim_end().ends_with(|c: char| c.is_ascii_digit())); let pm = lower.contains("pm") || bare_p; - let am = lower.contains("am"); + let bare_a = lower.strip_suffix('a').is_some_and(|rest| rest.trim_end().ends_with(|c: char| c.is_ascii_digit())); + let am = lower.contains("am") || bare_a; // Tokens: runs of digits (kept as text, so a run can be split between fields typed without // separators) and month names. let mut nums: VecDeque = VecDeque::new(); diff --git a/crates/forms/tests/af_date_parse.rs b/crates/forms/tests/af_date_parse.rs new file mode 100644 index 0000000000..802c40dc61 --- /dev/null +++ b/crates/forms/tests/af_date_parse.rs @@ -0,0 +1,10 @@ +use pdfcraft_forms::af::parse_date; + +#[test] +fn bare_a_suffix_is_am() { + assert_eq!(parse_date("12:05 a", "h:MM t").map(|d| d.hh), Some(0)); + assert_eq!(parse_date("12:30a", "h:MM t").map(|d| d.hh), Some(0)); + assert_eq!(parse_date("5:30 a", "h:MM t").map(|d| d.hh), Some(5)); + assert_eq!(parse_date("5:30p", "h:MM t").map(|d| d.hh), Some(17)); + assert_eq!(parse_date("12:05 p", "h:MM t").map(|d| d.hh), Some(12)); +} From d335ff375d4de3d1f9e6ae253915eef929a2d558 Mon Sep 17 00:00:00 2001 From: Unhumanje <54317404+petarlaf@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:57:00 +0200 Subject: [PATCH 29/34] fix(xfdf): a field name full of periods no longer overflows the stack on export The XFDF export nested one element per period-separated part of a field's full name. A /T can hold periods of its own, so a name can have far more parts than the field tree has levels. Past 64 levels the import refuses the file the export wrote, and around 100,000 parts the recursive write overflows the main thread's stack and aborts the process. The export now nests at most as many levels as the import reads back. The parts past the last level stay in the deepest element's name, and the import joins them back into the same full name. --- crates/xfdf/src/lib.rs | 8 +++++++- crates/xfdf/src/tests.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 35bc781541..30512f04e4 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -415,7 +415,13 @@ fn xfdf_fields(doc: &Document, out: &mut String) { continue; } let mut node = &mut root; - for part in f.name.split('.') { + // One `` level per part of the name, but no more levels than the import reads + // back. A `/T` can hold periods of its own, so a name can split into far more parts than + // the field tree has levels: nesting every part wrote a file the import refuses and, for + // a long enough name, recursed in `write_node` until the stack overflowed. The parts past + // the last level stay together in the deepest element's name, and the import joins them + // back with periods into the same full name. + for part in f.name.splitn(MAX_DEPTH, '.') { let i = match node.kids.iter().position(|(k, _)| k == part) { Some(i) => i, None => { diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index c33c115e75..8e6f753a8a 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1104,6 +1104,45 @@ fn wide_or_commented_xml_is_not_mistaken_for_deep() { assert!(check_form_xml_depth(&nested(MAX_XML_DEPTH + 1)).is_err()); } +/// [`one_field`] with its field's `/T` set to `name`, periods and all, holding `value`. +fn one_field_named(name: &str, value: &str) -> Document { + let mut doc = one_field(); + let r = fields(&doc).into_iter().find(|f| f.name == "Name").unwrap().obj; + doc.update_dict(r, |d| { + d.set(b"T".to_vec(), Object::String(PdfString::text(name))); + d.set(b"V".to_vec(), Object::String(PdfString::text(value))); + }) + .unwrap(); + doc +} + +/// A `/T` can hold periods of its own, so a field name can split into more parts than the field +/// tree has levels. The XFDF export nested one `` per part, and the import refuses more +/// than 64 levels, so the form's own export could not be read back. +#[test] +fn field_names_with_more_parts_than_the_import_reads_still_round_trip() { + for parts in [1, 2, 63, 64, 65, 66, 100, 1_000] { + let name = (0..parts).map(|i| format!("p{i}")).collect::>().join("."); + let doc = one_field_named(&name, "v"); + assert_eq!(values(&doc), vec![(name.clone(), vec!["v".to_string()])], "{parts} parts"); + let xfdf = export_xfdf(&doc, false, true, "form.pdf"); + assert_eq!(data_values(xfdf.as_bytes()), Ok(vec![(name.clone(), vec!["v".to_string()])]), "{parts} parts"); + let mut back = one_field_named(&name, "old"); + let report = import(&mut back, xfdf.as_bytes()).unwrap(); + assert_eq!((report.fields, values(&back)), (1, vec![(name.clone(), vec!["v".to_string()])]), "{parts} parts"); + } +} + +/// A name of a hundred thousand parts nested as deep, and writing it out recursed once per level +/// until the stack overflowed, which aborts the process. +#[test] +fn a_field_name_of_many_parts_exports_without_overflowing_the_stack() { + let name = vec!["p"; 100_000].join("."); + let want = vec![(name.clone(), vec!["v".to_string()])]; + let read = on_small_stack(move || data_values(export_xfdf(&one_field_named(&name, "v"), false, true, "form.pdf").as_bytes())); + assert_eq!(read, Ok(want)); +} + #[test] fn duplicate_xfdf_annotation_flags_do_not_turn_on_unrelated_bits() { // Flags are a set of bit masks, not additive integers. A repeated "print" flag From 7ddb6226b8f63e0ddcac43a52a23761262068682 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:22:45 +0800 Subject: [PATCH 30/34] M6: hovering a form field shows its tooltip (#3854) The field's /TU is read into FormField::tooltip and shown in the Fields panel, but resting the pointer on the field on the page showed nothing. page_input now attaches it to the hover, as Acrobat does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R3niW7kVeT8mi7TpWc8eAJ --- crates/ui-egui/src/forms_ui.rs | 5 +++++ crates/ui-egui/tests/forms.rs | 27 +++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 ++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index 612b985e65..ced8d76046 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -132,6 +132,11 @@ pub(crate) fn page_input( .map(|(i, w)| (f, i, w)) }); let Some((f, wi, w)) = hit else { return FieldPointer::Away }; + // The field's tooltip (`/TU`), as Acrobat shows it on hover (#3854). ISO 32000-2 Table 226 + // makes it the name the user interface identifies the field by. + if let Some(tip) = f.tooltip.as_deref().filter(|t| !t.trim().is_empty()) { + resp.clone().on_hover_text_at_pointer(tip); + } // Empty signature fields are signed by clicking them (Use a certificate); push buttons run // their action. let pressable = f.kind == FormFieldKind::PushButton && !f.read_only() && f.button.is_some(); diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index e153dcca94..7418b55a32 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -1,6 +1,7 @@ //! Filling in a form in the real shell (egui_kittest): typing, Tab, check boxes, radios, choices. use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; use pdfcraft_ui_egui::PdfCraftApp; fn harness() -> Harness<'static, PdfCraftApp> { @@ -44,6 +45,32 @@ fn click_field(h: &mut Harness<'static, PdfCraftApp>, name: &str, widget: usize) h.run_steps(3); } +/// Rest the pointer on the centre of a field's widget long enough for a tooltip. +fn rest_on_field(h: &mut Harness<'static, PdfCraftApp>, name: &str) { + let p = { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + let f = doc.form.iter().find(|f| f.name == name).unwrap(); + pdfcraft_ui_egui::forms_ui::field_screen_rect(&s.views[0], &doc.info, f, 0).expect("on screen").center() + }; + h.hover_at(p); + for _ in 0..40 { + h.step(); + std::thread::sleep(std::time::Duration::from_millis(15)); + } +} + +/// #3854: resting on a field shows its tooltip (`/TU`); a field without one shows none. +#[test] +fn hovering_a_field_shows_its_tooltip() { + let mut h = harness(); + assert!(h.query_by_label("Your name").is_none(), "no tooltip before the hover"); + rest_on_field(&mut h, "name"); + assert!(h.query_by_label("Your name").is_some(), "the name field's /TU shows on hover"); + rest_on_field(&mut h, "city"); + assert!(h.query_by_label("Your name").is_none(), "another field doesn't keep the tooltip"); +} + #[test] fn typing_into_a_text_field_and_tabbing_on() { let mut h = harness(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..7292ae3173 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4416,8 +4416,8 @@ milestone = "M6" status = "shipped" commands = ["form.fields"] tools = ["form_fill", "form_fields"] -evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/ui-egui/tests/forms.rs::auto_size_fields_shrink_and_wrap_typed_text_to_fit_their_box", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes", "crates/automation/tests/automation.rs::orphan_form_fields_through_tools", "crates/forms/src/tests.rs::fields_sharing_a_name_take_the_same_value"] -notes = "in-place editor; Enter or clicking away commits, Escape cancels; auto-size (0 Tf) fields shrink, and multiline ones wrap, the text being typed to fit the box, on screen as on the page (the screen measures in the interface font, so its size can be a little smaller than the page's Helvetica fit); fixed sizes are kept and clip, as in Acrobat; no format/keystroke JavaScript yet (M6.4)" +evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/ui-egui/tests/forms.rs::auto_size_fields_shrink_and_wrap_typed_text_to_fit_their_box", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes", "crates/automation/tests/automation.rs::orphan_form_fields_through_tools", "crates/forms/src/tests.rs::fields_sharing_a_name_take_the_same_value", "crates/ui-egui/tests/forms.rs::hovering_a_field_shows_its_tooltip"] +notes = "in-place editor; Enter or clicking away commits, Escape cancels; auto-size (0 Tf) fields shrink, and multiline ones wrap, the text being typed to fit the box, on screen as on the page (the screen measures in the interface font, so its size can be a little smaller than the page's Helvetica fit); fixed sizes are kept and clip, as in Acrobat; no format/keystroke JavaScript yet (M6.4); resting the pointer on any field shows its tooltip (/TU), as Acrobat does (#3854)" [[feature]] id = "form.fill-checkbox-radio" From 33150203a8a8bda0ff6490aec67bd70bb319eaf0 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:53:56 +0800 Subject: [PATCH 31/34] M6: test the read-only and blank-/TU tooltip cases (#3854) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R3niW7kVeT8mi7TpWc8eAJ --- crates/ui-egui/tests/forms.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index 7418b55a32..b261d33162 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -71,6 +71,33 @@ fn hovering_a_field_shows_its_tooltip() { assert!(h.query_by_label("Your name").is_none(), "another field doesn't keep the tooltip"); } +/// Give the `city` field these properties, as the Properties dialog would. +fn set_city(h: &mut Harness<'static, PdfCraftApp>, props: pdfcraft_engine::FieldProps) { + h.state_mut().apply_edit(pdfcraft_engine::Edit::SetFieldProps { name: "city".into(), props: Box::new(props) }); + h.run_steps(3); +} + +/// A read-only field still shows its tooltip: it can't be filled, but it can be identified. +#[test] +fn a_read_only_fields_tooltip_shows_on_hover() { + let mut h = harness(); + set_city(&mut h, pdfcraft_engine::FieldProps { tooltip: Some("Where you live".into()), read_only: Some(true), ..Default::default() }); + rest_on_field(&mut h, "city"); + assert!(h.query_by_label("Where you live").is_some()); +} + +/// A `/TU` of only spaces shows no tooltip (no empty box under the pointer). +#[test] +fn a_blank_tooltip_shows_nothing() { + let mut h = harness(); + set_city(&mut h, pdfcraft_engine::FieldProps { tooltip: Some(" ".into()), ..Default::default() }); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.form.iter().find(|f| f.name == "city").unwrap().tooltip.as_deref(), Some(" "), "the blank /TU is in the file"); + rest_on_field(&mut h, "city"); + assert!(egui::Tooltip::seconds_since_last_tooltip(&h.ctx).is_infinite(), "no tooltip was shown"); +} + #[test] fn typing_into_a_text_field_and_tabbing_on() { let mut h = harness(); From 81c08be3eaa4c4d7fed935db92d4be31ea191723 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:10:00 +0800 Subject: [PATCH 32/34] M6: a field without /TU shows no tooltip of its own (#3854) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R3niW7kVeT8mi7TpWc8eAJ --- crates/ui-egui/tests/forms.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index b261d33162..f2f90577fa 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -69,6 +69,9 @@ fn hovering_a_field_shows_its_tooltip() { assert!(h.query_by_label("Your name").is_some(), "the name field's /TU shows on hover"); rest_on_field(&mut h, "city"); assert!(h.query_by_label("Your name").is_none(), "another field doesn't keep the tooltip"); + // Nor shows one of its own: none has been shown since the pointer left `name` (well past the + // tooltip delay), so this isn't only the old label going away. + assert!(egui::Tooltip::seconds_since_last_tooltip(&h.ctx) > 0.3, "a field without /TU shows no tooltip"); } /// Give the `city` field these properties, as the Properties dialog would. From 64e915a15693a36e614b30c3895a83d82fa6e430 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 01:45:19 +0000 Subject: [PATCH 33/34] Integration: #3543 merge the duplicated doc comment on button_appearance --- crates/forms/src/author.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index a99ee0c659..c7aff9bc21 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -767,10 +767,10 @@ fn form_stream(place: crate::appearance::Placement, content: Vec, resources: } /// A push button: background, border, its icon (`/MK /I`, scaled to fit and centred) and its -/// centred caption (unless the layout is icon only, `/TP 1`). The caption is drawn in Helvetica, -/// with the first character it can't draw (drawn as `?`), for the caller to mark and report. -/// A push button's appearance: its icon and caption, the caption in the font its `/DA` asks for -/// (the widget's, else `field_da`, the field's own or inherited one). +/// centred caption (unless the layout is icon only, `/TP 1`). The caption uses the font, size and +/// color its `/DA` asks for (the widget's, else `field_da`, the field's own or inherited one). +/// Also returns the first caption character the font can't draw (drawn as `?`), for the caller to +/// mark and report. fn button_appearance(doc: &Document, w: &Widget, field_da: &str) -> (Stream, Option) { let (mut c, place) = frame_only(doc, w); let (width, height) = (place.layout_w, place.layout_h); From 3463d0e0f972b252e3e1f7060a9beb73c1544c58 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 01:46:20 +0000 Subject: [PATCH 34/34] Integration: #3323 drop docs/form-maxlen-compatibility.md --- docs/form-maxlen-compatibility.md | 117 ------------------------------ 1 file changed, 117 deletions(-) delete mode 100644 docs/form-maxlen-compatibility.md diff --git a/docs/form-maxlen-compatibility.md b/docs/form-maxlen-compatibility.md deleted file mode 100644 index adcc02ce6d..0000000000 --- a/docs/form-maxlen-compatibility.md +++ /dev/null @@ -1,117 +0,0 @@ -# AcroForm `/MaxLen 0` compatibility - -Status: implemented; proposed as a separate compatibility PR. Last updated: 2026-10-19. - -- `crates/forms/src/lib.rs::walk` reads inherited `/MaxLen`; the UI and value validation both - consume `Field::max_len`. - -The parser exposed `/MaxLen 0` as `Some(0)`, so both editor and API blocked input. It now -exposes only positive limits; zero clears an inherited limit. The original object entry is -preserved through save/reopen. This is an interoperability interpretation for zero, **not -a claim that ISO explicitly defines zero as unlimited**. ISO 32000-1 Table 229 defines an -optional inheritable maximum length without defining a special zero meaning: -[public specification](https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf). -Positive limits remain enforced; no field names or character-sheet rules enter product code. - -Regression: `crates/forms/src/tests.rs::zero_max_len_is_unlimited_and_overrides_inherited_limit`. -The forms README and `form.fill-text` parity entry document this behavior. - -Historical validation recorded before this review: - -- Forms unit suite: all 68 tests passed. The new zero-limit regression was first run against - the old parser and failed (`Some(0)` instead of `None`), then passed with the compatibility - interpretation. Its positive-limit control and preserved raw entry also passed. - -The user reports successful Class text input on the integrated release using their Pathfinder -character sheet. This is a manual functional result, without timings; the private document -was not used for automated regression tests in this review. - -## Independent branch validation - -Validated on upstream `a12fe8f4116984ebbc870eb514346ea922233316` with an isolated -Cargo target directory and no performance changes: - -- The focused regression failed with the original parser (`Some(0)` rather than `None`), - then passed after restoring the compatibility fix. -- `cargo test -p pdfcraft-forms -j 2`: 81 unit tests passed; doc tests passed. -- `cargo clippy -p pdfcraft-forms -j 2 --all-targets -- -D warnings`: passed. -- `cargo check --target wasm32-unknown-unknown -p pdfcraft-forms -j 2`: passed. -- `cargo fmt --check`, `git diff --check`, and `cargo xtask layers`, `assets`, `parity`: passed. - -These are local checks, not hosted CI. No private PDF was used. - -## Rebase validation - -Rebased onto freshly fetched upstream main -`932bf9c6bfdd63e2a21bb831696a3447b9def9cf` on 2026-10-18. The only conflict was -in the M6 roadmap row; upstream CSV/XML updates were retained alongside the compatibility -note. The implementation and original regression are unchanged from the independent branch. - -Validation used a fresh Cargo target directory, two build jobs, and locked, offline builds: - -- The original regression failed with the upstream parser (`Some(0)` instead of `None`), - then passed with the compatibility fix restored. -- Forms: 83 unit tests passed. Engine: 282 unit and 9 integration tests passed. - JavaScript: 47 unit tests passed. Doc tests passed. The timing probe and two private-file - tests remained ignored. -- The automation `javascript_through_tools` regression passed. -- Scoped Clippy for forms, engine and JavaScript, with `--all-targets -- -D warnings`, passed. -- WASM checks for those three crates passed. -- Rust formatting, whitespace, layering, assets and parity checks passed. -- A freshly built CLI filled synthetic fields with direct and inherited `/MaxLen 0`, rendered - their values, saved/reopened them and accepted longer text after reopening. Its positive-limit - control rejected six characters for `/MaxLen 5`; the synthetic input file stayed unchanged. - -No performance optimizations, JavaScript snapshot/diff changes or profiling instrumentation -are included. Existing executables, backups and private PDFs were not modified. - -## PR #3323 refresh - -Revalidated against freshly fetched upstream main -`4db6ac4ae2448df6389d8bb0bc54db6ba175721c` on 2026-10-18. Conflicts in both -roadmaps were resolved by keeping upstream's button-label and content-editing notes alongside -the `/MaxLen 0` documentation. The original compatibility implementation and regression are -preserved; no performance changes were added to the PR. - -Forms: 85 unit tests passed. Engine: 283 unit and 15 integration tests passed. JavaScript: -48 unit tests passed. The automation JavaScript regression, scoped Clippy with warnings denied, -WASM, formatting, whitespace, layering, assets and parity checks all passed. The synthetic CLI -fill/render/save/reopen and positive-limit control passed. The original regression again failed -with the old parser and passed with the fix restored. Private-file tests remained ignored. - -## 2026-10-19 revalidation - -Rebased onto freshly fetched upstream main -`459379ef9b127648cc30c0b2fe2b20c2b2375f04`. Only `ROADMAP.md` required manual -conflict resolution; all upstream entries and the compatibility note were retained. -The implementation, original regression and forms README remain byte-for-byte identical -to the previously reviewed compatibility head. No performance changes were introduced. - -Validation used a fresh isolated build directory, two jobs, and locked offline builds: - -- The original regression failed with the old parser and passed with the fix restored. -- Forms: 93 unit tests passed. Engine: 283 unit and 15 integration tests passed. - JavaScript: 48 tests passed. Doc tests passed; the timing probe and two private-file - tests remained ignored. -- Automation JavaScript, scoped all-targets Clippy with warnings denied, WASM, formatting, - whitespace, layering, assets and parity passed. -- Synthetic CLI fill/render/save/reopen checks passed for direct zero and zero overriding - an inherited positive limit. Longer text after reopening was accepted. The `/MaxLen 5` - control rejected six characters, and the synthetic input stayed unchanged. - -These are local checks, not hosted CI. Existing executables, backups and private PDFs -were not modified; only the compatibility worktree was changed. - -## Revision history - -- 2026-10-19: refresh PR #3323 onto upstream `459379ef`, retain all upstream roadmap entries - and independently repeat regression, native/WASM, repository and CLI validation. - -- 2026-10-18: refresh PR #3323 onto upstream `4db6ac4a`, retaining both roadmap updates and - independently revalidating compatibility and repository checks. - -- 2026-10-18: rebase onto fetched upstream `932bf9c6`; preserve upstream roadmap updates and - independently revalidate the regression, forms/engine/JavaScript suites and quality gates. - -- 2026-10-18: isolate the compatibility change on current upstream main; verify the - regression before and after the fix, the forms suite and scoped quality gates.