M5: comment imports are all or nothing and appearance export stops where import does (comment.import-fdf-xfdf, comment.export-fdf-xfdf)

- import and import_comments work on a copy, kept only on success.
- XFDF export shares the 64 MiB appearance budget import reads over all
  comments; import refuses a normal appearance of several states (XFDF has
  no /AS to select one, ISO 32000-2 §12.5.5).
- FDF import nulls a page or page tree an appearance reaches (/Type resolved).
- Rich text: an inherited default namespace other than XFDF's is declared,
  and a namespace declaration with spaces around `=` is recognised.
- A redaction mark's /Rect grows to hold its quads, so the outline shows.
- The repair log gets one line per import (first five, then a count), and
  no longer claims the comment is drawn.
- Tests: the sixty-form chain, and the redaction outline is stroked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA

crates/engine/src/edit/run.rs

@@ -162,12 +162,22 @@ pub(crate) fn check_permission(edit: &Edit, p: &pdfcraft_cos::Permissions) -> Re
     }
 }
 
-/// What a comment import left out of comments it did import (an appearance it could not read, so
-/// the comment is drawn from its other entries), in the repair log: said, not slipped in.
+/// What a comment import left out of comments it did import (an appearance it could not read),
+/// in the repair log as one line: said, not slipped in.
 fn note_skipped(doc: &mut pdfcraft_cos::Document, report: &pdfcraft_xfdf::Report) {
-    for why in &report.skipped {
-        doc.note_repair(format!("imported comment {why}; its appearance was left out and it is drawn from its other entries"));
+    if report.skipped.is_empty() {
+        return;
     }
+    let shown: Vec<&str> = report.skipped.iter().take(5).map(String::as_str).collect();
+    let more = report.skipped.len().saturating_sub(shown.len());
+    let more = if more > 0 { format!("; and {more} more") } else { String::new() };
+    doc.note_repair(format!(
+        "imported {} comment{} without the appearance the file gave {} (PdfCraft draws it where it can; it may look different or not show): {}{more}",
+        report.skipped.len(),
+        if report.skipped.len() == 1 { "" } else { "s" },
+        if report.skipped.len() == 1 { "it" } else { "them" },
+        shown.join("; ")
+    ));
 }
 
 /// Dates and unique ids stamped onto what an edit creates.

crates/engine/src/tests.rs

@@ -11287,7 +11287,12 @@ fn imported_bookmarks_undo_in_one_step() {
 fn a_comment_imported_without_its_appearance_is_said_in_the_repair_log() {
     let xfdf = r#"<?xml version="1.0"?><xfdf xmlns="http://ns.adobe.com/xfdf/"><annots><stamp page="0" name="st" rect="10,10,110,60" icon="Draft"><appearance>@@@not base 64@@@</appearance></stamp></annots></xfdf>"#;
     let said = |s: &Session, id: DocId| {
-        s.get(id).unwrap().repair_log().iter().filter(|l| l.contains("imported comment st: the appearance is not base 64")).count()
+        s.get(id)
+            .unwrap()
+            .repair_log()
+            .iter()
+            .filter(|l| l.contains("imported 1 comment without the appearance") && l.contains("st: the appearance is not base 64"))
+            .count()
     };
     let (mut s, id) = session_with(1);
     s.apply(id, Edit::ImportData { name: "notes.xfdf".into(), bytes: Arc::new(xfdf.as_bytes().to_vec()) }).unwrap();

crates/xfdf/README.md

@@ -30,7 +30,10 @@ let report = import(&mut doc, &bytes)?;                 // XFDF, FDF, XML, CSV o
   PdfCraft always draws) and otherwise draws the comment and records the reason in
   `Report::skipped` (the engine puts it in the repair log); it refuses one over 16 MiB, 64 MiB per
   import, 64 nesting levels or 100,000 values. Export leaves out one past those limits or whose
-  rect has no area; where `/AS` selects one of several states, the selected stream is written.
+  rect has no area, and stops at the 64 MiB an import reads; where `/AS` selects one of several
+  states, the selected stream is written (and import, with no `/AS` in XFDF, refuses a normal
+  appearance of several states). Import and FDF import are all or nothing: on an error the
+  document is left as it was.
 - **FDF**: fields (with `/Kids` hierarchies) and indirect comments. Nested annotation values
   are copied under FDF object numbers; shared references and cycles remain shared. A stamp's appearance
   (`/AP`, with the forms and images it draws) is copied, as in XFDF, and an `/AP` in the file comes

crates/xfdf/src/appearance.rs

@@ -37,7 +37,11 @@ const ASCII_DATA_LIMIT: usize = 4096;
 /// XFDF has no appearance state (`/AS`), so where `/N`, `/R` or `/D` holds one stream per state
 /// only the selected state's stream is written (ISO 32000-2 §12.5.5: `/AS` picks it); a normal
 /// appearance with no selected stream has nothing to draw and nothing is written.
-pub(crate) fn encode(doc: &Document, d: &Dict) -> Option<String> {
+///
+/// `budget` is what the export may still write (decoded XML), shared by all of its comments, as
+/// import reads at most [`MAX_IMPORT_APPEARANCE_BYTES`]: an appearance past it is left out rather
+/// than written for import to skip. It is charged for what is written.
+pub(crate) fn encode(doc: &Document, d: &Dict, budget: &mut usize) -> Option<String> {
     if !has_appearance(doc, d) {
         return None;
     }
@@ -55,13 +59,15 @@ pub(crate) fn encode(doc: &Document, d: &Dict) -> Option<String> {
             }
         }
     }
-    let mut writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new() };
+    let limit = MAX_APPEARANCE_BYTES.min(*budget);
+    let mut writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new(), limit };
     writer.dict(Some(b"AP"), &ap, 0)?;
     // The checks while writing leave room for the closing tags and line breaks: the finished XML
     // must fit what the reader accepts, or the comment would arrive without its appearance.
