M9: name a laid-out XFA form's signature fields by their SOM path, so other viewers find the signature (#4507)

* M9: name a laid-out XFA form's signature fields by their SOM path, so other viewers find the signature

* M9: doc comments on the functions the signature naming touches

* M9: say other viewers in the signature naming's comments

* M9: a new layout keeps a signature a generated signature box holds

* M9: the re-layout test checks the signature's picture is kept too

* M9: a signature field named by its SOM path doesn't repeat a name a field already has

* M9: every name a field of the pass takes is known before a signature field is named, in either order

* M9: the signed-box test's doc comment is back on its test

A merge had left it above the next test, which has its own.

crates/automation/tests/it/automation.rs

@@ -5848,6 +5848,28 @@ fn signing_with_an_image_typed_or_drawn_graphic_through_tools() {
     assert!(inked(60, 110), "the strokes are drawn");
 }
 
+/// Signing a laid-out XFA form's signature box through the tools signs the field named by its
+/// SOM path, which is how another viewer, laying the form out again, finds the signature; the
+/// signed file reopens with the signature intact and on that field.
+#[test]
+fn a_dynamic_xfa_forms_signature_box_is_signed_under_its_som_path() {
+    let dir = workdir("xfa-sign");
+    std::fs::write(dir.join("form.pdf"), pdfcraft_xfa::fixtures::shell(&pdfcraft_xfa::fixtures::fields_template(""))).unwrap();
+    let mut a = auto(&dir);
+    ok(&mut a, "sign_id_create", json!({ "name": "Ada Lovelace", "key": "p256", "password": "secret12", "path": "ada.p12" }));
+    let doc = ok(&mut a, "doc_open", json!({ "path": "form.pdf" }))["doc"].as_u64().unwrap();
+    let som = "form[0].page1[0].sign[0]";
+    let fields = ok(&mut a, "form_fields", json!({ "doc": doc }));
+    assert!(fields["fields"].as_array().unwrap().iter().any(|f| f["name"] == som), "{fields}");
+    ok(&mut a, "sign_document", json!({ "doc": doc, "id": "ada.p12", "password": "secret12", "field": som, "out": "signed.pdf" }));
+    let signed = ok(&mut a, "doc_open", json!({ "path": "signed.pdf" }))["doc"].as_u64().unwrap();
+    let list = ok(&mut a, "sign_list", json!({ "doc": signed }));
+    let s = list["signatures"].as_array().unwrap().iter().find(|s| s["signed"] == true).cloned().unwrap_or_else(|| panic!("{list}"));
+    assert_eq!(s["field"], som, "{s}");
+    assert_eq!(s["status"], "unknown", "intact, the signer not trusted: {s}");
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn digital_ids_signing_and_validation_through_tools() {
     let dir = workdir("signing");

crates/engine/src/tests.rs

@@ -5667,6 +5667,8 @@ fn xfa_data_no_field_binds_to_survives_an_edit_and_save() {
     assert!(doc.xfa_warnings.iter().any(|w| w.contains("structured content")));
 }
 
+/// An XFA form's choice lists, signature and picture fields open as their kinds of field, take
+/// values that scripts read, and are saved into the datasets.
 #[test]
 fn xfa_choice_lists_signature_and_picture_fields_open_fill_script_and_save() {
     let data = "<form><page1><country>Japan</country><langs>English\nSpanish</langs></page1></form>";
@@ -5686,7 +5688,8 @@ fn xfa_choice_lists_signature_and_picture_fields_open_fill_script_and_save() {
     assert_eq!(langs.value, vec!["English".to_string(), "Spanish".to_string()]);
     assert!(field(&s, "other").has(pdfcraft_forms::flags::EDIT));
     assert!(field(&s, "pin").has(pdfcraft_forms::flags::PASSWORD));
-    assert_eq!(field(&s, "sign").kind, K::Signature);
+    // The signature field is named by its SOM path, as another viewer finds a signature.
+    assert_eq!(field(&s, "form[0].page1[0].sign[0]").kind, K::Signature);
     // Scripts read a choice list's saved value.
     assert_eq!(field(&s, "summary").value, vec!["Country: JP".to_string()], "{:?}", s.take_js_output(id));
     // Choosing another item recalculates; the list takes several values; a typed value is kept

crates/xfa/README.md

@@ -38,8 +38,11 @@ that warning and keeps the existing layout and accepted widget values.
   XObjects, and widgets: text (multi-line, max length, alignment), date (Acrobat's `AFDate`
   format and keystroke actions from the picture clause), check boxes and radio groups (from
   `exclGroup`), push buttons with Reset / Print / Save As / URI actions recognised from their
-  scripts. Appearance streams are left empty for the forms layer to generate. Each field keeps
-  its SOM path in `/PCSom`; the AcroForm gets `/PCXfaLayout` so a saved form is not laid out
+  scripts, and signature fields, each named by its SOM path (`form1[0].page1[0].sign[0]`, as
+  fields under one for each step above it) because other viewers lay a dynamic form out again
+  and find a signature by that name. A box whose path, or a part of it, the file already gives
+  another field keeps its own name, and a new layout keeps a signature a generated box holds. Appearance streams are left empty for the forms layer to
+  generate. Each field keeps its SOM path in `/PCSom`; the AcroForm gets `/PCXfaLayout` so a saved form is not laid out
   twice. The XFA packets stay, so Adobe's viewers keep rendering the form from them.
 
 - **Data** (`data`): the `datasets` packet. On layout, a field takes its value from the data
@@ -82,6 +85,7 @@ be measured exponentially often in their nesting. A measurement budget backs thi
   | `/PCSom` | generated fields | the field's SOM path, so values go back to the data |
   | `/PCXfaLayout` | AcroForm | what laying out produced, so a saved form is not laid out again |
   | `/PCXfaOverrides` | AcroForm | `<< /Presence << /som (hidden) … >> /Access << /som (readOnly) … >> >>`: presence and access scripts set, by SOM path (instance 0 of a repeating subform stands for all); applied to the template before every layout, so undo, save and reopen keep them. Adobe's viewers draw from the XFA packets and don't read it; at most 10 000 entries are read or written |
+  | `/PCXfaNode` | generated fields above a signature field | `true`: the field names a step of the signature field's SOM path; a new layout replaces it with the fields it holds |
   | `/PCXfaClick` | generated push buttons | `true`: the template has a click script for this button. Such buttons carry no `/A`: XFA JavaScript is not Acrobat JavaScript, so it is never written as a `/S /JavaScript` action other viewers would run. PdfCraft finds the script by the field's `/PCSom` |
 
 ## API sketch

crates/xfa/src/lib.rs

@@ -99,7 +99,7 @@ pub use data::{
 pub use layout::{Action, BorderShape, Form, Item, MAX_PAGES, Page, Widget, WidgetKind, formatted_value, layout};
 pub use packets::{Encoding, Packets, decode as decode_packet, encode as encode_packet, read_packets};
 pub use parse::{measure, parse};
-pub use pdf::{CLICK_KEY, LAYOUT_KEY, SOM_KEY, Stripped, XfaShape, existing_layout, strip, xfa_shape};
+pub use pdf::{CLICK_KEY, LAYOUT_KEY, NODE_KEY, SOM_KEY, Stripped, XfaShape, existing_layout, strip, xfa_shape};
 pub use picture::typed_date;
 pub use script::{
     FormNode, LiveForm, MAX_FORM_NODES, NodeKind, OVERRIDES_KEY, Overrides, ScriptEvent, apply_overrides, data_of, field_grows_with_text,

crates/xfa/src/pdf.rs

@@ -2,7 +2,7 @@
 //! streams (standard-14 fonts, WinAnsi), image XObjects and AcroForm widgets. Widget appearance
 //! streams are left empty here; the forms layer regenerates them from the field dictionaries.
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 
 use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, Stream};
 use pdfcraft_fonts::{literal, win_ansi};
@@ -21,6 +21,10 @@ pub const LAYOUT_KEY: &[u8] = b"PCXfaLayout";
 /// PDF action (the script is XFA's, not Acrobat JavaScript); PdfCraft runs the script by the
 /// field's SOM path.
 pub const CLICK_KEY: &[u8] = b"PCXfaClick";
+/// Private key on the fields generated to name a signature field by its SOM path, one for each
+/// step of the path above it (`form1[0]`, then `page1[0]`): a new layout replaces them with the
+/// fields they hold.
+pub const NODE_KEY: &[u8] = b"PCXfaNode";
 
 /// Width and height of a JPEG from its first frame header (SOF0–SOF15, not the DNL/JPG/DAC/DHT
 /// markers). `None` for anything else.
@@ -264,6 +268,11 @@ struct Emitter<'a> {
     warnings: Vec<String>,
     /// The signatures the file holds, by the SOM path of the field signed.
     signed: HashMap<String, Signed>,
+    /// The full names of the fields the file came with, which a generated name must not repeat.
+    taken: HashSet<String>,
+    /// The fields above the signature fields named by their SOM paths, by the path they name:
+    /// each one's object and the fields it holds.
+    nodes: HashMap<String, (ObjRef, Vec<ObjRef>)>,
 }
 
 /// A signature field of the file that has been signed.
@@ -279,13 +288,16 @@ const MAX_FIELDS_SEARCHED: usize = 20_000;
 /// Deepest field nesting followed.
 const MAX_FIELD_DEPTH: usize = 32;
 
-/// The signed signature fields the document came with, by full name. Signing applications name
-/// a dynamic form's fields by their SOM path (`form1[0].page1[0].sign[0]`), which is how a
-/// laid-out signature box finds its signature. Fields of an earlier layout pass are not read.
-fn signed_fields(doc: &Document, catalog: &Dict) -> HashMap<String, Signed> {
+/// The fields the document came with: the full name of each, and the signed signature fields by
+/// SOM path. Signing applications name a dynamic form's fields by their SOM path
+/// (`form1[0].page1[0].sign[0]`), which is how a laid-out signature box finds its signature. A
+/// field an earlier layout pass generated gives no name, but a signature on it is kept, by its
+/// `/PCSom`, so a new layout keeps the box signed.
+fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap<String, Signed>, HashSet<String>) {
     let mut out = HashMap::new();
-    let Some(acro) = catalog.get(b"AcroForm").map(|a| doc.resolve(a)) else { return out };
-    let Some(fields) = acro.as_dict().and_then(|a| a.get(b"Fields")).map(|f| doc.resolve(f)) else { return out };
+    let mut names = HashSet::new();
+    let Some(acro) = catalog.get(b"AcroForm").map(|a| doc.resolve(a)) else { return (out, names) };
+    let Some(fields) = acro.as_dict().and_then(|a| a.get(b"Fields")).map(|f| doc.resolve(f)) else { return (out, names) };
     let mut stack: Vec<(Object, String, bool, usize)> =
         fields.as_array().into_iter().flatten().map(|f| (f.clone(), String::new(), false, 0)).collect();
     let mut seen = 0usize;
@@ -296,7 +308,22 @@ fn signed_fields(doc: &Document, catalog: &Dict) -> HashMap<String, Signed> {
         }
         let field = doc.resolve(&field);
         let Some(d) = field.as_dict() else { continue };
+        if d.contains(NODE_KEY) {
+            // Generated to name a signature field: only what it holds is looked at.
+            if depth < MAX_FIELD_DEPTH {
+                let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default();
+                stack.extend(kids.into_iter().map(|k| (k, String::new(), false, depth + 1)));
+            }
+            continue;
+        }
         if d.contains(SOM_KEY) {
+            let som = d.get(SOM_KEY).map(|s| doc.resolve(s)).and_then(|s| s.as_string().map(|s| s.to_text()));
+            if d.name(b"FT") == Some(b"Sig")
+                && let Some(som) = som
+                && let Some(value) = d.get(b"V").filter(|v| doc.resolve(v).as_dict().is_some())
+            {
+                out.entry(som).or_insert(Signed { value: value.clone(), appearance: d.get(b"AP").cloned() });
+            }
             continue;
         }
         let name = match d.get(b"T").map(|t| doc.resolve(t)).and_then(|t| t.as_string().map(|t| t.to_text())) {
@@ -304,6 +331,9 @@ fn signed_fields(doc: &Document, catalog: &Dict) -> HashMap<String, Signed> {
             Some(t) => format!("{prefix}.{t}"),
             None => prefix,
         };
+        if !name.is_empty() {
+            names.insert(name.clone());
+        }
         let is_sig = match d.name(b"FT") {
             Some(ft) => ft == b"Sig",
             None => parent_is_sig,
@@ -318,7 +348,7 @@ fn signed_fields(doc: &Document, catalog: &Dict) -> HashMap<String, Signed> {
             stack.extend(kids.into_iter().map(|k| (k, name.clone(), is_sig, depth + 1)));
         }
     }
-    out
+    (out, names)
 }
 
 impl Emitter<'_> {
@@ -363,6 +393,9 @@ impl Emitter<'_> {
             ff |= 1;
         }
         let mut top_level = true;
+        // The SOM path of the generated field above this one, for a signature field named by its
+        // SOM path.
+        let mut under: Option<String> = None;
         match &w.kind {
             WidgetKind::Text | WidgetKind::Date(_) | WidgetKind::Password => {
                 d.set(b"FT".to_vec(), Object::name("Tx"));
@@ -464,6 +497,16 @@ impl Emitter<'_> {
                     }
                     ff |= 1;
                 }
+                if let Some((above, last)) = self.som_steps(&w.som)
+                    && let Some(parent) = self.node(&above)
+                {
+                    // Named by its SOM path, as fields under one for each step above it: other
+                    // viewers lay a dynamic form out again and find a signature by that name.
+                    d.set(b"T".to_vec(), Object::String(PdfString::text(&last)));
+                    d.set(b"Parent".to_vec(), Object::Ref(parent));
+                    top_level = false;
+                    under = Some(above.join("."));
+                }
             }
             WidgetKind::Image => {
                 // A push button with nothing drawn: the forms layer treats one so marked as an
@@ -574,6 +617,12 @@ impl Emitter<'_> {
         if top_level {
             self.fields.push(r);
             self.field_count += 1;
+        } else if let Some(path) = under {
+            if let Some((_, kids)) = self.nodes.get_mut(&path) {
+                kids.push(r);
+            }
+            self.taken.insert(w.som.clone());
+            self.field_count += 1;
         } else if let WidgetKind::Radio { group, .. } = &w.kind
             && let Some((_, kids)) = self.radio_groups.get_mut(group)
         {
@@ -582,6 +631,60 @@ impl Emitter<'_> {
         Ok(Some(r))
     }
 
+    /// The steps of SOM path `som` above the last, and the last, when a field can be named by the
+    /// path: two to [`MAX_FIELD_DEPTH`] steps, none empty, and no field of the file or of this
+    /// pass already named by the path or by a part of it. `None` keeps the field's own name.
+    fn som_steps(&self, som: &str) -> Option<(Vec<String>, String)> {
+        let steps: Vec<&str> = som.split('.').collect();
+        if steps.len() < 2 || steps.len() > MAX_FIELD_DEPTH || steps.iter().any(|s| s.is_empty()) {
+            return None;
+        }
+        for i in 1..=steps.len() {
+            let path = steps.get(..i)?.join(".");
+            // A part already generated by this pass is shared; one the file has is not touched.
+            let generated = i < steps.len() && self.nodes.contains_key(&path);
+            if !generated && self.taken.contains(&path) {
+                return None;
+            }
+        }
+        let (last, above) = steps.split_last()?;
+        Some((above.iter().map(|s| s.to_string()).collect(), last.to_string()))
+    }
+
+    /// The generated field named by the SOM path of `steps`, made (with those above it) when this
+    /// pass has none yet; the topmost goes into the AcroForm's fields. `None` for no steps.
+    fn node(&mut self, steps: &[String]) -> Option<ObjRef> {
+        let mut parent: Option<(ObjRef, String)> = None;
+        for step in steps {
+            let path = match &parent {
+                Some((_, above)) => format!("{above}.{step}"),
+                None => step.clone(),
+            };
+            let r = match self.nodes.get(&path) {
+                Some((r, _)) => *r,
+                None => {
+                    let mut d = Dict::new();
+                    d.set(b"T".to_vec(), Object::String(PdfString::text(step)));
+                    d.set(b"Kids".to_vec(), Object::Array(Vec::new()));
+                    d.set(NODE_KEY.to_vec(), Object::Bool(true));
+                    if let Some((p, _)) = &parent {
+                        d.set(b"Parent".to_vec(), Object::Ref(*p));
+                    }
+                    let r = self.doc.add(Object::Dict(d));
+                    match parent.as_ref().and_then(|(_, above)| self.nodes.get_mut(above)) {
+                        Some((_, kids)) => kids.push(r),
+                        None => self.fields.push(r),
+                    }
+                    self.nodes.insert(path.clone(), (r, Vec::new()));
+                    self.taken.insert(path.clone());
+                    r
+                }
+            };
+            parent = Some((r, path));
+        }
+        parent.map(|(r, _)| r)
+    }
+
     /// An image XObject for a picture: JPEGs as they are (`DCTDecode`), PNGs, GIFs, BMPs and TIFFs
     /// decoded to samples (`FlateDecode`, with a soft mask for transparency). `None`, with a warning,
     /// for anything else.
@@ -908,6 +1011,9 @@ fn earlier_images(doc: &Document, pages: ObjRef) -> HashMap<[u8; 32], ObjRef> {
     out
 }
 
+/// Write laid-out `form` into `doc`: its pages replace the document's, its widgets join the
+/// AcroForm's fields (a box the file holds a signature for keeps it), and the AcroForm is marked
+/// laid out. What was written, with any warnings, comes back.
 pub fn write_form(doc: &mut Document, form: &Form) -> Result<Written, XfaError> {
     let catalog_ref = doc.root().ok_or_else(|| XfaError::Malformed("the document has no catalog".into()))?;
     let mut catalog = doc.get(catalog_ref).as_dict().cloned().ok_or_else(|| XfaError::Malformed("the catalog is not a dictionary".into()))?;
@@ -925,7 +1031,7 @@ pub fn write_form(doc: &mut Document, form: &Form) -> Result<Written, XfaError>
     };
     // Pictures embedded by earlier passes, from the pages this pass replaces.
     let images = earlier_images(doc, pages_ref);
-    let signed = signed_fields(doc, &catalog);
+    let (signed, taken) = existing_fields(doc, &catalog);
     let mut em = Emitter {
         doc,
         images,
@@ -935,7 +1041,17 @@ pub fn write_form(doc: &mut Document, form: &Form) -> Result<Written, XfaError>
         field_count: 0,
         warnings: Vec::new(),
         signed,
+        taken,
+        nodes: HashMap::new(),
     };
+    // Every name a field of this pass takes at the top, wherever and in whatever order it is
+    // written: a signature field named by its SOM path must not repeat one.
+    for w in form.pages.iter().flat_map(|p| &p.items).filter_map(|i| if let Item::Widget(w) = i { Some(w) } else { None }) {
+        em.taken.insert(match &w.kind {
+            WidgetKind::Radio { group, .. } => group.clone(),
+            _ => w.name.clone(),
+        });
+    }
     let mut kids = Vec::new();
     for page in &form.pages {
         kids.push(Object::Ref(em.page(page, pages_ref)?));
@@ -950,6 +1066,10 @@ pub fn write_form(doc: &mut Document, form: &Form) -> Result<Written, XfaError>
             }
         })?;
     }
+    let nodes: Vec<(ObjRef, Vec<ObjRef>)> = em.nodes.values().cloned().collect();
+    for (node, kids) in nodes {
+        em.doc.update_dict(node, |d| d.set(b"Kids".to_vec(), Object::Array(kids.iter().map(|r| Object::Ref(*r)).collect())))?;
+    }
     let Emitter { doc, fields, field_count, warnings, .. } = em;
     let count = kids.len();
     doc.update_dict(pages_ref, |d| {
@@ -1174,7 +1294,7 @@ pub fn strip(doc: &mut Document) -> Result<Stripped, XfaError> {
             out.image_fields += 1;
         }
         let private: Vec<Vec<u8>> =
-            [SOM_KEY, CLICK_KEY, IMAGE_FIELD_KEY, b"PCItems".as_slice()].iter().filter(|k| d.contains(k)).map(|k| k.to_vec()).collect();
+            [SOM_KEY, CLICK_KEY, IMAGE_FIELD_KEY, NODE_KEY, b"PCItems".as_slice()].iter().filter(|k| d.contains(k)).map(|k| k.to_vec()).collect();
         if !private.is_empty() {
             out.field_keys += private.len();
             doc.update_dict(r, |d| {

crates/xfa/src/script.rs

@@ -586,13 +586,31 @@ pub fn rerender(doc: &mut Document, tpl: &Template) -> Result<Report, XfaError>
     if form.pages.is_empty() {
         return Err(XfaError::Malformed("the template laid out to no pages".into()));
     }
-    // The AcroForm keeps only the fields PdfCraft did not generate. Any generated fields edited in
-    // this overlay (along with their appearance streams and widget kids) are freed so a replaced
-    // password widget's value or masked appearance cannot stay in the overlay.
+    // The fields an earlier pass generated give way to the new layout's, after it is written: it
+    // keeps a signature one of them holds. The AcroForm keeps every field PdfCraft did not generate.
     let Some(root) = doc.root() else { return Err(XfaError::NotXfa) };
-    if let Some((acro_ref, mut acro)) = acroform(doc) {
-        let fields = acro.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default();
-        let mut kept = Vec::new();
+    let top = |doc: &Document| -> Vec<Object> {
+        acroform(doc).and_then(|(_, acro)| acro.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned())).unwrap_or_default()
+    };
+    let generated: std::collections::HashSet<pdfcraft_cos::ObjRef> = top(doc)
+        .iter()
+        .filter_map(Object::as_ref)
+        .filter(|r| doc.get(*r).as_dict().is_some_and(|d| d.contains(crate::pdf::SOM_KEY) || d.contains(crate::pdf::NODE_KEY)))
+        .collect();
+    let written = crate::pdf::write_form(doc, &form)?;
+    if !generated.is_empty()
+        && let Some((acro_ref, mut acro)) = acroform(doc)
+    {
+        let kept: Vec<Object> = top(doc).into_iter().filter(|f| f.as_ref().is_none_or(|r| !generated.contains(&r))).collect();
+        acro.set(b"Fields".to_vec(), Object::Array(kept));
+        match acro_ref {
+            Some(r) => doc.set(r, Object::Dict(acro)),
+            None => doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(acro)))?,
+        }
+        // A generated field edited in this overlay is freed with its appearance and widget kids,
+        // so a replaced password widget's value or masked appearance cannot stay in the overlay.
+        // Signature fields, and the fields that name them by their SOM path, stay: the new
+        // layout's signature field took their value and appearance.
         let free_ap = |doc: &mut Document, d: &Dict| {
             if let Some(ap) = d.get(b"AP").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned())
                 && let Some(n) = ap.get(b"N").and_then(Object::as_ref)
@@ -601,36 +619,28 @@ pub fn rerender(doc: &mut Document, tpl: &Template) -> Result<Report, XfaError>
                 doc.free(n);
             }
         };
-        for f in fields {
-            if let Some(r) = f.as_ref()
-                && let Some(d) = doc.get(r).as_dict().cloned()
-                && d.contains(crate::pdf::SOM_KEY)
-            {
-                free_ap(doc, &d);
-                if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) {
-                    for k in kids.iter().filter_map(Object::as_ref) {
-                        if let Some(kd) = doc.get(k).as_dict().cloned() {
-                            free_ap(doc, &kd);
-                        }
-                        if doc.is_edited(k.num) {
-                            doc.free(k);
+        let signature = |d: &Dict| d.name(b"FT") == Some(b"Sig".as_slice()) || d.contains(crate::pdf::NODE_KEY);
+        for r in generated {
+            let Some(d) = doc.get(r).as_dict().cloned().filter(|d| !signature(d)) else { continue };
+            free_ap(doc, &d);
+            if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) {
+                for k in kids.iter().filter_map(Object::as_ref) {
+                    if let Some(kd) = doc.get(k).as_dict().cloned() {
+                        if signature(&kd) {
+                            continue;
                         }
+                        free_ap(doc, &kd);
+                    }
+                    if doc.is_edited(k.num) {
+                        doc.free(k);
                     }
                 }
-                if doc.is_edited(r.num) {
-                    doc.free(r);
-                }
-            } else {
-                kept.push(f);
+            }
+            if doc.is_edited(r.num) {
+                doc.free(r);
             }
         }
-        acro.set(b"Fields".to_vec(), Object::Array(kept));
-        match acro_ref {
-            Some(r) => doc.set(r, Object::Dict(acro)),
-            None => doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(acro)))?,
-        }
     }
-    let written = crate::pdf::write_form(doc, &form)?;
     let mut warnings = form.warnings;
     warnings.extend(written.warnings);
     if written.usage_rights {

crates/xfa/src/tests.rs

@@ -1508,6 +1508,8 @@ fn top_fields(doc: &Document) -> Vec<(String, pdfcraft_cos::Dict)> {
         .collect()
 }
 
+/// Choice lists, a password, a signature and pictures in the template become their kinds of
+/// widget, and their AcroForm fields and image XObjects once written.
 #[test]
 fn choice_lists_signatures_passwords_and_pictures_become_fields_and_images() {
     let form = layout_xml(&fields_template("")).unwrap();
@@ -1570,7 +1572,11 @@ fn choice_lists_signatures_passwords_and_pictures_become_fields_and_images() {
     assert_eq!(opt[0].as_string().unwrap().to_text(), "English", "same shown text and value: a plain string");
     assert_eq!(field("other").int(b"Ff"), Some((1 << 17) | (1 << 18)), "an editable combo box");
     assert_eq!(field("pin").int(b"Ff"), Some(1 << 13), "a password field");
-    assert_eq!(field("sign").name(b"FT"), Some(b"Sig".as_slice()));
+    // The signature field is named by its SOM path, under a generated field for each step above it.
+    assert!(fields.iter().all(|(name, _)| name != "sign"));
+    let named = full_names(&doc);
+    let sign = named.iter().find(|(name, _)| name == "form[0].page1[0].sign[0]").map(|(_, d)| d).expect("named by its SOM path");
+    assert_eq!(sign.name(b"FT"), Some(b"Sig".as_slice()));
     let streams: Vec<pdfcraft_cos::Stream> = doc
         .object_numbers()
         .into_iter()
@@ -3009,6 +3015,213 @@ fn a_zero_caption_reserve_sizes_the_caption_as_none_does() {
     assert!((x_of("withZero") - x_of("without")).abs() < 0.01, "as without a reserve: {} {}", x_of("withZero"), x_of("without"));
 }
 
+/// The terminal fields by full name, down the field tree.
+fn full_names(doc: &Document) -> Vec<(String, Dict)> {
+    let root = doc.get(doc.root().unwrap());
+    let acro = doc.resolve(root.as_dict().unwrap().get(b"AcroForm").unwrap());
+    let mut todo: Vec<(Object, String)> =
+        doc.resolve(acro.as_dict().unwrap().get(b"Fields").unwrap()).as_array().unwrap().iter().map(|f| (f.clone(), String::new())).collect();
+    let mut out = Vec::new();
+    while let Some((f, prefix)) = todo.pop() {
+        let d = doc.resolve(&f).as_dict().unwrap().clone();
+        let t = d.get(b"T").map(|t| doc.resolve(t)).and_then(|t| t.as_string().map(|t| t.to_text()));
+        let name = match t {
+            Some(t) if prefix.is_empty() => t,
+            Some(t) => format!("{prefix}.{t}"),
+            None => prefix,
+        };
+        match d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) {
+            Some(kids) if kids.iter().any(|k| doc.resolve(k).as_dict().is_some_and(|k| k.contains(b"T"))) => {
+                todo.extend(kids.into_iter().map(|k| (k, name.clone())));
+            }
+            _ => out.push((name, d)),
+        }
+    }
+    out
+}
+
+/// A laid-out signature field is named by its SOM path, under a generated field for each step
+/// above it, as other viewers lay a dynamic form out again and find its signature by that name.
+/// Boxes in one subform share those fields; a new layout replaces them rather than adding more;
+/// taking the XFA out leaves them as ordinary fields.
+#[test]
+fn signature_fields_are_named_by_their_som_path() {
+    let xdp = r##"<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
+<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">
+<subform name="form" layout="tb">
+ <pageSet><pageArea name="page"><contentArea x="0.5in" y="0.5in" w="7.5in" h="10in"/><medium short="8.5in" long="11in"/></pageArea></pageSet>
+ <subform name="body" layout="tb" w="7.5in">
+  <field name="name" w="3in" h="0.3in"><ui><textEdit/></ui></field>
+  <field name="sign" w="3in" h="1in"><ui><signature/></ui></field>
+  <field name="other" w="3in" h="1in"><ui><signature/></ui></field>
+ </subform>
+ <subform name="end" layout="tb" w="7.5in">
+  <field name="last" w="3in" h="1in"><ui><signature/></ui></field>
+ </subform>
+</subform>
+</template>
+</xdp:xdp>
+"##;
+    let mut doc = Document::open(Arc::new(shell(xdp))).unwrap();
+    let report = render_into(&mut doc).unwrap();
+    assert_eq!(report.fields, 4, "name and the three signature fields");
+    let doc = Document::open(Arc::new(write_incremental(&doc, &SaveOptions::default()).unwrap())).unwrap();
+    let check = |doc: &Document| {
+        let top: Vec<String> = pdfcraft_forms_free_fields(doc).into_iter().map(|(name, _)| name).collect();
+        assert_eq!(top.iter().filter(|n| *n == "form[0]").count(), 1, "one field for the root subform: {top:?}");
+        assert!(top.iter().any(|n| n == "name"), "other fields keep their names: {top:?}");
+        let named = full_names(doc);
+        for som in ["form[0].body[0].sign[0]", "form[0].body[0].other[0]", "form[0].end[0].last[0]"] {
+            let (_, d) = named.iter().find(|(name, _)| name == som).unwrap_or_else(|| panic!("{som} in {named:?}"));
+            assert_eq!(d.name(b"FT"), Some(b"Sig".as_slice()));
+            assert_eq!(fields_by_som(doc).get(som).map(String::as_str), Some(som), "the data finds it by its SOM path");
+        }
+        assert_eq!(named.len(), 4, "{named:?}");
+    };
+    check(&doc);
+    let body = |doc: &Document| {
+        let named = full_names(doc);
+        let parent = |som: &str| named.iter().find(|(name, _)| name == som).and_then(|(_, d)| d.get(b"Parent").and_then(Object::as_ref));
+        (parent("form[0].body[0].sign[0]"), parent("form[0].body[0].other[0]"))
+    };
+    let (a, b) = body(&doc);
+    assert!(a.is_some() && a == b, "the two boxes of one subform share its field");
+    // Laid out again: the same fields, not a second set.
+    let mut doc = doc;
+    let tpl = crate::script::template_of(&doc).unwrap();
+    rerender(&mut doc, &tpl).unwrap();
+    check(&doc);
+    // The XFA taken out: ordinary fields with the same names, without the private key.
+    strip(&mut doc).unwrap();
+    let named = full_names(&doc);
+    assert!(named.iter().any(|(name, _)| name == "form[0].end[0].last[0]"), "{named:?}");
+    let top = pdfcraft_forms_free_fields(&doc);
+    let node = top.iter().find(|(name, _)| name == "form[0]").map(|(_, d)| d.clone()).expect("the root subform's field");
+    assert!(!node.contains(NODE_KEY));
+}
+
+/// The terminal field named `full`, down the field tree.
+fn field_ref(doc: &Document, full: &str) -> Option<pdfcraft_cos::ObjRef> {
+    let root = doc.get(doc.root()?);
+    let acro = doc.resolve(root.as_dict()?.get(b"AcroForm")?);
+    let mut todo: Vec<(pdfcraft_cos::ObjRef, String)> =
+        doc.resolve(acro.as_dict()?.get(b"Fields")?).as_array()?.iter().filter_map(Object::as_ref).map(|r| (r, String::new())).collect();
+    while let Some((r, prefix)) = todo.pop() {
+        let d = doc.get(r).as_dict()?.clone();
+        let t = d.get(b"T").map(|t| doc.resolve(t)).and_then(|t| t.as_string().map(|t| t.to_text())).unwrap_or_default();
+        let name = if prefix.is_empty() { t } else { format!("{prefix}.{t}") };
+        if name == full {
+            return Some(r);
+        }
+        let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default();
+        todo.extend(kids.iter().filter_map(Object::as_ref).map(|k| (k, name.clone())));
+    }
+    None
+}
+
+/// A box signed on a laid-out form stays signed when the form is laid out again (as after a
+/// script adds a row): the new layout's field, named by the same SOM path, holds the same
+/// signature and is locked, and the old field is gone from the AcroForm.
+#[test]
+fn a_signature_on_a_laid_out_box_survives_a_new_layout() {
+    let mut doc = Document::open(Arc::new(shell(&fields_template("")))).unwrap();
+    render_into(&mut doc).unwrap();
+    let som = "form[0].page1[0].sign[0]";
+    let old = field_ref(&doc, som).expect("the box");
+    // Signed as signing does it: a signature dictionary as the field's value, and a picture.
+    let mut sig = Dict::new();
+    sig.set(b"Type".to_vec(), Object::name("Sig"));
+    let sig = doc.add(Object::Dict(sig));
+    let picture = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(Dict::new(), b"0 0 1 rg 0 0 10 10 re f".to_vec())));
+    doc.update_dict(old, |d| {
+        d.set(b"V".to_vec(), Object::Ref(sig));
+        let mut ap = Dict::new();
+        ap.set(b"N".to_vec(), Object::Ref(picture));
+        d.set(b"AP".to_vec(), Object::Dict(ap));
+    })
+    .unwrap();
+    let tpl = crate::script::template_of(&doc).unwrap();
+    rerender(&mut doc, &tpl).unwrap();
+    let new = field_ref(&doc, som).expect("still named by its SOM path");
+    assert_ne!(new, old, "a field of the new layout");
+    let d = doc.get(new).as_dict().unwrap().clone();
+    assert_eq!(d.get(b"V").and_then(Object::as_ref), Some(sig), "the same signature");
+    let shown = d.get(b"AP").map(|ap| doc.resolve(ap)).and_then(|ap| ap.as_dict().and_then(|ap| ap.get(b"N").and_then(Object::as_ref)));
+    assert_eq!(shown, Some(picture), "and the same picture");
+    assert!(matches!(&*doc.get(picture), Object::Stream(_)), "which is still in the file");
+    assert!(matches!(&*doc.get(sig), Object::Dict(_)), "as is the signature");
+    assert_eq!(d.int(b"Ff").unwrap_or(0) & 1, 1, "a signed box is not signed again");
+    let top = pdfcraft_forms_free_fields(&doc);
+    assert_eq!(top.iter().filter(|(name, _)| name == "form[0]").count(), 1, "the old fields are gone");
+}
+
+/// A box right in the form's root subform, signed and then laid out again, keeps its signature
+/// and picture in the file: its field sits just under the one naming it, and the old fields that
+/// give way to the new layout's (freed when this session made them) take nothing it still uses.
+#[test]
+fn a_signed_box_in_the_root_subform_keeps_its_picture_through_a_new_layout() {
+    let xdp = r#"<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/"><template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">
+<subform name="form" layout="tb"><pageSet><pageArea name="front"><contentArea x="0.5in" y="0.5in" w="7.5in" h="10in"/><medium short="8.5in" long="11in"/></pageArea></pageSet>
+<field name="sign" w="3in" h="1in"><ui><signature/></ui></field>
+</subform></template></xdp:xdp>"#;
+    let mut doc = Document::open(Arc::new(shell(xdp))).unwrap();
+    render_into(&mut doc).unwrap();
+    let som = "form[0].sign[0]";
+    let old = field_ref(&doc, som).expect("the box");
+    let sig = doc.add(Object::Dict(Dict::new()));
+    let picture = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(Dict::new(), b"0 0 1 rg 0 0 10 10 re f".to_vec())));
+    doc.update_dict(old, |d| {
+        d.set(b"V".to_vec(), Object::Ref(sig));
+        let mut ap = Dict::new();
+        ap.set(b"N".to_vec(), Object::Ref(picture));
+        d.set(b"AP".to_vec(), Object::Dict(ap));
+    })
+    .unwrap();
+    let tpl = crate::script::template_of(&doc).unwrap();
+    rerender(&mut doc, &tpl).unwrap();
+    let new = field_ref(&doc, som).expect("still named by its SOM path");
+    let d = doc.get(new).as_dict().unwrap().clone();
+    let shown = d.get(b"AP").map(|ap| doc.resolve(ap)).and_then(|ap| ap.as_dict().and_then(|ap| ap.get(b"N").and_then(Object::as_ref)));
+    assert_eq!(shown, Some(picture));
+    assert!(matches!(&*doc.get(picture), Object::Stream(_)), "the picture is still in the file");
+    assert!(matches!(&*doc.get(sig), Object::Dict(_)), "and the signature");
+}
+
+/// A field of the same pass keeps a signature field named by its SOM path from repeating its
+/// name, written before the box or after it: with a text field named `form[0]` (as a hostile
+/// template may name one), the box keeps its own name and no two fields share one.
+#[test]
+fn a_signature_field_does_not_repeat_a_name_another_field_has() {
+    let text = r#"<field name="form[0]" w="3in" h="0.3in"><ui><textEdit/></ui></field>"#;
+    let sign = r#"<field name="sign" w="3in" h="1in"><ui><signature/></ui></field>"#;
+    for (first, second) in [(text, sign), (sign, text)] {
+        let xdp = format!(
+            r##"<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
+<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">
+<subform name="form" layout="tb">
+ <pageSet><pageArea name="page"><contentArea x="0.5in" y="0.5in" w="7.5in" h="10in"/><medium short="8.5in" long="11in"/></pageArea></pageSet>
+ <subform name="body" layout="tb" w="7.5in">{first}{second}</subform>
+</subform>
+</template>
+</xdp:xdp>
+"##
+        );
+        let mut doc = Document::open(Arc::new(shell(&xdp))).unwrap();
+        render_into(&mut doc).unwrap();
+        let mut top: Vec<String> = pdfcraft_forms_free_fields(&doc).into_iter().map(|(name, _)| name).collect();
+        assert!(top.iter().any(|n| n == "sign"), "the box keeps its own name: {top:?}");
+        top.sort();
+        let len = top.len();
+        top.dedup();
+        assert_eq!(top.len(), len, "no name twice: {top:?}");
+    }
+}
+
+/// A signature box the file already holds a signature for shows that signature, the file's own
+/// objects, and is locked; the other box stays unsigned.
 #[test]
 fn a_signed_signature_box_shows_the_signature_the_file_holds() {
     let mut doc = Document::open(Arc::new(crate::fixtures::signed_shell())).unwrap();
@@ -3017,7 +3230,10 @@ fn a_signed_signature_box_shows_the_signature_the_file_holds() {
     let acro = doc.resolve(catalog.as_dict().unwrap().get(b"AcroForm").unwrap());
     let fields = doc.resolve(acro.as_dict().unwrap().get(b"Fields").unwrap()).as_array().unwrap().clone();
     let laid: Vec<_> = fields.iter().map(|f| doc.resolve(f)).filter(|f| f.as_dict().unwrap().contains(SOM_KEY)).collect();
+    // The file has a field named `form[0]` already: the unsigned box keeps its own name rather
+    // than repeat that one.
     let (signed, unsigned) = (laid[0].as_dict().unwrap(), laid[1].as_dict().unwrap());
+    assert_eq!(unsigned.get(b"T").and_then(|t| t.as_string()).map(|t| t.to_text()).as_deref(), Some("other"));
     // The signature dictionary and the picture are the file's own objects, not copies.
     let value = doc.resolve(signed.get(b"V").unwrap());
     assert_eq!(value.as_dict().unwrap().name(b"Type"), Some(&b"Sig"[..]));