fix(xfdf): an imported FDF comment's pop-up is placed on the page, not copied in as a direct dictionary (#4493)

This commit is contained in:
Marco Arnulfo
2026-10-25 10:00:10 -04:00
committed by GitHub
parent af4a307e99
commit d9270bf16a
2 changed files with 77 additions and 13 deletions
+28 -13
View File
@@ -1126,6 +1126,24 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result<ObjRef, Da
Ok(r)
}
/// A pop-up window for the comment `parent`, on its page.
fn add_popup(doc: &mut Document, page: ObjRef, parent: ObjRef, rect: &[f64], open: bool) -> Result<(), DataError> {
let mut p = Dict::new();
p.set(b"Type".to_vec(), Object::name("Annot"));
p.set(b"Subtype".to_vec(), Object::name("Popup"));
p.set(b"Rect".to_vec(), arr(rect));
p.set(b"Parent".to_vec(), Object::Ref(parent));
p.set(b"Open".to_vec(), Object::Bool(open));
let pr = doc.add(Object::Dict(p));
doc.update_dict(parent, |d| d.set(b"Popup".to_vec(), Object::Ref(pr)))?;
doc.update_dict(page, |pd| {
if let Some(Object::Array(a)) = pd.get_mut(b"Annots") {
a.push(Object::Ref(pr));
}
})?;
Ok(())
}
fn import_xfdf(doc: &mut Document, text: &str) -> Result<Report, DataError> {
check_form_xml_depth(text)?;
let xml = parse_form_xml(text).map_err(|e| DataError::Malformed(e.to_string()))?;
@@ -1157,19 +1175,7 @@ fn import_xfdf(doc: &mut Document, text: &str) -> Result<Report, DataError> {
if let Some(pop) = node.children().find(|c| c.has_tag_name("popup"))
&& let Some(rect) = pop.attribute("rect").map(csv).filter(|r| r.len() == 4)
{
let mut p = Dict::new();
p.set(b"Type".to_vec(), Object::name("Annot"));
p.set(b"Subtype".to_vec(), Object::name("Popup"));
p.set(b"Rect".to_vec(), arr(&rect));
p.set(b"Parent".to_vec(), Object::Ref(r));
p.set(b"Open".to_vec(), Object::Bool(pop.attribute("open") == Some("yes")));
let pr = doc.add(Object::Dict(p));
doc.update_dict(r, |d| d.set(b"Popup".to_vec(), Object::Ref(pr)))?;
doc.update_dict(page, |pd| {
if let Some(Object::Array(a)) = pd.get_mut(b"Annots") {
a.push(Object::Ref(pr));
}
})?;
add_popup(doc, page, r, &rect, pop.attribute("open") == Some("yes"))?;
}
report.comments += 1;
}
@@ -1445,6 +1451,11 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result<Report, DataError> {
// The parent is another comment: link it below rather than copying it in.
let irt = src.get(b"IRT").cloned();
src.remove(b"IRT");
// The pop-up is an annotation of its own (/Popup shall be an indirect reference, ISO
// 32000-2 Table 172): copied in, it would become a direct dictionary. A new one is
// placed on the page below, as the XFDF import does.
let popup = src.get(b"Popup").map(|p| fdf.resolve(p)).and_then(|p| p.as_dict().cloned());
src.remove(b"Popup");
let mut path: Vec<ObjRef> = a.as_ref().into_iter().collect();
let Object::Dict(mut d) = deep(&fdf, &Object::Dict(src), 0, &mut path, &mut budget)? else { continue };
let Some(page) = d.get(b"Page").and_then(Object::as_int).and_then(|p| usize::try_from(p).ok()).and_then(|p| pages.get(p).copied()) else {
@@ -1457,6 +1468,10 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result<Report, DataError> {
d.set(b"P".to_vec(), Object::Ref(page));
let r = place(doc, page, d)?;
later.remove(&r);
if let Some(rect) = popup.as_ref().and_then(|p| rect_strict(&fdf, p.get(b"Rect"))) {
let open = popup.as_ref().and_then(|p| p.get(b"Open")).is_some_and(|o| matches!(&*fdf.resolve(o), Object::Bool(true)));
add_popup(doc, page, r, &rect, open)?;
}
if let Some(fr) = a.as_ref() {
placed.insert(fr, r);
}
+49
View File
@@ -1933,3 +1933,52 @@ fn annots_of(doc: &Document) -> Vec<Dict> {
.filter(|d| d.get(b"Subtype") != Some(&Object::name("Widget")))
.collect()
}
/// An FDF comment's pop-up is an annotation of its own (`/Popup` → a `/Subtype /Popup` object
/// with `/Parent`). The import copied the pop-up's dictionary into the comment as a direct value,
/// which ISO 32000-2 Table 172 says shall be an indirect reference to an annotation on the page.
/// The import now places a pop-up of its own, as the XFDF import does, keeping its place and
/// open state.
#[test]
fn an_fdf_comment_popup_is_not_copied_in_as_a_direct_dictionary() {
let fdf = fdf_with(
"/Annots [2 0 R 3 0 R]",
&[
"<< /Type /Annot /Subtype /Text /Rect [10 10 30 30] /NM (n) /Page 0 /Contents (note) /Popup 3 0 R >>".into(),
"<< /Type /Annot /Subtype /Popup /Rect [40 10 140 60] /Parent 2 0 R /Page 0 /Open true >>".into(),
],
);
let mut dst = blank();
assert_eq!(import(&mut dst, &fdf).unwrap().comments, 1);
let note = find_by_name(&dst, "n").unwrap();
let popup = dst.get(note).as_dict().unwrap().get(b"Popup").cloned();
let page = pdfcraft_model::pages(&dst)[0].dict.get(b"Annots").map(|a| dst.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap();
let pr = popup.as_ref().and_then(Object::as_ref).filter(|r| page.contains(&Object::Ref(*r)));
let pr = pr.unwrap_or_else(|| panic!("/Popup must be a reference to an annotation on the page: {popup:?}"));
let pd = dst.get(pr).as_dict().cloned().unwrap();
assert_eq!(pd.name(b"Subtype"), Some(b"Popup".as_slice()));
assert_eq!(pd.get(b"Parent"), Some(&Object::Ref(note)));
assert_eq!(pd.get(b"Open"), Some(&Object::Bool(true)));
let rect: Vec<f64> = pd.get(b"Rect").and_then(Object::as_array).unwrap().iter().filter_map(Object::as_f64).collect();
assert_eq!(rect, [40.0, 10.0, 140.0, 60.0]);
}
/// A pop-up without a usable `/Rect` cannot be placed: the comment still imports, with no
/// `/Popup` at all (neither the FDF's dictionary copied in nor a pop-up on the page).
#[test]
fn an_fdf_comment_popup_without_a_rect_is_dropped() {
let fdf = fdf_with(
"/Annots [2 0 R 3 0 R]",
&[
"<< /Type /Annot /Subtype /Text /Rect [10 10 30 30] /NM (n) /Page 0 /Contents (note) /Popup 3 0 R >>".into(),
"<< /Type /Annot /Subtype /Popup /Parent 2 0 R /Page 0 /Open true >>".into(),
],
);
let mut dst = blank();
assert_eq!(import(&mut dst, &fdf).unwrap().comments, 1);
let note = find_by_name(&dst, "n").unwrap();
assert!(dst.get(note).as_dict().unwrap().get(b"Popup").is_none());
let page = pdfcraft_model::pages(&dst)[0].dict.get(b"Annots").map(|a| dst.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap();
let popups = page.iter().filter(|a| dst.resolve(a).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Popup".as_slice()))).count();
assert_eq!(popups, 0);
}