-    if writer.out.len() > MAX_APPEARANCE_BYTES {
+    if writer.out.len() > limit {
         return None;
     }
+    *budget = budget.saturating_sub(writer.out.len());
     let b64 = base64::engine::general_purpose::STANDARD.encode(writer.out.as_bytes());
     // Line breaks every 76 characters, as base 64 text usually has them; the reader skips them.
     let mut wrapped = String::with_capacity(b64.len() + b64.len() / 76 + 1);
@@ -80,6 +86,8 @@ struct Writer<'a> {
     values: usize,
     /// The references being written, so a reference back to one of them ends the cycle.
     path: Vec<ObjRef>,
+    /// At most this much XML.
+    limit: usize,
 }
 
 impl Writer<'_> {
@@ -88,7 +96,7 @@ impl Writer<'_> {
     }
 
     fn spend(&mut self, depth: usize) -> Option<()> {
-        if depth > MAX_NESTING || self.out.len() > MAX_APPEARANCE_BYTES {
+        if depth > MAX_NESTING || self.out.len() > self.limit {
             return None;
         }
         self.values = self.values.checked_sub(1)?;
@@ -151,7 +159,7 @@ impl Writer<'_> {
             Object::Dict(d) => self.dict(key, d, depth)?,
             Object::Stream(s) => {
                 // The data plus its hexadecimal form must fit what is left of the budget.
-                if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > MAX_APPEARANCE_BYTES {
+                if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > self.limit {
                     return None;
                 }
                 self.out.push_str(&format!("<STREAM{k}>\n"));
@@ -218,7 +226,8 @@ pub(crate) enum Node {
 
 /// Read the text of an `<appearance>` element: base 64, then the XML it holds. `budget` is what
 /// the import may still read (decoded bytes) and is charged. The result is the `/AP` dictionary,
-/// with a normal appearance (`/N`) that is a stream or a dictionary of streams.
+/// with a normal appearance (`/N`) that is one stream: XFDF has no appearance state (`/AS`), so a
+/// dictionary of states would leave nothing selected to draw (ISO 32000-2 §12.5.5).
 pub(crate) fn decode(text: &str, budget: &mut usize) -> Result<Node, String> {
     let limit = MAX_APPEARANCE_BYTES.min(*budget);
     // Base 64 is 4 characters for 3 bytes: refuse a text that decodes past the limit before
@@ -250,14 +259,7 @@ pub(crate) fn decode(text: &str, budget: &mut usize) -> Result<Node, String> {
     let mut values = MAX_VALUES;
     let node = read(root, 0, &mut values)?;
     let usable = match &node {
-        Node::Dict(entries) => entries.iter().any(|(k, v)| {
-            k.as_slice() == b"N"
-                && match v {
-                    Node::Stream(..) => true,
-                    Node::Dict(states) => !states.is_empty() && states.iter().all(|(_, s)| matches!(s, Node::Stream(..))),
-                    _ => false,
-                }
-        }),
+        Node::Dict(entries) => entries.iter().any(|(k, v)| k.as_slice() == b"N" && matches!(v, Node::Stream(..))),
         _ => false,
     };
     if !usable {

crates/xfdf/src/fdf.rs

@@ -413,6 +413,10 @@ fn deep(
             copy?
         }
         Object::Array(a) => Object::Array(a.iter().map(|x| deep(src, dst, x, depth + 1, path, budget)).collect::<Result<_, _>>()?),
+        // A page or page tree the comment reaches is the file's, not the comment's: copied in, it
+        // would bring its contents along (as export, which nulls them; `resolve` bounds the hops
+        // of an indirect /Type).
+        Object::Dict(d) if d.get(b"Type").is_some_and(|t| matches!(src.resolve(t).as_name(), Some(b"Page" | b"Pages"))) => Object::Null,
         Object::Dict(d) => {
             Object::Dict(d.iter().map(|(k, v)| Ok((k.clone(), deep(src, dst, v, depth + 1, path, budget)?))).collect::<Result<_, DataError>>()?)
         }

crates/xfdf/src/lib.rs

@@ -338,8 +338,15 @@ fn tab_delimited(t: &str) -> Result<Vec<(String, Vec<String>)>, DataError> {
 }
 
 /// Import comments and/or field values from XFDF, FDF, XML, CSV or tab-delimited text
-/// (detected from the content).
+/// (detected from the content). All or nothing: on an error `doc` is left as it was.
 pub fn import(doc: &mut Document, bytes: &[u8]) -> Result<Report, DataError> {
+    let mut work = doc.clone();
+    let report = import_into(&mut work, bytes)?;
+    *doc = work;
+    Ok(report)
+}
+
+fn import_into(doc: &mut Document, bytes: &[u8]) -> Result<Report, DataError> {
     let report = if bytes.starts_with(b"%FDF") {
         import_fdf(doc, bytes)?
     } else {

crates/xfdf/src/tests_format.rs

@@ -535,6 +535,10 @@ fn unreadable_appearances_are_skipped_recorded_and_drawn_again() {
         ("a bad number", b64(r#"<DICT KEY="AP"><STREAM KEY="N"><FIXED KEY="X" VAL="nan"/><DATA ENCODING="ASCII">q Q</DATA></STREAM></DICT>"#)),
         ("a DTD", b64(r#"<!DOCTYPE d [<!ENTITY a "aaaa">]><DICT KEY="AP"><STREAM KEY="N"><DATA ENCODING="ASCII">&a;</DATA></STREAM></DICT>"#)),
         ("too deep", b64(&deep)),
+        (
+            "states with none selected",
+            b64(r#"<DICT KEY="AP"><DICT KEY="N"><STREAM KEY="On"><DATA ENCODING="ASCII">q Q</DATA></STREAM></DICT></DICT>"#),
+        ),
     ] {
         let mut dst = pages(1);
         let report = import(&mut dst, appearance_xfdf(&inner, "square").as_bytes()).unwrap_or_else(|e| panic!("{why}: {e}"));
@@ -817,7 +821,7 @@ fn an_exported_appearance_always_fits_what_import_reads() {
     let mut src = pages(1);
     content_stamp(&mut src, "big", vec![0u8; (appearance::MAX_APPEARANCE_BYTES / 2) - 4096]);
     let d = named(&src, "big");
-    if let Some(text) = appearance::encode(&src, &d) {
+    if let Some(text) = appearance::encode(&src, &d, &mut appearance::MAX_IMPORT_APPEARANCE_BYTES.clone()) {
         let mut budget = appearance::MAX_IMPORT_APPEARANCE_BYTES;
         appearance::decode(&text, &mut budget).expect("what export writes, import reads");
     }
@@ -891,6 +895,7 @@ fn the_selected_appearance_state_is_what_xfdf_carries() {
 fn a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused() {
     let chain = |forms: usize| {
         let mut src = pages(1);
+        put(&mut src, annot("Text", "first", [10.0, 10.0, 30.0, 30.0]));
         let mut inner = src.add(Object::Stream(Stream { dict: Dict::new(), raw: b"%innermost".to_vec().into() }));
         for _ in 0..forms {
             let mut xo = Dict::new();
@@ -910,18 +915,23 @@ fn a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused() {
         export_fdf(&src, true, false, "src.pdf").unwrap()
     };
     let mut dst = pages(1);
-    import(&mut dst, &chain(20)).unwrap();
+    import(&mut dst, &chain(60)).unwrap();
     let d = named(&dst, "chain");
     let mut at = dst.resolve(sub_dict(&dst, &d, b"AP").get(b"N").unwrap());
-    for level in 0..20 {
+    for level in 0..60 {
         let Object::Stream(s) = at.as_ref() else { panic!("level {level}: {at:?}") };
         let fm = sub_dict(&dst, &sub_dict(&dst, &s.dict, b"Resources"), b"XObject").get(b"Fm").cloned().unwrap_or(Object::Null);
         at = dst.resolve(&fm);
     }
     let Object::Stream(innermost) = at.as_ref() else { panic!("the innermost form was lost: {at:?}") };
     assert_eq!(innermost.raw.as_ref(), b"%innermost");
-    let err = import(&mut pages(1), &chain(80)).unwrap_err();
+    let mut dst = pages(1);
+    let before = dst.add(Object::Null);
+    let err = import(&mut dst, &chain(80)).unwrap_err();
     assert!(err.to_string().contains("nest more than"), "{err}");
+    // All or nothing: the note before the stamp, and the forms copied so far, are not left behind.
+    assert_eq!(dst.add(Object::Null).num, before.num + 1);
+    assert!(xfdf::find_by_name(&dst, "first").is_none());
 }
 
 /// A `/P` an FDF comment holds points into the FDF; it is replaced, and nothing it reaches is
@@ -1005,7 +1015,9 @@ fn an_imported_redaction_mark_is_drawn_even_when_cloudy_or_undrawable() {
         let d = named(&dst, "x");
         let n = dst.resolve(sub_dict(&dst, &d, b"AP").get(b"N").unwrap());
         let Object::Stream(n) = n.as_ref() else { panic!("{extra}: no appearance") };
-        assert_ne!(n.decoded().unwrap(), b"%nothing", "{extra}");
+        let drawn = n.decoded().unwrap();
+        assert_ne!(drawn, b"%nothing", "{extra}");
+        assert!(String::from_utf8_lossy(&drawn).contains("h S"), "{extra}: the outline is stroked: {}", String::from_utf8_lossy(&drawn));
         assert!(!d.contains(b"BE"), "{extra}");
         // The file's appearance was not read in at all.
         for num in before.num..dst.add(Object::Null).num {
@@ -1048,3 +1060,69 @@ fn a_long_dash_array_comes_back() {
     let (_, dst) = round_trip_xfdf(&src);
     assert_eq!(f64s(&dst, sub_dict(&dst, &named(&dst, "sq"), b"BS").get(b"D")), dashes);
 }
+
+/// Export stops writing appearances where import would stop reading them, so every appearance
+/// written comes back.
+#[test]
+fn export_writes_no_more_appearance_than_import_reads() {
+    let mut src = pages(1);
+    content_stamp(&mut src, "a", b"0 0 m 1 1 l S".to_vec());
+    let d = named(&src, "a");
+    let mut probe = usize::MAX;
+    let one = appearance::encode(&src, &d, &mut probe).unwrap();
+    let size = usize::MAX - probe;
+    let mut budget = size * 3 / 2;
+    assert_eq!(appearance::encode(&src, &d, &mut budget), Some(one));
+    assert_eq!(appearance::encode(&src, &d, &mut budget), None, "past what import reads");
+}
+
+/// A page an FDF stamp's appearance reaches (its /Type a reference here) is not copied in with
+/// its contents.
+#[test]
+fn an_fdf_appearance_that_reaches_a_page_copies_no_page_in() {
+    let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [2 0 R] >> >>\nendobj\n\
+2 0 obj\n<< /Type /Annot /Subtype /Stamp /Rect [10 10 60 60] /NM (s) /Page 0 /AP << /N 3 0 R >> >>\nendobj\n\
+3 0 obj\n<< /Subtype /Form /BBox [0 0 1 1] /PieceInfo 4 0 R /Length 3 >>\nstream\nq Q\nendstream\nendobj\n\
+4 0 obj\n<< /Type 6 0 R /Contents 5 0 R >>\nendobj\n\
+5 0 obj\n<< /Length 11 >>\nstream\npagecontent\nendstream\nendobj\n\
+6 0 obj\n/Page\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n";
+    let mut dst = pages(1);
+    let before = dst.add(Object::Null);
+    import(&mut dst, fdf).unwrap();
+    for num in before.num..dst.add(Object::Null).num {
+        if let Object::Stream(s) = dst.get(ObjRef::new(num, 0)).as_ref() {
+            assert_ne!(s.raw.as_ref(), b"pagecontent", "the page's contents were copied in");
+        }
+    }
+    let n = dst.resolve(sub_dict(&dst, &named(&dst, "s"), b"AP").get(b"N").unwrap());
+    let Object::Stream(n) = n.as_ref() else { panic!("no appearance") };
+    assert!(matches!(n.dict.get(b"PieceInfo"), None | Some(Object::Null)), "{:?}", n.dict.get(b"PieceInfo"));
+}
+
+/// Rich text keeps the default (XHTML) namespace it inherits from `<contents-richtext>`, and a
+/// prefix it redeclares with spaces around `=` is not declared twice.
+#[test]
+fn rich_text_keeps_an_inherited_default_namespace() {
+    let xml = r#"<?xml version="1.0"?><xfdf xmlns="http://ns.adobe.com/xfdf/" xmlns:a="urn:a" xmlns:b="urn:b"><annots>
+<text page="0" rect="10,10,30,30" name="d"><contents>hi</contents><contents-richtext xmlns="http://www.w3.org/1999/xhtml"><body><p>hi</p></body></contents-richtext></text>
+<text page="0" rect="40,10,60,30" name="w"><contents>hi</contents><contents-richtext><body xmlns="http://www.w3.org/1999/xhtml" xmlns:a = "urn:a2" a:x="1" b:y="2"><p>hi</p></body></contents-richtext></text>
+</annots></xfdf>"#;
+    let mut dst = pages(1);
+    import(&mut dst, xml.as_bytes()).unwrap();
+    let rc = text(&dst, &named(&dst, "d"), b"RC").unwrap();
+    let parsed = common::parse_form_xml(&rc).unwrap();
+    assert_eq!(parsed.root_element().tag_name().namespace(), Some("http://www.w3.org/1999/xhtml"), "{rc}");
+    let rc = text(&dst, &named(&dst, "w"), b"RC").unwrap();
+    assert!(common::parse_form_xml(&rc).is_ok(), "{rc}");
+    assert!(!rc.contains(r#"xmlns:a="urn:a""#), "{rc}");
+}
+
+/// A redaction mark whose /Rect has no area shows its quads all the same: the /Rect grows to hold
+/// them.
+#[test]
+fn a_redaction_marks_rect_holds_its_quads() {
+    let xml = r#"<?xml version="1.0"?><xfdf xmlns="http://ns.adobe.com/xfdf/"><annots><redact page="0" name="r" rect="50,50,50,50" coords="10,40,90,40,10,20,90,20"/></annots></xfdf>"#;
+    let mut dst = pages(1);
+    import(&mut dst, xml.as_bytes()).unwrap();
+    assert_eq!(f64s(&dst, named(&dst, "r").get(b"Rect")), vec![10.0, 20.0, 90.0, 50.0]);
+}

crates/xfdf/src/xfdf.rs

@@ -357,7 +357,7 @@ fn annotation_names(doc: &Document, pages: &[pdfcraft_model::Page], only: Option
 
 /// The elements inside a comment's XFDF element: its text (plain and rich), default appearance
 /// and style, ink, vertices, appearance (with `ap`) and pop-up (on page `pi`).
-fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap: bool) {
+fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap: Option<&mut usize>) {
     let rc = text_of(doc, d, b"RC");
     match text_of(doc, d, b"Contents") {
         Some(c) => {
@@ -386,7 +386,9 @@ fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap
     if let Some(ds) = text_of(doc, d, b"DS") {
         let _ = write!(out, "<defaultstyle>{}</defaultstyle>", esc(&ds));
     }
-    if ap && let Some(b64) = super::appearance::encode(doc, d) {
+    if let Some(budget) = ap
+        && let Some(b64) = super::appearance::encode(doc, d, budget)
+    {
         let _ = write!(out, "<appearance>{b64}</appearance>");
     }
     let ink = d.get(b"InkList").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default();
@@ -428,6 +430,8 @@ fn xfdf_annots(doc: &Document, out: &mut String, only: Option<&HashSet<(usize, u
     // NM of every annotation, for inreplyto. A selected annotation without one (other tools
     // leave /NM out) gets a name for this export, so a reply copied with it still finds it.
     let names = annotation_names(doc, &pages, only);
+    // What the import reads of appearances, over all of the comments.
+    let mut appearance_budget = super::appearance::MAX_IMPORT_APPEARANCE_BYTES;
     for (pi, p) in pages.iter().enumerate() {
         for (ai, a) in p.dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default().into_iter().enumerate() {
             if only.is_some_and(|only| !only.contains(&(pi, ai))) {
@@ -516,7 +520,7 @@ fn xfdf_annots(doc: &Document, out: &mut String, only: Option<&HashSet<(usize, u
             // XFDF 3.0 carries a stamp's appearance, and none for a rect with no area; a copy and
             // paste (`own`) carries every comment's, so the copy looks the same.
             let has_area = rect.len() == 4 && rect[0] != rect[2] && rect[1] != rect[3];
-            xfdf_annot_children(doc, out, &d, pi, has_area && (own || sub == b"Stamp"));
+            xfdf_annot_children(doc, out, &d, pi, (has_area && (own || sub == b"Stamp")).then_some(&mut appearance_budget));
             let _ = writeln!(out, "</{tag}>");
         }
     }
@@ -980,7 +984,10 @@ fn read_rich_text(rich: roxmltree::Node, has_contents: bool, d: &mut Dict) {
     };
     let source = rich.document().input_text();
     let Some(markup) = source.get(first.range().start..last.range().end) else { return };
-    let markup = if elements.len() == 1 { standalone(rich, markup) } else { markup.to_string() };
+    let markup = match elements.as_slice() {
+        [only] => standalone(rich, *only, markup),
+        _ => markup.to_string(),
+    };
     // Rich text is an XML document of its own; export left its declaration out.
     d.set(b"RC".to_vec(), PdfString::text(&format!("<?xml version=\"1.0\"?>{markup}")));
     if !has_contents {
@@ -988,31 +995,52 @@ fn read_rich_text(rich: roxmltree::Node, has_contents: bool, d: &mut Dict) {
     }
 }
 
-/// The one element `markup` (cut out of `rich`, a `<contents-richtext>`) as an XML document of
-/// its own: the namespace prefixes it uses but inherits from the XFDF around it are declared on
-/// it. Unchanged when it stands alone already, or when the declarations would not make it parse.
-fn standalone(rich: roxmltree::Node, markup: &str) -> String {
-    if parse_form_xml(markup).is_ok() {
-        return markup.to_string();
-    }
+/// Whether start tag `tag` declares attribute `name` (white space may come before the `=`).
+fn declares(tag: &str, name: &str) -> bool {
+    tag.match_indices(name).any(|(i, _)| {
+        let before = tag.get(..i).and_then(|b| b.chars().next_back());
+        let after = tag.get(i + name.len()..).map(str::trim_start);
+        before.is_some_and(char::is_whitespace) && after.is_some_and(|a| a.starts_with('='))
+    })
+}
+
+/// The one element `markup` (`element`, cut out of `rich`, a `<contents-richtext>`) as an XML
+/// document of its own: the namespaces it inherits from the XFDF around it are declared on it,
+/// the default one (XHTML declared on `<contents-richtext>`, say; not XFDF's own, which a
+/// standalone document has no use for) and the prefixes it uses. Unchanged when nothing is
+/// inherited, or when the declarations would not make it parse.
+fn standalone(rich: roxmltree::Node, element: roxmltree::Node, markup: &str) -> String {
     // The start tag's name ends at white space, `/` or `>`.
     let Some(name_end) = markup.get(1..).and_then(|t| t.find(|c: char| c.is_whitespace() || c == '/' || c == '>')).map(|i| i + 1) else {
         return markup.to_string();
     };
     let start_tag = markup.get(..markup.find('>').unwrap_or(markup.len())).unwrap_or(markup);
     let mut declared = String::new();
+    if let Some(uri) = element.tag_name().namespace()
+        && !markup.get(1..name_end).is_some_and(|n| n.contains(':'))
+        && uri != XFDF_NS
+        && !declares(start_tag, "xmlns")
+    {
+        let _ = write!(declared, " xmlns=\"{}\"", esc(uri));
+    }
     for ns in rich.namespaces() {
         let Some(prefix) = ns.name() else { continue };
-        if prefix == "xml" || !markup.contains(&format!("{prefix}:")) || start_tag.contains(&format!("xmlns:{prefix}=")) {
+        if prefix == "xml" || !markup.contains(&format!("{prefix}:")) || declares(start_tag, &format!("xmlns:{prefix}")) {
             continue;
         }
         let _ = write!(declared, " xmlns:{prefix}=\"{}\"", esc(ns.uri()));
     }
+    if declared.is_empty() {
+        return markup.to_string();
+    }
     let (Some(head), Some(tail)) = (markup.get(..name_end), markup.get(name_end..)) else { return markup.to_string() };
     let fixed = format!("{head}{declared}{tail}");
     if parse_form_xml(&fixed).is_ok() { fixed } else { markup.to_string() }
 }
 
+/// XFDF's own namespace (ISO 19444-1).
+const XFDF_NS: &str = "http://ns.adobe.com/xfdf/";
+
 /// Put an annotation dictionary on a page, replacing one with the same `/NM`. Returns its
 /// reference.
 /// `/F` bits an imported file may not set on a redaction mark: Invisible, Hidden, NoView — so a
@@ -1066,6 +1094,24 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result
                 }
             }
         }
+        // The outline is drawn inside /Rect: it is grown to hold every quad, so the whole area
+        // the mark removes shows (a /Rect with no area would show none of it).
+        let quads: Vec<f64> =
+            d.get(b"QuadPoints").and_then(Object::as_array).map(|q| q.iter().filter_map(Object::as_f64).collect()).unwrap_or_default();
+        if !quads.is_empty() {
+            let rect = rect_strict(doc, d.get(b"Rect"));
+            let start = rect.map(|[x0, y0, x1, y1]| [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)]);
+            let bounds = quads
+                .as_chunks::<2>()
+                .0
+                .iter()
+                .fold(start, |b, [x, y]| Some(b.map_or([*x, *y, *x, *y], |[l, bo, r, t]| [l.min(*x), bo.min(*y), r.max(*x), t.max(*y)])));
+            if let Some(b) = bounds
+                && Some(b) != rect
+            {
+                d.set(b"Rect".to_vec(), arr(&b));
+            }
+        }
     }
     // An appearance the file carries is how the comment looked where it came from, and is kept
     // (XFDF 3.0 "stamp": it takes precedence over the icon). A redaction mark is always drawn by
@@ -1163,7 +1209,18 @@ pub fn import_comments(
     fresh: Option<&str>,
     offset: Option<[f64; 2]>,
 ) -> Result<Report, DataError> {
-    import_xfdf_with(doc, text, page, fresh, offset.filter(|o| o.iter().all(|v| v.is_finite())), super::appearance::MAX_IMPORT_APPEARANCE_BYTES)
+    // Into a copy, kept only when the whole import succeeds.
+    let mut work = doc.clone();
+    let report = import_xfdf_with(
+        &mut work,
+        text,
+        page,
+        fresh,
+        offset.filter(|o| o.iter().all(|v| v.is_finite())),
+        super::appearance::MAX_IMPORT_APPEARANCE_BYTES,
+    )?;
+    *doc = work;
+    Ok(report)
 }
 
 pub(crate) fn import_xfdf(doc: &mut Document, text: &str) -> Result<Report, DataError> {

parity/acrobat-features.toml

@@ -4335,7 +4335,7 @@ status = "shipped"
 commands = ["comment.import"]
 tools = ["doc_import_data"]
 evidence = ["crates/xfdf/src/tests.rs::xfdf_round_trips_comments_and_values", "crates/xfdf/src/tests.rs::fdf_round_trips_values_and_comments", "crates/automation/tests/it/automation.rs::comments_and_form_data_travel_as_xfdf_fdf_and_text", "crates/xfdf/src/tests_format.rs::formatting_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::an_appearance_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::unreadable_appearances_are_skipped_recorded_and_drawn_again", "crates/xfdf/src/tests_format.rs::fdf_carries_appearances_with_their_resources", "crates/xfdf/src/tests_format.rs::the_intent_is_written_and_read_as_intent_or_it", "crates/xfdf/src/tests_format.rs::a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused", "crates/xfdf/src/tests_format.rs::an_imported_redaction_mark_is_drawn_even_when_cloudy_or_undrawable", "crates/xfdf/src/tests_format.rs::the_import_budget_is_shared_by_every_comment", "crates/engine/src/tests.rs::a_comment_imported_without_its_appearance_is_said_in_the_repair_log"]
-notes = "replies, pop-ups, ink, quads, line ends; same /NM replaces. XFDF formatting comes in as XFDF 3.0 names it: contents-richtext (/RC; plain text in it, or markup without <contents>, also gives /Contents), defaultstyle, width/style/dashes (a dashed border stays dashed), cloudy style and intensity, rotation, justification, fringe, leaderLength/leaderExtend/leader-offset, caption attributes, caret symbol, redaction overlay text, the intent (`intent` or `IT`). A stamp's XFDF <appearance> (base 64 of the DICT/STREAM/DATA appearance XML other tools write) is kept; an unreadable or oversized one (16 MiB each, 64 MiB per import, 64 levels, 100,000 values) is left out, said in the import report and the repair log, and the comment is drawn by PdfCraft. FDF /AP streams are kept with their resources; an FDF whose comments expand past the import budget (1,000,000 values or 64 MiB of data) or nest more than 256 levels is refused with an error and nothing is imported. A redaction mark is always drawn by PdfCraft, and one with an area it cannot draw is refused. Rich text is kept as data; when PdfCraft draws a text box itself (no appearance in the file), it draws the plain text with /DA, not the rich text's fonts, and does not turn it by /Rotate."
+notes = "replies, pop-ups, ink, quads, line ends; same /NM replaces. XFDF formatting comes in as XFDF 3.0 names it: contents-richtext (/RC; plain text in it, or markup without <contents>, also gives /Contents), defaultstyle, width/style/dashes (a dashed border stays dashed), cloudy style and intensity, rotation, justification, fringe, leaderLength/leaderExtend/leader-offset, caption attributes, caret symbol, redaction overlay text, the intent (`intent` or `IT`). A stamp's XFDF <appearance> (base 64 of the DICT/STREAM/DATA appearance XML other tools write) is kept; an unreadable or oversized one (16 MiB each, 64 MiB per import, 64 levels, 100,000 values) (or one whose normal appearance is several states, which XFDF cannot select) is left out, said in the import report and the repair log, and the comment is drawn by PdfCraft. FDF /AP streams are kept with their resources; an FDF whose comments expand past the import budget (1,000,000 values or 64 MiB of data) or nest more than 256 levels is refused with an error and nothing is imported (an import is all or nothing); a page the file's appearance reaches is not copied in. A redaction mark is always drawn by PdfCraft, and one with an area it cannot draw is refused. Rich text is kept as data; when PdfCraft draws a text box itself (no appearance in the file), it draws the plain text with /DA, not the rich text's fonts, and does not turn it by /Rotate."
 
 [[feature]]
 id = "comment.export-fdf-xfdf"