From 4f0493cc21e512087f6e625a5ddbd5426b777d21 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:09:53 +0300 Subject: [PATCH 01/26] organize: split a combined field/widget so every copied page owns its widget --- crates/organize/src/import.rs | 90 ++++++++++++++++++++++++++++------- crates/organize/src/tests.rs | 64 ++++++++++++++++++++----- 2 files changed, 125 insertions(+), 29 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 588539866..ef18a121f 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -34,6 +34,13 @@ const SKIP_ON_ANNOT: &[&[u8]] = &[b"StructParent", b"P"]; /// objects, each shallow enough for the parser, would otherwise exhaust the stack (#1800). const MAX_COPY_DEPTH: usize = 256; +/// Keys that stay on the widget when a combined field/widget is split (#1810): the annotation's +/// appearance, placement and page. Field state (name, value, flags, default appearance) moves to +/// the new parent field, where the widgets inherit it. +const WIDGET_KEYS: &[&[u8]] = &[ + b"Type", b"Subtype", b"Rect", b"P", b"AP", b"AS", b"MK", b"BS", b"Border", b"C", b"CA", b"F", b"H", b"R", b"NM", b"M", b"StructParent", b"StructParents", b"OC", b"Tabs", +]; + struct Copier<'a> { src: &'a Document, /// Current recursion depth, bounded by [`MAX_COPY_DEPTH`]. @@ -393,27 +400,32 @@ fn import_pages_mapped( let r = dst.add(Object::Null); let c = copier.copy_value(dst, &resolved, Some(r))?; dst.set(r, c); - // `/Kids` may be an array object of its own: add to it there, so the - // field keeps its other widgets. - match dst.get(copied).as_dict().and_then(|f| f.get(b"Kids").cloned()) { - Some(Object::Ref(list)) => { - let mut kids = dst.get(list).as_array().cloned().unwrap_or_default(); - kids.push(Object::Ref(r)); - dst.set(list, Object::Array(kids)); - } - _ => dst.update_dict(copied, |f| { - let mut kids = f.get(b"Kids").and_then(|k| k.as_array()).cloned().unwrap_or_default(); - kids.push(Object::Ref(r)); - f.set(b"Kids".to_vec(), Object::Array(kids)); - })?, - } + add_kid(dst, copied, r)?; annot_pages.insert(r, new); out.push(Object::Ref(r)); continue; } - // A widget that is its own field (no parent to hang a second widget on) stays - // the one object, shown on both pages, so the field stays one. + // A widget that is its own field (no parent to hang a second widget on): on + // its first copy it is split into a parent field and a widget child (#1810); + // every later copy of the page gets one more widget child of that field, so + // the pages no longer share one widget whose `/P` names only the last page. let own_field = resolved.as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget") && !d.contains(b"Parent")); + if own_field + && let Some(first) = a.as_ref().and_then(|r| copier.map.get(&r).copied()) + && let Some(parent) = dst.get(first).as_dict().and_then(|d| d.reference(b"Parent")) + { + let r = dst.add(Object::Null); + let c = copier.copy_value(dst, &resolved, Some(r))?; + let Object::Dict(mut d) = c else { continue }; + d.set(b"Parent".to_vec(), Object::Ref(parent)); + // The new widget is not a field root of its own. + copier.fields.retain(|f| *f != r); + dst.set(r, Object::Dict(d)); + add_kid(dst, parent, r)?; + annot_pages.insert(r, new); + out.push(Object::Ref(r)); + continue; + } let annots = if own_field { &mut copier } else { own.as_mut().unwrap_or(&mut copier) }; // Direct (inline) annotation dictionaries become indirect objects, so every // copied annotation can be fixed up the same way. @@ -427,6 +439,9 @@ fn import_pages_mapped( } _ => continue, }; + if own_field && copier.fields.contains(&r) { + split_own_field(dst, &mut copier.fields, r); + } annot_pages.insert(r, new); out.push(Object::Ref(r)); } @@ -805,6 +820,49 @@ fn qualified_optional(prefix: &str, name: &str) -> String { if name.is_empty() { prefix.to_string() } else { qualified(prefix, name) } } +/// Add `kid` to `field`'s `/Kids`, in the array object itself when `/Kids` is one, so the field +/// keeps its other widgets. +fn add_kid(dst: &mut Document, field: ObjRef, kid: ObjRef) -> Result<(), OrganizeError> { + match dst.get(field).as_dict().and_then(|f| f.get(b"Kids").cloned()) { + Some(Object::Ref(list)) => { + let mut kids = dst.get(list).as_array().cloned().unwrap_or_default(); + kids.push(Object::Ref(kid)); + dst.set(list, Object::Array(kids)); + } + _ => dst.update_dict(field, |f| { + let mut kids = f.get(b"Kids").and_then(|k| k.as_array()).cloned().unwrap_or_default(); + kids.push(Object::Ref(kid)); + f.set(b"Kids".to_vec(), Object::Array(kids)); + })?, + } + Ok(()) +} + +/// PdfCraft patch (#1810): split a widget that doubles as its own field into a parent field +/// holding the field keys (name, value, flags…) and a widget child holding the annotation keys +/// (rectangle, appearance, page). The parent takes the widget's place among the top-level fields; +/// later copies of the page add widget children of it, so every page has its own widget and the +/// field stays one. +fn split_own_field(dst: &mut Document, fields: &mut [ObjRef], widget: ObjRef) { + let Some(d) = dst.get(widget).as_dict().cloned() else { return }; + let (mut field, mut child) = (Dict::new(), Dict::new()); + for (k, v) in d.iter() { + if WIDGET_KEYS.contains(&k.as_slice()) { + child.set(k.clone(), v.clone()); + } else { + field.set(k.clone(), v.clone()); + } + } + let parent = dst.add(Object::Null); + field.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(widget)])); + child.set(b"Parent".to_vec(), Object::Ref(parent)); + dst.set(parent, Object::Dict(field)); + dst.set(widget, Object::Dict(child)); + if let Some(entry) = fields.iter_mut().find(|f| **f == widget) { + *entry = parent; + } +} + /// Add top-level fields to the destination's `/AcroForm /Fields` (creating the form if needed). fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), OrganizeError> { if fields.is_empty() { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index d34b1e431..c1e20a132 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -672,9 +672,40 @@ fn links_are_rewired_to_copied_pages_or_dropped() { assert!(!link.contains(b"Dest") && !link.contains(b"A")); } -/// A widget annotation that is its own form field (no `/Parent`). -fn own_field(doc: &Document, r: ObjRef) -> bool { - doc.get(r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget") && !d.contains(b"Parent")) +/// Issue #1810: a widget that is its own field was shared between both copies of a repeated +/// page, so only the last page could identify it (its `/P`), and per-page annotation edits were +/// impossible. Each copy must get its own widget sharing one parent field (name and value). +#[test] +fn repeated_page_splits_a_combined_field_widget() { + let out = full_roundtrip(&extract_pages(&doc_a(), &[1, 1]).unwrap()); + let ps = pages(&out).unwrap(); + let widget = |i: usize| -> ObjRef { + let list = page_dict(&out, i).get(b"Annots").map(|a| out.resolve(a).as_array().cloned().unwrap_or_default()).unwrap_or_default(); + list.iter().filter_map(|a| a.as_ref()).find(|r| out.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))).expect("a widget annotation") + }; + let (w1, w2) = (widget(0), widget(1)); + assert_ne!(w1, w2, "each copy of the page has its own widget"); + for (i, w) in [(0, w1), (1, w2)] { + let d = out.get(w).as_dict().unwrap().clone(); + assert_eq!(d.reference(b"P"), Some(ps[i].obj), "/P names the page holding the widget"); + let parent = d.reference(b"Parent").expect("the widget has a parent field"); + let f = out.get(parent).as_dict().unwrap().clone(); + assert_eq!(f.get(b"T").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("name")); + assert_eq!(f.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); + let kids: Vec = f.get(b"Kids").and_then(|k| out.resolve(k).as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); + assert!(kids.contains(&w1) && kids.contains(&w2), "one field holds both widgets: {kids:?}"); + let fields: Vec = catalog(&out) + .get(b"AcroForm") + .map(|f| out.resolve(f)) + .and_then(|f| f.as_dict().cloned()) + .and_then(|f| f.get(b"Fields").map(|x| out.resolve(x))) + .and_then(|f| f.as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(|f| f.as_ref()) + .collect(); + assert_eq!(fields, [parent], "the parent is the one top-level field"); + } } #[test] @@ -690,15 +721,11 @@ fn repeated_page_gets_independent_annotations() { assert!(!refs[i].is_empty()); assert_eq!(refs[i].len(), refs[i + 1].len()); for (a, b) in refs[i].iter().zip(&refs[i + 1]) { - // A widget that is its own field stays one object on both pages, so the field stays - // one (#790); every other annotation is copied. - if !own_field(&out, *a) { - assert_ne!(a, b, "each copy of the page has its own annotation objects"); - } + assert_ne!(a, b, "each copy of the page has its own annotation objects"); } } for (i, list) in refs.iter().enumerate() { - for a in list.iter().filter(|a| !own_field(&out, **a)) { + for a in list.iter() { assert_eq!(out.get(*a).as_dict().unwrap().reference(b"P"), Some(ps[i].obj), "/P names the page holding the copy"); } } @@ -719,7 +746,9 @@ fn form_fields_come_along_and_are_registered() { assert_eq!(fields.len(), 1); let field = out.resolve(&fields[0]).as_dict().cloned().unwrap(); assert_eq!(field.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); - assert_eq!(field.reference(b"P"), Some(pages(&out).unwrap()[0].obj)); + // The widget child, not the field, names the page (#1810). + let kid = field.get(b"Kids").map(|k| out.resolve(k)).and_then(|k| k.as_array().cloned()).and_then(|k| k.first().and_then(|k| k.as_ref())).unwrap(); + assert_eq!(out.get(kid).as_dict().unwrap().reference(b"P"), Some(pages(&out).unwrap()[0].obj)); } #[test] @@ -2547,8 +2576,9 @@ fn import_renames_a_field_colliding_with_another_incoming_field() { assert_eq!(form_field_names(&dst), ["Other", "Text1", "Text1-1"]); } -/// A page listed twice shows its widget on both copies — also when the field was renamed: both -/// pages' widgets belong to the one renamed field, and no second Text1 appears. +/// A page listed twice shows its widget on both copies — also when the field was renamed: each +/// copy has its own widget, both widgets belong to the one renamed field (#1810), and no second +/// Text1 appears. #[test] fn a_repeated_page_joins_the_renamed_field() { let mut dst = doc_with_field("Text1"); @@ -2565,10 +2595,18 @@ fn a_repeated_page_joins_the_renamed_field() { .and_then(|l| l.as_array().cloned()) .unwrap(); let renamed = fields[1].clone(); + let mut widgets = Vec::new(); for page in [1, 2] { let list = page_dict(&dst, page).get(b"Annots").map(|a| dst.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap(); - assert!(list.contains(&renamed), "page {page}'s widget belongs to the renamed field"); + let widget = list.iter().find_map(|a| a.as_ref()).expect("a widget on the page"); + widgets.push(widget); + assert_eq!( + dst.get(widget).as_dict().unwrap().reference(b"Parent"), + renamed.as_ref(), + "page {page}'s widget belongs to the renamed field" + ); } + assert_ne!(widgets[0], widgets[1], "each copy has its own widget"); } /// Combining forms keeps same-named fields separate — the second file's colliding field is From 755266cf9f93b2a7cf1a221e342b733917d8f7c3 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 21 Oct 2026 18:45:04 +0500 Subject: [PATCH 02/26] cli: the argument bugs photocraft#7071 lists are live here too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of them, measured on main: pdfcraft-cli extract f.pdf --pages 1 --out --password (exit 0, and the PDF is in a file called `--password`) pdfcraft-cli check-one --edit f.pdf pdfcraft-cli: check-one: missing file pdfcraft-cli info /tmp/café.pdf (that é as one Latin-1 byte) thread 'main' panicked at library/std/src/env.rs:878:51: called `Result::unwrap()` on an `Err` value - An option whose value is a **file or folder** refuses a value that is another option, and names the way out for a file that really starts with `-`: `./--name`, as `--root` already offered. That also closes the gap the option walk left for those: an unknown option in a value position was skipped along with the option it followed, so `combine --out --nonsense` wrote `--nonsense` instead of saying what `--nonsense` was. Only those options. A password or a title can be any string, `--page` included — `an_option_shaped_password_is_not_parsed_as_a_page_flag` pins that, and a bad `--page` or `--dpi` is reported by the command in its own terms either way. - `positional` asks the option table which tokens are values, instead of skipping the one after every option. The token after a flag that takes none is an argument: `check-one --edit f.pdf` read the file as `--edit`'s operand and then reported it missing. - Arguments come through `args_os`, so one that is not UTF-8 is a usage error rather than a panic. `std::env::args` panics while collecting them, so even `--help` ended that way. tests/options.rs covers each, and each fails without its fix (the file called `--password` comes back; "check-one: missing file"; exit 101 with that report). --- apps/pdfcraft-cli/src/main.rs | 116 ++++++++++++++++++++++------- apps/pdfcraft-cli/tests/options.rs | 81 ++++++++++++++++++++ 2 files changed, 169 insertions(+), 28 deletions(-) diff --git a/apps/pdfcraft-cli/src/main.rs b/apps/pdfcraft-cli/src/main.rs index 5888e5111..3fd67e3bb 100644 --- a/apps/pdfcraft-cli/src/main.rs +++ b/apps/pdfcraft-cli/src/main.rs @@ -84,7 +84,24 @@ help and feedback: https://discord.gg/artcraft" ); fn main() -> ExitCode { - let args: Vec = std::env::args().skip(1).collect(); + // `std::env::args` panics on an argument that is not UTF-8 ("called `Result::unwrap()` on an + // `Err` value"), which a Latin-1 file name is, and it panics while collecting them: even + // `--help` ended that way. Such a name cannot be passed on either, since the paths here are + // `str`, so it is a usage error (photocraft#1108). + let mut args: Vec = Vec::new(); + for arg in std::env::args_os().skip(1) { + match arg.into_string() { + Ok(a) => args.push(a), + Err(bad) => { + let _ = writeln!( + std::io::stderr().lock(), + "pdfcraft-cli: `{}` is not valid UTF-8, so it cannot be used as an argument", + bad.to_string_lossy() + ); + return ExitCode::FAILURE; + } + } + } // ` --help` too; only right after the command, so a later `-h` (a value) still reaches it. let command = match args.get(1).map(String::as_str) { Some("--help" | "-h") => Some("--help"), @@ -169,7 +186,7 @@ fn version() -> Result<(), CliError> { /// found so scripts and CI can gate on it, and when the audit itself cannot run. fn audit(args: &[String]) -> Result<(), CliError> { validate_options("audit", args, AUDIT_OPTIONS)?; - let path = *positional(args).first().ok_or("audit: missing file")?; + let path = *positional(args, AUDIT_OPTIONS).first().ok_or("audit: missing file")?; let bytes = read(path)?; let password = flag(args, "--password"); let doc = pdfcraft_cos::Document::open_with_password(bytes.clone(), password).map_err(|e| format!("{path}: {e}"))?; @@ -203,6 +220,28 @@ struct OptionSpec { takes_value: bool, } +/// The options whose value is a file or folder. Only these refuse a value that is another +/// option: a file named `--password` is never what was meant, while a password can be anything +/// (`--password --page` is a password of `--page`, which tests/text.rs pins), and so can a title. +/// The way to name a file that does start with `-` is `./--name`, as `--root` already offered. +const PATH_VALUE_OPTIONS: [&str; 6] = ["--out", "--out-dir", "--json", "--root", "--control", "--script"]; + +/// Whether a token can be the value of a path option. Another option cannot: `--out --password` +/// used to write the PDF to a file called `--password` and exit 0 (photocraft#6944). +fn can_be_path_value(name: &str, value: Option<&str>) -> bool { + !PATH_VALUE_OPTIONS.contains(&name) || value.is_some_and(|v| !v.is_empty() && !v.starts_with("--")) +} + +/// The complaint about a path option whose value is missing or is another option. +fn needs_value(command: &str, name: &str, value: Option<&str>) -> String { + match value { + Some(v) if !v.is_empty() => { + format!("{command}: {name} needs a value and `{v}` is another option; write `./{v}` for a file of that name") + } + _ => format!("{command}: {name} needs a value"), + } +} + /// Refuse unknown options and malformed confinement flags before any file is read. fn validate_options(command: &str, args: &[String], specs: &[OptionSpec]) -> Result<(), CliError> { if specs.iter().any(|spec| spec.name == "--root") { @@ -225,9 +264,19 @@ fn validate_options(command: &str, args: &[String], specs: &[OptionSpec]) -> Res let Some(spec) = specs.iter().find(|spec| spec.name == arg) else { return Err(format!("{command}: unknown option --{name}").into()); }; - // Other missing or malformed values are left to the command, which reports them in its own - // terms (`text` and `extract` explain what a page value must be). - i += if spec.takes_value { 2 } else { 1 }; + // A file's name is not another option, which also closes the gap the walk left for + // those: an unknown option in that value position was skipped along with it. Other + // values, missing or malformed, are left to the command, which reports them in its + // own terms (`text` and `extract` explain what a page value must be). + i += if spec.takes_value { + let value = args.get(i + 1).map(String::as_str); + if !can_be_path_value(spec.name, value) { + return Err(needs_value(command, spec.name, value).into()); + } + 2 + } else { + 1 + }; } else { i += 1; } @@ -273,19 +322,24 @@ const UI_OPTIONS: &[OptionSpec] = &[OptionSpec { name: "--control", takes_value: #[cfg(feature = "mcp")] const MCP_OPTIONS: &[OptionSpec] = &[OptionSpec { name: "--root", takes_value: true }, OptionSpec { name: "--compact", takes_value: false }]; -fn positional(args: &[String]) -> Vec<&str> { +/// The arguments that are not options or their values. Which tokens are values is the spec's +/// answer: skipping the one after every option made `check-one --edit file.pdf` report "missing +/// file", the file having been read as `--edit`'s value. +fn positional<'a>(args: &'a [String], specs: &[OptionSpec]) -> Vec<&'a str> { let mut out = Vec::new(); - let mut skip = false; - for a in args { - if skip { - skip = false; - continue; + let mut i = 0; + while let Some(a) = args.get(i) { + match specs.iter().find(|spec| spec.name == a.as_str()) { + Some(spec) if spec.takes_value => i += 2, + // An option that takes no value, and an unknown one, which `validate_options` has + // already refused by the time this runs. + Some(_) => i += 1, + None if a.starts_with("--") => i += 1, + None => { + out.push(a.as_str()); + i += 1; + } } - if a.starts_with("--") { - skip = true; - continue; - } - out.push(a.as_str()); } out } @@ -296,7 +350,7 @@ fn read(path: &str) -> Result>, String> { fn info(args: &[String]) -> Result<(), CliError> { validate_options("info", args, INFO_OPTIONS)?; - let path = *positional(args).first().ok_or("info: missing file")?; + let path = *positional(args, INFO_OPTIONS).first().ok_or("info: missing file")?; let bytes = read(path)?; let password = flag(args, "--password"); let mut info = inspect(bytes.clone(), password).map_err(|e| e.to_string())?; @@ -347,7 +401,7 @@ fn info(args: &[String]) -> Result<(), CliError> { fn text(args: &[String]) -> Result<(), CliError> { validate_options("text", args, TEXT_OPTIONS)?; - let path = *positional(args).first().ok_or("text: missing file")?; + let path = *positional(args, TEXT_OPTIONS).first().ok_or("text: missing file")?; let mut selected_page = None; // Validate every supplied value before reading, retaining the first valid selection. let mut options = args.iter(); @@ -472,7 +526,13 @@ fn edit(args: &[String]) -> Result<(), CliError> { continue; } } - let value = args.get(i + 1).map(String::as_str).ok_or_else(|| format!("edit: {arg} needs a value"))?; + // `edit` parses its own options (they run in order), so `--out` is checked here in the + // same way: it wrote a PDF to a file called `--full`. + let value = args.get(i + 1).map(String::as_str); + if !can_be_path_value(arg, value) { + return Err(needs_value("edit", arg, value).into()); + } + let value = value.ok_or_else(|| format!("edit: {arg} needs a value"))?; match arg { "--out" => out = Some(value), "--password" => password = Some(value), @@ -545,7 +605,7 @@ fn file_stem(path: &str) -> String { fn combine(args: &[String]) -> Result<(), CliError> { validate_options("combine", args, COMBINE_OPTIONS)?; let out = flag(args, "--out").ok_or("combine: missing --out")?; - let inputs = positional(args); + let inputs = positional(args, COMBINE_OPTIONS); if inputs.len() < 2 { return Err("combine: give at least two input files".into()); } @@ -556,7 +616,7 @@ fn combine(args: &[String]) -> Result<(), CliError> { fn extract(args: &[String]) -> Result<(), CliError> { validate_options("extract", args, EXTRACT_OPTIONS)?; - let path = *positional(args).first().ok_or("extract: missing file")?; + let path = *positional(args, EXTRACT_OPTIONS).first().ok_or("extract: missing file")?; let out = flag(args, "--out").ok_or("extract: missing --out")?; let pages = page_list(flag(args, "--pages").ok_or("extract: missing --pages")?)?; let mut session = pdfcraft_engine::Session::new(); @@ -568,7 +628,7 @@ fn extract(args: &[String]) -> Result<(), CliError> { fn split(args: &[String]) -> Result<(), CliError> { validate_options("split", args, SPLIT_OPTIONS)?; use pdfcraft_engine::SplitBy; - let path = *positional(args).first().ok_or("split: missing file")?; + let path = *positional(args, SPLIT_OPTIONS).first().ok_or("split: missing file")?; let by = match (flag(args, "--every"), flag(args, "--before")) { (Some(n), None) => SplitBy::PageCount(n.parse().map_err(|_| "bad --every")?), (None, Some(list)) => SplitBy::Before(page_list(list)?), @@ -592,7 +652,7 @@ fn split(args: &[String]) -> Result<(), CliError> { fn render(args: &[String]) -> Result<(), CliError> { validate_options("render", args, RENDER_OPTIONS)?; - let path = *positional(args).first().ok_or("render: missing file")?; + let path = *positional(args, RENDER_OPTIONS).first().ok_or("render: missing file")?; let page: usize = flag(args, "--page").unwrap_or("1").parse().map_err(|_| "bad --page")?; let page_index = page.checked_sub(1).ok_or("--page must be at least 1")?; let dpi: f32 = flag(args, "--dpi").unwrap_or("96").parse().map_err(|_| "bad --dpi")?; @@ -646,7 +706,7 @@ fn render(args: &[String]) -> Result<(), CliError> { /// Child-process body for `check`: prints one JSON line. fn check_one(args: &[String]) -> Result<(), CliError> { validate_options("check-one", args, CHECK_ONE_OPTIONS)?; - let path = *positional(args).first().ok_or("check-one: missing file")?; + let path = *positional(args, CHECK_ONE_OPTIONS).first().ok_or("check-one: missing file")?; let dpi: f32 = flag(args, "--dpi").unwrap_or("36").parse().map_err(|_| "bad --dpi")?; let start = Instant::now(); let bytes = read(path)?; @@ -739,7 +799,7 @@ fn collect(paths: &[&str]) -> Vec { fn check(args: &[String]) -> Result<(), CliError> { validate_options("check", args, CHECK_OPTIONS)?; - let files = collect(&positional(args)); + let files = collect(&positional(args, CHECK_OPTIONS)); if files.is_empty() { return Err("check: no PDF files found".into()); } @@ -924,9 +984,9 @@ fn run(args: &[String]) -> Result<(), CliError> { } return Ok(()); } - let tool = *positional(args).first().ok_or("run: missing tool name (see `pdfcraft-cli tools`)")?; + let tool = *positional(args, RUN_OPTIONS).first().ok_or("run: missing tool name (see `pdfcraft-cli tools`)")?; let mut obj = serde_json::Map::new(); - for kv in positional(args).iter().skip(1) { + for kv in positional(args, RUN_OPTIONS).iter().skip(1) { let (k, v) = kv.split_once('=').ok_or(format!("run: expected key=value, got {kv:?}"))?; let value = serde_json::from_str(v).unwrap_or_else(|_| serde_json::Value::String(v.to_string())); obj.insert(k.to_string(), value); @@ -992,7 +1052,7 @@ fn ui(args: &[String]) -> Result<(), CliError> { // Refused rather than truncated, which would quietly reach some other port. let port = u16::try_from(port).map_err(|_| format!("{file}: {port} is not a port number"))?; let token = info["token"].as_str().ok_or(format!("{file}: no token"))?; - let pos = positional(args); + let pos = positional(args, UI_OPTIONS); let method = *pos.first().ok_or("ui: missing method (state, inspect, click, drag, type, key, command, commands, set, open, screenshot)")?; let method = if method.starts_with("ui.") { method.to_string() } else { format!("ui.{method}") }; let mut params = serde_json::Map::new(); diff --git a/apps/pdfcraft-cli/tests/options.rs b/apps/pdfcraft-cli/tests/options.rs index 9576d59a8..14b3e617c 100644 --- a/apps/pdfcraft-cli/tests/options.rs +++ b/apps/pdfcraft-cli/tests/options.rs @@ -27,3 +27,84 @@ fn non_edit_commands_reject_unknown_options_before_io() { assert!(stderr.contains("unknown option") && stderr.contains(unknown), "{args:?}: {stderr}"); } } + +/// photocraft#6944: an option took the next token whatever it was, so a line with a value missing +/// in the middle of it did something else entirely — `extract in.pdf --pages 1 --out --password` +/// wrote the PDF to a file called `--password` and exited 0. The unknown option hidden in a value +/// position passed the check above too, since the walk skipped it along with the option. +/// +/// Only the options whose value is a file or folder refuse one: a password or a title can be any +/// string, `--page` included, which `text`'s own tests pin and this does not change. +#[test] +fn a_file_option_refuses_a_value_that_is_another_option() { + let dir = std::env::temp_dir().join(format!("pdfcraft-cli-values-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("temp dir"); + for (option, args) in [ + ("--out", vec!["extract", "missing.pdf", "--pages", "1", "--out", "--password"]), + ("--out", vec!["combine", "a.pdf", "b.pdf", "--out", "--nonsense"]), + ("--out", vec!["render", "missing.pdf", "--out", "--dpi", "96"]), + ("--out-dir", vec!["split", "missing.pdf", "--every", "2", "--out-dir", "--password"]), + ("--json", vec!["check", "missing.pdf", "--json", "--dpi"]), + ("--control", vec!["ui", "--control", "--out", "state"]), + // `edit` parses its own options, in the order given, and had the same gap. + ("--out", vec!["edit", "missing.pdf", "--out", "--full"]), + ] { + let out = Command::new(BIN).current_dir(&dir).args(&args).output().expect("run cli"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!out.status.success(), "{args:?} unexpectedly succeeded"); + assert!(stderr.contains(&format!("{option} needs a value")), "{args:?}: {stderr}"); + assert!(stderr.contains("another option"), "{args:?}: {stderr}"); + } + assert_eq!(std::fs::read_dir(&dir).expect("read dir").count(), 0, "nothing was written"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// And a file whose name really does start with `-` is still reachable, the way the shell reaches +/// one and `--root` already documented: `./--name`. The error is then about the input file, which +/// is as far as these lines get here. +#[test] +fn a_file_whose_name_starts_with_dashes_is_still_reachable() { + for args in [ + vec!["extract", "missing.pdf", "--pages", "1", "--out", "./--dash.pdf"], + vec!["edit", "missing.pdf", "--out", "./--dash.pdf"], + vec!["info", "./--dash.pdf"], + ] { + let out = Command::new(BIN).args(&args).output().expect("run cli"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!out.status.success(), "{args:?}: no such file either way"); + assert!(stderr.contains("No such file"), "{args:?}: {stderr}"); + assert!(!stderr.contains("needs a value"), "{args:?}: {stderr}"); + } +} + +/// The token after an option that takes no value is not its value: `check-one --edit file.pdf` +/// reported "check-one: missing file", the file having been read as `--edit`'s operand. +#[test] +fn a_flag_that_takes_no_value_does_not_swallow_the_file() { + for args in [vec!["check-one", "--edit", "missing.pdf"], vec!["check-one", "missing.pdf", "--edit"]] { + let out = Command::new(BIN).args(&args).output().expect("run cli"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!out.status.success(), "{args:?}"); + assert!(stderr.contains("missing.pdf") && stderr.contains("No such file"), "{args:?}: {stderr}"); + assert!(!stderr.contains("missing file"), "the file was found, and then not found: {stderr}"); + } +} + +/// photocraft#1108: an argument that is not UTF-8 panicked in `std::env::args` ("called +/// `Result::unwrap()` on an `Err` value"), while collecting the arguments — so it panicked even +/// for `--help`. Such a name cannot be used here, where paths are `str`, so it is a usage error. +#[cfg(unix)] +#[test] +fn an_argument_that_is_not_utf8_is_an_error_not_a_panic() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + for args in [vec![OsStr::new("info"), OsStr::from_bytes(b"/tmp/caf\xe9.pdf")], vec![OsStr::new("--help"), OsStr::from_bytes(b"caf\xe9")]] { + let out = Command::new(BIN).args(&args).output().expect("run cli"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "an error, not a panic (101): {stderr}"); + assert!(stderr.contains("not valid UTF-8"), "{stderr}"); + assert!(!stderr.contains("panicked"), "{stderr}"); + } +} From b6b67a86d834c74230563a1d309cc6a3826bfbb9 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Thu, 22 Oct 2026 15:08:56 +0800 Subject: [PATCH 03/26] fix(automation): guard MCP dispatch and bound line and file reads (#3924) --- crates/automation/src/a11y.rs | 10 +- crates/automation/src/comments.rs | 4 +- crates/automation/src/content.rs | 4 +- crates/automation/src/file_access.rs | 6 +- crates/automation/src/forms.rs | 2 +- crates/automation/src/lib.rs | 75 +++++++++++-- crates/automation/src/mcp.rs | 154 +++++++++++++++++++++++++-- crates/automation/src/signing.rs | 4 +- 8 files changed, 225 insertions(+), 34 deletions(-) diff --git a/crates/automation/src/a11y.rs b/crates/automation/src/a11y.rs index 212454973..423e6d251 100644 --- a/crates/automation/src/a11y.rs +++ b/crates/automation/src/a11y.rs @@ -607,9 +607,9 @@ impl Automation { for p in paths { let name = std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into()); let result = - self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| std::fs::read(&src).map_err(|e| e.to_string())).and_then(|bytes| { - pdfcraft_engine::ocr::recognize_file(&name, std::sync::Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true) - }); + self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| crate::read_input_bytes(&src).map_err(|e| e.to_string())).and_then( + |bytes| pdfcraft_engine::ocr::recognize_file(&name, std::sync::Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true), + ); out.push(match result { Ok(r) => { let target = child(&folder, &name); @@ -696,7 +696,7 @@ impl Automation { let mut files = Vec::new(); for p in a.strs("paths")? { let path = self.resolve(p, false)?; - files.push((p.to_string(), std::fs::read(&path).map_err(|e| failed(format!("{p}: {e}")))?)); + files.push((p.to_string(), crate::read_input_bytes(&path).map_err(|e| failed(format!("{p}: {e}")))?)); } let csv = pdfcraft_engine::merge_data_files(&files).map_err(failed)?; let target = self.resolve(a.str("path")?, true)?; @@ -841,7 +841,7 @@ impl Automation { let result = self .resolve(p, false) .map_err(|e| e.to_string()) - .and_then(|src| std::fs::read(&src).map_err(|e| e.to_string())) + .and_then(|src| crate::read_input_bytes(&src).map_err(|e| e.to_string())) .and_then(|bytes| run_on(&action, &name, std::sync::Arc::new(bytes), |_, _| {})); out.push(match result { Ok(r) => { diff --git a/crates/automation/src/comments.rs b/crates/automation/src/comments.rs index 3c53563be..1266db010 100644 --- a/crates/automation/src/comments.rs +++ b/crates/automation/src/comments.rs @@ -385,7 +385,7 @@ impl Automation { "attachment" => { let [x, y] = a.need::<2>("at", "an attachment (its icon's top-left)")?; let path = self.resolve(a.str("path")?, false)?; - let data = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let data = crate::read_input(&path)?; let file = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let icon = match a.opt_str("icon")? { Some(n) => pdfcraft_engine::AttachIcon::from_name(n) @@ -679,7 +679,7 @@ impl Automation { let page = self.page(a)?; let info = self.doc(a)?.info.pages[page].clone(); let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; let file = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let name = a.opt_str("name")?.map(str::to_owned).unwrap_or_else(|| file.rsplit_once('.').map_or(file.clone(), |(s, _)| s.to_owned())); let [x, y] = a.need::<2>("at", "a stamp (its centre)")?; diff --git a/crates/automation/src/content.rs b/crates/automation/src/content.rs index 8926a1368..a69f56517 100644 --- a/crates/automation/src/content.rs +++ b/crates/automation/src/content.rs @@ -118,7 +118,7 @@ impl Automation { let page = self.page(a)?; let ph = self.doc(a)?.info.pages[page].height as f64; let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let rect = rect_arg(a, "rect")?.map(|r| to_display(r, ph)); let mut out = self.apply(a, Edit::AddImage { page, rect, name, bytes: std::sync::Arc::new(bytes) })?; @@ -188,7 +188,7 @@ impl Automation { } if let Some(p) = a.opt_str("image")? { let path = self.resolve(p, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); // Box and transforms first, then the new picture. self.apply(a, Edit::UpdateContent { page, index, content: content.clone() })?; diff --git a/crates/automation/src/file_access.rs b/crates/automation/src/file_access.rs index 55b10662f..4820743e2 100644 --- a/crates/automation/src/file_access.rs +++ b/crates/automation/src/file_access.rs @@ -345,7 +345,8 @@ mod tests { // Helpers that are only called with a resolved path. `doc_batch` (batch.rs) resolves its // inputs and out_dir, walks only inside them (skipping links) and resolves every // discovered file again before `process` reads it or `publish` writes it. - const RESOLVED_BY_CALLER: &[&str] = &["write_atomic", "write_atomic_with", "read_review_sidecar", "process", "linked", "walk", "publish"]; + const RESOLVED_BY_CALLER: &[&str] = + &["write_atomic", "write_atomic_with", "read_input", "read_input_bytes", "read_review_sidecar", "process", "linked", "walk", "publish"]; let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut offenders = vec![]; for entry in std::fs::read_dir(&dir).unwrap() { @@ -365,7 +366,8 @@ mod tests { || !(trimmed.contains("std::fs") || trimmed.contains("File::open") || trimmed.contains("File::create") - || trimmed.contains("OpenOptions")) + || trimmed.contains("OpenOptions") + || trimmed.contains("read_input")) { continue; } diff --git a/crates/automation/src/forms.rs b/crates/automation/src/forms.rs index 26e7b9d06..00e430b30 100644 --- a/crates/automation/src/forms.rs +++ b/crates/automation/src/forms.rs @@ -684,7 +684,7 @@ impl Automation { pub(crate) fn doc_import_data(&mut self, a: &Args) -> Result { let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let before = (self.doc(a)?.info.annotations.len(), self.doc(a)?.form.iter().filter(|f| !f.value.is_empty()).count()); let mut out = self.apply(a, Edit::ImportData { name, bytes: std::sync::Arc::new(bytes) })?; diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c90240875..f5c08ce10 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -394,7 +394,7 @@ impl Automation { "page_replace" => { let pages = self.pages(&a, "pages")?; let path = self.resolve(a.str("path")?, false)?; - let bytes = Arc::new(std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?); + let bytes = Arc::new(read_input(&path)?); let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let src_pages = match a.opt_ints("from_pages")? { Some(p) => one_based(&p)?, @@ -543,7 +543,7 @@ impl Automation { "delete" => ImageEdit::Delete, "replace" => { let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; ImageEdit::Replace { name: path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), bytes: Arc::new(bytes), @@ -766,7 +766,7 @@ impl Automation { "form_fill" => self.form_fill(&a)?, "form_set_image" => { let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; self.apply(&a, Edit::SetFieldImage { name: a.str("field")?.to_owned(), image: Arc::new(bytes) })? } "form_reset" => self.form_reset(&a)?, @@ -1000,7 +1000,7 @@ impl Automation { fn doc_open(&mut self, a: &Args) -> Result { let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let id = self.session.open(name, Some(path.to_string_lossy().into_owned()), Arc::new(bytes), a.opt_str("password")?).map_err(failed)?; let doc = self.session.get(id).ok_or_else(|| failed("the document vanished"))?; @@ -1270,7 +1270,7 @@ impl Automation { fn mark_file(&self, a: &Args) -> Result> { let Some(p) = a.opt_str("file")? else { return Ok(None) }; let path = self.resolve(p, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; let page = a.opt_int("file_page")?.unwrap_or(1).max(1) as usize - 1; Ok(Some(pdfcraft_engine::MarkFile { name: path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), @@ -1444,7 +1444,7 @@ impl Automation { let mut images = Vec::new(); for p in a.strs("paths")? { let path = self.resolve(p, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; images.push((path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), bytes)); } let name = if images.len() == 1 { @@ -1459,7 +1459,7 @@ impl Automation { (Some(t), _) => ("Text".to_string(), t.to_string()), (None, Some(p)) => { let path = self.resolve(p, false)?; - let t = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let t = read_input(&path)?; (path.file_stem().map(|s| s.to_string_lossy().into_owned()).unwrap_or_default(), pdfcraft_engine::decode_text(&t)) } (None, None) => return Err(ToolError::InvalidArgs("text needs `text` or `path`".into())), @@ -1634,7 +1634,7 @@ impl Automation { fn insert_file(&mut self, a: &Args) -> Result { let at = self.position(a, "at")?; let path = self.resolve(a.str("path")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; let pages = a.opt_ints("pages")?.map(|p| one_based(&p)).transpose()?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); self.apply(a, Edit::InsertPagesFrom { name, bytes: Arc::new(bytes), pages, at, field_conflicts: pdfcraft_organize::FieldConflicts::Rename }) @@ -1677,7 +1677,7 @@ impl Automation { // Each file is read, converted and copied in turn, so a long list holds one file at a time. let sources = paths.into_iter().zip(ranges).map(|(p, range)| -> std::result::Result { let path = self.resolve(p, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = read_input(&path)?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let (_, pdf) = self.session.convert_to_pdf(&name, &Arc::new(bytes)).map_err(failed)?; let title = path.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); @@ -1694,7 +1694,7 @@ impl Automation { let mut out = Vec::new(); for &p in paths { let converted = self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| { - let bytes = std::fs::read(&src).map_err(|e| format!("{}: {e}", src.display()))?; + let bytes = read_input_bytes(&src).map_err(|e| format!("{}: {e}", src.display()))?; let name = src.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let stem = src.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document".into()); let (kind, pdf) = self.session.convert_to_pdf(&name, &Arc::new(bytes)).map_err(|e| e.to_string())?; @@ -1798,7 +1798,7 @@ impl Automation { if let Some(g) = group { firsts.push((g, path.clone(), sources.len())); } - let bytes = Arc::new(std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?); + let bytes = Arc::new(read_input(&path)?); // Images and text files are made into PDFs first (#569), as in the Combine files tab. let file_name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); self.session.as_pdf(&file_name, &bytes).map_err(failed)? @@ -2700,6 +2700,28 @@ fn unused(dir: &Path, stem: &str) -> PathBuf { (2..10_000u32).map(|n| child(dir, &format!("{stem} ({n}).pdf"))).find(|p| !p.exists()).unwrap_or(first) } +/// Largest input file a tool reads into memory (512 MiB, matching `doc_batch`). +pub(crate) const MAX_INPUT: u64 = 512 * 1024 * 1024; + +/// Read a resolved input file of at most [`MAX_INPUT`] bytes without buffering past the cap. +pub(crate) fn read_input_bytes(path: &Path) -> std::io::Result> { + use std::io::Read as _; + let file = std::fs::File::open(path)?; + if file.metadata().is_ok_and(|m| m.is_file() && m.len() > MAX_INPUT) { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "input exceeds 512 MiB")); + } + let mut bytes = Vec::new(); + file.take(MAX_INPUT + 1).read_to_end(&mut bytes)?; + if bytes.len() as u64 > MAX_INPUT { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "input exceeds 512 MiB")); + } + Ok(bytes) +} + +pub(crate) fn read_input(path: &Path) -> Result> { + read_input_bytes(path).map_err(|e| failed(format!("{}: {e}", path.display()))) +} + /// Write and sync a temporary file in the same directory, then rename over the target. /// This flushes staged contents; it does not promise parent-directory durability after a /// power loss. Filesystems that refuse sync cause the save to fail before publication. @@ -3257,4 +3279,35 @@ mod tests { assert!(a.session.get(id).is_none()); } } + + #[test] + fn tool_file_reads_refuse_inputs_exceeding_512_mib() { + let dir = staging_dir("capped-read"); + let huge = dir.join("huge.bin"); + let file = std::fs::File::create(&huge).unwrap(); + file.set_len(MAX_INPUT + 1).unwrap(); + drop(file); + let huge_str = huge.to_string_lossy().into_owned(); + + let mut a = Automation::new(); + let id = a.session.open("a.pdf", None, Arc::new(numbered_pages(2)), None).unwrap().0; + for (tool, args) in [ + ("doc_open", json!({ "path": huge_str })), + ("page_insert_file", json!({ "doc": id, "at": 1, "path": huge_str })), + ("page_replace", json!({ "doc": id, "pages": [1], "path": huge_str })), + ("page_add_image", json!({ "doc": id, "page": 1, "path": huge_str })), + ("doc_import_data", json!({ "doc": id, "path": huge_str })), + ("doc_create", json!({ "from": "text", "path": huge_str })), + ("doc_create", json!({ "from": "images", "paths": [huge_str] })), + ("doc_combine", json!({ "paths": [huge_str, huge_str] })), + ("form_merge_data", json!({ "paths": [huge_str], "path": dir.join("out.csv").to_string_lossy() })), + ("comment_add", json!({ "doc": id, "page": 1, "type": "attachment", "at": [72.0, 72.0], "path": huge_str })), + ("stamp_custom", json!({ "doc": id, "page": 1, "at": [72.0, 72.0], "path": huge_str })), + ("sign_trust", json!({ "paths": [huge_str] })), + ] { + let err = a.call(tool, &args).unwrap_err().to_string(); + assert!(err.contains("input exceeds 512 MiB"), "{tool}: {err}"); + } + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/automation/src/mcp.rs b/crates/automation/src/mcp.rs index f5fda5be9..84cc21b25 100644 --- a/crates/automation/src/mcp.rs +++ b/crates/automation/src/mcp.rs @@ -25,6 +25,10 @@ const PARSE_ERROR: i64 = -32700; const INVALID_REQUEST: i64 = -32600; const METHOD_NOT_FOUND: i64 = -32601; const INVALID_PARAMS: i64 = -32602; +const INTERNAL_ERROR: i64 = -32603; + +/// Longest JSON-RPC request line [`McpServer::serve`] buffers into memory (16 MiB). +pub const MAX_LINE_BYTES: usize = 16 * 1024 * 1024; const INSTRUCTIONS: &str = "PdfCraft edits PDFs. Open a file with doc_open to get a document id, then inspect \ (doc_info, text_extract, text_find, page_render) or edit it (page_*, doc_set_info). Edits are undoable \ @@ -83,18 +87,36 @@ impl McpServer { } /// Serve until `input` reaches end of file. - pub fn serve(&mut self, input: impl BufRead, mut output: impl Write) -> std::io::Result<()> { - for line in input.lines() { - let line = line?; - if line.trim().is_empty() { - continue; + pub fn serve(&mut self, mut input: impl BufRead, mut output: impl Write) -> std::io::Result<()> { + let mut buf = Vec::new(); + loop { + match read_line_limited(&mut input, &mut buf, MAX_LINE_BYTES)? { + LineRead::End => return Ok(()), + LineRead::TooLong => { + drain_line(&mut input)?; + buf.clear(); + buf.shrink_to(64 * 1024); + let reply = error(Value::Null, INVALID_REQUEST, &format!("request line exceeds {MAX_LINE_BYTES} bytes")); + writeln!(output, "{reply}")?; + output.flush()?; + continue; + } + LineRead::Line => {} } - if let Some(reply) = self.handle_line(&line) { + let reply = match std::str::from_utf8(&buf) { + Ok(line) if line.trim().is_empty() => None, + Ok(line) => self.handle_line(line), + Err(e) => Some(error(Value::Null, PARSE_ERROR, &format!("parse error: {e}")).to_string()), + }; + if buf.capacity() > 64 * 1024 { + buf.clear(); + buf.shrink_to(64 * 1024); + } + if let Some(reply) = reply { writeln!(output, "{reply}")?; output.flush()?; } } - Ok(()) } /// Handle one JSON-RPC message; returns the serialized reply, if one is due. @@ -116,7 +138,7 @@ impl McpServer { }; let params = obj.get("params").cloned().unwrap_or(Value::Null); let id = id?; // notifications/initialized, notifications/cancelled, …: nothing to answer - let result = if method == "tools/call" { guarded_tool(|| self.dispatch(method, ¶ms)) } else { self.dispatch(method, ¶ms) }; + let result = guarded_dispatch(method, || self.dispatch(method, ¶ms)); Some(match result { Ok(result) => json!({ "jsonrpc": "2.0", "id": id, "result": result }), Err((code, message)) => error(id, code, &message), @@ -323,11 +345,67 @@ fn unwrap_tool_call(args: &Value) -> Result<(String, Value), String> { Ok((name.to_owned(), inner)) } +/// What [`read_line_limited`] found. +#[derive(Debug, PartialEq, Eq)] +enum LineRead { + /// A line, now in the buffer (newline and a trailing `\r` stripped). + Line, + /// End of stream. + End, + /// The line is longer than the limit; at most `max + 1` bytes of it were read. + TooLong, +} + +/// Read one `\n`-terminated line of at most `max` bytes into `buf`. +fn read_line_limited(reader: &mut impl BufRead, buf: &mut Vec, max: usize) -> std::io::Result { + use std::io::Read as _; + buf.clear(); + // `take` bounds what is buffered for one line: never more than `max + 1` bytes. + let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1); + if reader.take(limit).read_until(b'\n', buf)? == 0 { + return Ok(LineRead::End); + } + if buf.last() == Some(&b'\n') { + buf.pop(); + if buf.last() == Some(&b'\r') { + buf.pop(); + } + return Ok(LineRead::Line); + } + // No newline: either the limit was passed, or this is a last line before end of stream. + Ok(if buf.len() > max { LineRead::TooLong } else { LineRead::Line }) +} + +/// Discard the rest of an overlong line up to `\n` or end of stream without buffering it. +fn drain_line(reader: &mut impl BufRead) -> std::io::Result<()> { + loop { + let chunk = reader.fill_buf()?; + if chunk.is_empty() { + return Ok(()); + } + if let Some(pos) = chunk.iter().position(|&b| b == b'\n') { + reader.consume(pos + 1); + return Ok(()); + } + let len = chunk.len(); + reader.consume(len); + } +} + /// Last-resort boundary shared by every tools/call route, including compact dispatch. fn guarded_tool(f: impl FnOnce() -> Result) -> Result { pdfcraft_engine::guard(f).unwrap_or_else(|message| Ok(tool_error(&format!("internal error: {message}")))) } +/// Last-resort boundary for every JSON-RPC method: tool calls report panics in the tool result +/// (`isError: true`), while resource and protocol methods report a JSON-RPC internal error. +fn guarded_dispatch(method: &str, f: impl FnOnce() -> Result) -> Result { + if method == "tools/call" { + return guarded_tool(f); + } + pdfcraft_engine::guard(f).unwrap_or_else(|message| Err((INTERNAL_ERROR, format!("internal error: {message}")))) +} + fn tool_content(name: &str, content: Vec) -> Value { let failed = matches!(name, "command_batch" | "doc_batch") && content.iter().any(|c| match c { @@ -509,7 +587,12 @@ fn call_result(content: Vec) -> Value { #[cfg(test)] mod tests { - use super::{Resource, first_sentence, parse_uri}; + use std::io::{BufReader, Read as _}; + + use serde_json::{Value, json}; + + use super::{McpServer, Resource, first_sentence, parse_uri}; + use crate::Automation; #[test] fn conventions_panic_is_a_tool_error() { @@ -520,6 +603,59 @@ mod tests { assert_eq!(next["ok"], true); } + #[test] + fn guarded_dispatch_catches_resource_and_protocol_panics() { + for method in ["resources/read", "resources/list", "resources/templates/list", "initialize"] { + let (code, msg) = super::guarded_dispatch(method, || panic!("synthetic {method} failure")).unwrap_err(); + assert_eq!(code, -32603, "{method}"); + assert!(msg.contains(&format!("synthetic {method} failure")), "{msg}"); + } + let ok = super::guarded_dispatch("resources/list", || Ok(json!({"resources": []}))).unwrap(); + assert_eq!(ok["resources"], json!([])); + } + + #[test] + fn read_line_limited_and_drain_line_respect_boundaries() { + use super::{LineRead, drain_line, read_line_limited}; + let mut buf = Vec::new(); + let mut input: &[u8] = b"aaaaaaaaaa\nbb\r\ncc"; + assert_eq!(read_line_limited(&mut input, &mut buf, 10).unwrap(), LineRead::Line); + assert_eq!(buf, b"aaaaaaaaaa", "a line exactly at the limit is accepted"); + assert_eq!(read_line_limited(&mut input, &mut buf, 10).unwrap(), LineRead::Line); + assert_eq!(buf, b"bb", "CRLF is stripped"); + assert_eq!(read_line_limited(&mut input, &mut buf, 10).unwrap(), LineRead::Line); + assert_eq!(buf, b"cc", "a last line without a newline is still read"); + assert_eq!(read_line_limited(&mut input, &mut buf, 10).unwrap(), LineRead::End); + + let mut overlong: &[u8] = b"0123456789abcdef\nafter\n"; + assert_eq!(read_line_limited(&mut overlong, &mut buf, 10).unwrap(), LineRead::TooLong); + assert_eq!(buf.len(), 11, "at most max + 1 bytes are buffered"); + drain_line(&mut overlong).unwrap(); + assert_eq!(read_line_limited(&mut overlong, &mut buf, 10).unwrap(), LineRead::Line); + assert_eq!(buf, b"after", "the next line after an overlong line is read cleanly"); + } + + #[test] + fn serve_answers_non_utf8_and_overlong_lines_and_keeps_serving() { + let init = json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { "protocolVersion": "2025-06-18", "capabilities": {}, "clientInfo": { "name": "test", "version": "0" } } + }) + .to_string(); + let tail = format!("\n{init}\n"); + let stream = (&b"\xff\n"[..]).chain(std::io::repeat(b'a').take(64 * 1024 * 1024)).chain(tail.as_bytes()); + let mut out = Vec::new(); + McpServer::new(Automation::new()).serve(BufReader::new(stream), &mut out).unwrap(); + let replies: Vec = String::from_utf8(out).unwrap().lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + assert_eq!(replies.len(), 3, "{replies:?}"); + assert_eq!(replies[0]["error"]["code"], -32700); + assert_eq!(replies[1]["error"]["code"], -32600); + assert_eq!(replies[2]["id"], 1); + assert_eq!(replies[2]["result"]["serverInfo"]["name"], "pdfcraft"); + } + #[test] fn first_sentence_skips_abbreviations() { assert_eq!(first_sentence("Rotate pages. Undoable."), "Rotate pages."); diff --git a/crates/automation/src/signing.rs b/crates/automation/src/signing.rs index c7a043c89..743a36ff5 100644 --- a/crates/automation/src/signing.rs +++ b/crates/automation/src/signing.rs @@ -189,7 +189,7 @@ impl Automation { return Err(failed(format!("windows:{r}: Windows certificate store identities are only available on Windows"))); } let path = self.resolve(a.str("id")?, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; sign::pkcs12::open(&bytes, a.opt_str("password")?.unwrap_or("")).map_err(|e| match e { sign::SignError::WrongPassword => bad("the digital ID password is incorrect"), e => failed(e), @@ -320,7 +320,7 @@ impl Automation { }; for p in paths { let path = self.resolve(&p, false)?; - let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; + let bytes = crate::read_input(&path)?; let lower = p.to_ascii_lowercase(); let found = if lower.ends_with(".p12") || lower.ends_with(".pfx") { let id = sign::pkcs12::open(&bytes, a.opt_str("password")?.unwrap_or("")).map_err(failed)?; From 01744fd7eb1621188de5cce285135c9d287b421c Mon Sep 17 00:00:00 2001 From: Lombardi2003 Date: Tue, 20 Oct 2026 23:41:17 +0200 Subject: [PATCH 04/26] feat(organize): Extract pages takes typed page ranges (#3593) Extract pages only took the selected pages. Its dialog now has a Pages field for ranges such as "1-5, 8, 11-13", read like Combine files' and Print's ranges, in the order typed and each page once. Left empty (as the dialog opens), it takes the selection as before, shown as the hint. A range it can't use says why and turns Extract off; Extract and delete removes the typed pages. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs.rs | 30 ++++++++++++---- crates/ui-egui/src/files.rs | 46 ++++++++++++++++++++---- crates/ui-egui/tests/it/pickers.rs | 56 ++++++++++++++++++++++++++++-- parity/acrobat-features.toml | 4 +-- 4 files changed, 119 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 0337a8cbf..f4b2aca82 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -594,15 +594,31 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { return; } Dialog::Extract => { - let count = app.active_ids().map_or(0, |(i, _)| app.views[i].target_pages().len()); + let (selected, total) = app.active_ids().map_or((Vec::new(), 0), |(i, _)| (app.views[i].target_pages(), app.views[i].page_count())); ui.label(egui::RichText::new(tl!("Extract pages")).font(theme::semibold(18.0))); ui.add_space(8.0); - let text = if count == 1 { - tl!("1 page selected.").to_string() - } else { - crate::i18n::fmt(tl!("{n} pages selected."), &[("n", &count.to_string())]) + // Page ranges to take instead of the selection (#3593); the hint is the selection. + let hint = match crate::combine_ui::range_of(&selected, total) { + all if all.is_empty() && total > 1 => format!("1-{total}"), + all if all.is_empty() => "1".to_string(), + some => some, }; - ui.label(text); + ui.horizontal(|ui| { + let l = ui.label(tl!("Pages")); + ui.add(egui::TextEdit::singleline(&mut app.extract_draft.pages).hint_text(hint).desired_width(240.0)).labelled_by(l.id); + }); + let chosen = app.extract_draft.pages_to_extract(selected, total); + match &chosen { + Ok(pages) if pages.len() == 1 => { + ui.label(tl!("1 page selected.")); + } + Ok(pages) => { + ui.label(crate::i18n::fmt(tl!("{n} pages selected."), &[("n", &pages.len().to_string())])); + } + Err(e) => { + ui.colored_label(ui.visuals().error_fg_color, e); + } + } ui.add_space(4.0); // The name the pages are saved under (#737); separate files add " (page N)". ui.horizontal(|ui| { @@ -613,7 +629,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { ui.add(widgets::checkbox(&mut app.extract_draft.separate, tl!("Extract pages as separate files"))); ui.add_space(12.0); ui.with_layout(Layout::right_to_left(Align::Center), |ui| { - if widgets::pill_button(ui, tl!("Extract"), true).clicked() { + if ui.add_enabled_ui(chosen.is_ok(), |ui| widgets::pill_button(ui, tl!("Extract"), true)).inner.clicked() { extract_now = true; close = true; } diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index 1ec9d383f..299578649 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -166,6 +166,28 @@ pub struct ExtractDraft { /// the document's name when it opens; empty means that name too. As separate files each /// page is ` (page N).pdf`, and a single page the user renamed is `.pdf`. pub name: String, + /// The pages to extract as typed ranges, 1-based ("1-5, 8, 11-13") (#3593). Empty means the + /// selected pages (or the current one); the dialog opens with it empty. + pub pages: String, +} + +impl ExtractDraft { + /// The pages to extract (0-based, in the order typed, each once) from a document of `count` + /// pages: the typed [`Self::pages`] ranges, or `selected` when none are typed. Says what is + /// wrong with a range that can't be read or takes no page. + pub fn pages_to_extract(&self, selected: Vec, count: usize) -> Result, String> { + if self.pages.trim().is_empty() { + return Ok(selected); + } + match pdfcraft_engine::print::select_pages(count, Some(&self.pages), &[], pdfcraft_engine::print::Subset::All, false) { + Ok(p) if p.is_empty() => Err(tl!("No pages selected").to_string()), + Ok(p) => { + let mut seen = std::collections::HashSet::new(); + Ok(p.into_iter().filter(|page| seen.insert(*page)).collect()) + } + Err(e) => Err(e.to_string()), + } + } } /// `name` made safe as a file name on every platform: no path separators, reserved or control @@ -604,23 +626,35 @@ impl PdfCraftApp { } } - /// Show Extract pages, its file name filled in with the active document's (#737). + /// Show Extract pages, its file name filled in with the active document's (#737) and no + /// page ranges typed, so it takes the selected pages until some are (#3593). pub fn open_extract_dialog(&mut self) { self.extract_draft.name = self.active_ids().and_then(|(_, id)| self.session.get(id)).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default(); + self.extract_draft.pages.clear(); self.dialog = Some(crate::Dialog::Extract); } - /// Copy the selected pages (or the current page) into a new unsaved document tab. + /// Copy the pages typed in Extract pages, else the selected pages (or the current page), into + /// a new unsaved document tab. pub fn extract_selection(&mut self) { // What's typed in a form field is part of the document (#166). if !self.commit_form_typing() { return; } let Some((i, id)) = self.active_ids() else { return }; - let pages = self.views[i].target_pages(); - let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default(); let opts = self.extract_draft.clone(); + let pages = match opts.pages_to_extract(self.views[i].target_pages(), self.views[i].page_count()) { + Ok(pages) => pages, + Err(e) => { + self.notify_fmt("Couldn't extract pages: {e}", &[("e", &e)]); + return; + } + }; + // Deleting takes the pages in document order (typed ranges may run backwards). + let mut doomed = pages.clone(); + doomed.sort_unstable(); + let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default(); // The name typed in the dialog (#737), else the document's. let chosen = file_stem_from_user(&opts.name); let renamed = !chosen.is_empty() && chosen != file_stem_from_user(&stem); @@ -650,7 +684,7 @@ impl PdfCraftApp { } let unchanged = app.session.get(id).map(|d| d.edit_generation()) == generation; if unchanged && app.active_ids().map(|(_, active)| active) == Some(id) { - app.apply_edit(pdfcraft_engine::Edit::DeletePages { pages }); + app.apply_edit(pdfcraft_engine::Edit::DeletePages { pages: doomed }); } else { app.notify_tr("The pages were extracted but not deleted, because the document changed while you were choosing a folder."); } @@ -677,7 +711,7 @@ impl PdfCraftApp { if opts.delete { // Back on the original document. self.active = Some(i); - self.apply_edit(pdfcraft_engine::Edit::DeletePages { pages }); + self.apply_edit(pdfcraft_engine::Edit::DeletePages { pages: doomed }); } } diff --git a/crates/ui-egui/tests/it/pickers.rs b/crates/ui-egui/tests/it/pickers.rs index e4ce56d7c..ade218aaa 100644 --- a/crates/ui-egui/tests/it/pickers.rs +++ b/crates/ui-egui/tests/it/pickers.rs @@ -7,8 +7,8 @@ use crate::support; use egui_kittest::Harness; -use egui_kittest::kittest::Queryable; -use pdfcraft_render::PageRenderer; +use egui_kittest::kittest::{NodeT, Queryable}; +use pdfcraft_render::{PageRenderer, RenderRequest, RequestKind}; use pdfcraft_ui_egui::PdfCraftApp; /// An `n`-page document with a proper xref table. @@ -255,6 +255,58 @@ fn extract_pages_saves_under_the_file_name_typed_in_the_dialog() { let _ = std::fs::remove_dir_all(&dir); } +/// #3593: Extract pages takes page ranges typed in its Pages field, in the order typed, each page +/// once; left empty, it takes the selected pages as before. A range it can't use is refused, with +/// the reason in the dialog and the Extract button off. +#[test] +fn extract_pages_takes_the_page_ranges_typed_in_the_dialog() { + let texts = |app: &PdfCraftApp, view: usize| { + let doc = app.session.get(app.views[view].id).unwrap(); + let mut r = PageRenderer::new(doc.bytes.clone(), Default::default()); + (0..r.page_count()) + .map(|p| { + let out = r.render(RenderRequest { page: p, kind: RequestKind::Text, scale: 1.0, ..Default::default() }); + out.text.map(|t| t.plain_text().trim().to_string()).unwrap_or_default() + }) + .collect::>() + }; + let mut h = harness(&[6]); + h.state_mut().views[0].select_pages(&[1]); + assert!(h.state_mut().execute("page.extract")); + h.run_steps(2); + assert_eq!(h.state().extract_draft.pages, "", "opens on the selection"); + h.get_by_label("Pages"); + h.get_by_label_contains("1 page selected."); + // Ranges, a backward one and a repeat: in that order, each page once. + h.state_mut().extract_draft.pages = "5-6, 3-2, 6".into(); + h.run_steps(2); + h.get_by_label_contains("4 pages selected."); + h.state_mut().extract_selection(); + h.run_steps(2); + let s = h.state(); + assert_eq!(s.views.len(), 2, "the pages open in a new tab"); + assert_eq!(texts(s, 1), ["Page 5", "Page 6", "Page 3", "Page 2"]); + // A page that isn't there: the dialog says so, Extract is off and extracts nothing. + h.state_mut().active = Some(0); + h.state_mut().open_extract_dialog(); + h.state_mut().extract_draft.pages = "2, 9".into(); + h.run_steps(2); + assert!(h.get_by_label("Extract").accesskit_node().is_disabled(), "Extract is off"); + h.state_mut().extract_selection(); + h.run_steps(2); + assert_eq!(h.state().views.len(), 2, "nothing extracted"); + // Extract and delete takes the typed pages out of the document. + h.state_mut().active = Some(0); + h.state_mut().open_extract_dialog(); + h.state_mut().extract_draft.pages = "6, 1-2".into(); + h.state_mut().extract_draft.delete = true; + h.state_mut().extract_selection(); + h.run_steps(2); + let s = h.state(); + assert_eq!(texts(s, 2), ["Page 6", "Page 1", "Page 2"]); + assert_eq!(texts(s, 0), ["Page 3", "Page 4", "Page 5"]); +} + /// Left as the document's name (or emptied), the names are the ones Extract always used. #[test] fn extract_pages_keeps_the_usual_names_when_the_file_name_is_unchanged() { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 362c2aa77..f0ea5933f 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2788,8 +2788,8 @@ milestone = "M4" status = "shipped" commands = ["page.extract"] tools = ["page_extract"] -evidence = ["crates/ui-egui/tests/it/editing.rs::extract_button_copies_selected_pages_to_a_new_tab", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/organize/src/tests.rs::extract_and_split_keep_the_pdfx_output_intent_and_identification"] -notes = "Carries the PDF/X output intent and identification (#263) in a new XMP packet; the source XMP (dates, other schemas) is not copied." +evidence = ["crates/ui-egui/tests/it/editing.rs::extract_button_copies_selected_pages_to_a_new_tab", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/organize/src/tests.rs::extract_and_split_keep_the_pdfx_output_intent_and_identification", "crates/ui-egui/tests/it/pickers.rs::extract_pages_takes_the_page_ranges_typed_in_the_dialog"] +notes = "Carries the PDF/X output intent and identification (#263) in a new XMP packet; the source XMP (dates, other schemas) is not copied. The dialog's Pages field takes page ranges (\"1-5, 8, 11-13\", in the order typed, each page once) instead of the selection (#3593); page labels are not accepted there yet." [[feature]] id = "organize.extract-separate-files" From 19f40e6554415bd897c5315e4863762625d3a557 Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 22 Oct 2026 21:21:00 +0500 Subject: [PATCH 05/26] The two file tests ask which file, not how the OS says it is missing Review of #3849: both tests asserted `stderr.contains("No such file")`, which is the Unix wording. On Windows the same error reads "The system cannot find the file specified. (os error 2)", so `test (windows-latest)` failed on them. Neither test is about the wording. What they are about: - `a_flag_that_takes_no_value_does_not_swallow_the_file`: the error is about `missing.pdf`, and is not "check-one: missing file" -- the file reached the command instead of being eaten as `--edit`'s operand. - `a_file_whose_name_starts_with_dashes_is_still_reachable`: the error names the file the command could not open (`missing.pdf`, or `./--dash.pdf` for the `info` case), and is not a usage error about a value or an unknown option. Both now assert exactly that, plus the absence of our own usage phrases. Every string they match is one this CLI prints, so the OS has no say in it. --- apps/pdfcraft-cli/tests/options.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/apps/pdfcraft-cli/tests/options.rs b/apps/pdfcraft-cli/tests/options.rs index 14b3e617c..7955157e2 100644 --- a/apps/pdfcraft-cli/tests/options.rs +++ b/apps/pdfcraft-cli/tests/options.rs @@ -65,16 +65,19 @@ fn a_file_option_refuses_a_value_that_is_another_option() { /// is as far as these lines get here. #[test] fn a_file_whose_name_starts_with_dashes_is_still_reachable() { - for args in [ - vec!["extract", "missing.pdf", "--pages", "1", "--out", "./--dash.pdf"], - vec!["edit", "missing.pdf", "--out", "./--dash.pdf"], - vec!["info", "./--dash.pdf"], + // The error is about the file the command could not open, so the test asks which file it + // names -- not how the OS words "not found", which differs on Windows (#3849 review). + for (names, args) in [ + ("missing.pdf", vec!["extract", "missing.pdf", "--pages", "1", "--out", "./--dash.pdf"]), + ("missing.pdf", vec!["edit", "missing.pdf", "--out", "./--dash.pdf"]), + ("./--dash.pdf", vec!["info", "./--dash.pdf"]), ] { let out = Command::new(BIN).args(&args).output().expect("run cli"); let stderr = String::from_utf8_lossy(&out.stderr); assert!(!out.status.success(), "{args:?}: no such file either way"); - assert!(stderr.contains("No such file"), "{args:?}: {stderr}"); - assert!(!stderr.contains("needs a value"), "{args:?}: {stderr}"); + assert!(stderr.contains(names), "{args:?}: the error is about {names}: {stderr}"); + // Not a usage error: the `./--dash.pdf` reached the command as a file name. + assert!(!stderr.contains("needs a value") && !stderr.contains("unknown option"), "{args:?}: {stderr}"); } } @@ -86,8 +89,10 @@ fn a_flag_that_takes_no_value_does_not_swallow_the_file() { let out = Command::new(BIN).args(&args).output().expect("run cli"); let stderr = String::from_utf8_lossy(&out.stderr); assert!(!out.status.success(), "{args:?}"); - assert!(stderr.contains("missing.pdf") && stderr.contains("No such file"), "{args:?}: {stderr}"); + // Which file the error is about, not how the OS says it is missing (#3849 review). + assert!(stderr.contains("missing.pdf"), "{args:?}: the error is about the file: {stderr}"); assert!(!stderr.contains("missing file"), "the file was found, and then not found: {stderr}"); + assert!(!stderr.contains("needs a value"), "{args:?}: {stderr}"); } } From 5856c12eb3748ff1e1be936695aa8014896a08c2 Mon Sep 17 00:00:00 2001 From: ffishman <20211677+ffishman@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:55:44 -0300 Subject: [PATCH 06/26] fix(sign): judge a page's indirect /Annots array by the annotations it gains A page whose /Annots is an object of its own keeps its dictionary when a later revision adds a signature widget or a comment: only the array changes. That array fell through to "other changes", so countersigning such a document (as many signing services do) turned every earlier signature invalid, including PdfCraft's own second signature. A changed array that was a signed page's /Annots is now classified like an inline /Annots: by the annotations it gained (signature, form fill or comments), under the same DocMDP rules. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/sign/src/pdf.rs | 12 +++++++++++ crates/sign/tests/it/pdf.rs | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 82a801dee..cd555a989 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -1265,6 +1265,11 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod // attacker-written version carries (relabelling a signed page /Type /Sig, or dropping its // /Type and adding /ModDate, must not reclassify it away from page content). let signed_pages: HashSet = pdfcraft_annot::page_refs(&old).unwrap_or_default().into_iter().collect(); + // The signed pages' /Annots arrays that are objects of their own. Adding an annotation updates + // such an array in place (the page dictionary does not change), so it is judged like a + // page's inline /Annots: by the annotations it gained. + let signed_annots: HashSet = + signed_pages.iter().filter_map(|pg| old.get(*pg).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref))).collect(); // What each signed page draws from, by the role it held when signed (#1787). let signed_resources = signed_page_resources(&old, &signed_pages); let old_info = old.trailer().get(b"Info").and_then(Object::as_ref); @@ -1333,6 +1338,13 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod (Some(k), _) => k, // The Document Security Store's own arrays and dictionaries, when they only grew. (_, _) if dss.contains(&num) => kind::SECURITY_STORE, + // A signed page's indirect /Annots array, still an array: what it gained. + (_, Object::Array(now)) if signed_annots.contains(&r) && before.as_deref().is_some_and(|b| b.as_array().is_some()) => { + let was: HashSet = + before.as_deref().and_then(Object::as_array).map(|a| a.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); + let added: Vec = now.iter().filter_map(Object::as_ref).filter(|r| !was.contains(r)).collect(); + added_annots(doc, &added) + } (_, Object::Dict(d)) => { let was = before.as_deref().and_then(Object::as_dict); if signed_pages.contains(&r) { diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 61517b53d..cd0b713a8 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -139,6 +139,47 @@ fn signing_an_existing_field_and_counter_signing() { )); } +/// A page whose /Annots is an object of its own: signing and commenting update that array in +/// place, not the page. A second signature is a signature change for the first, as with an +/// inline /Annots, and a comment is still a comment (refused under a form-fill certification). +#[test] +fn an_indirect_annots_array_is_judged_by_the_annotations_it_gains() { + let page = pdfcraft_cos::ObjRef { num: 3, generation: 0 }; + let unsigned = edit_after(&fixture(), |doc| { + let annots = doc.add(Object::Array(vec![Object::Ref(pdfcraft_cos::ObjRef { num: 6, generation: 0 })])); + doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Ref(annots))).unwrap(); + }); + let annots_ref = |bytes: &[u8]| open(bytes).get(page).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref)); + let annots = annots_ref(&unsigned).unwrap(); + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let first = pdfcraft_sign::sign(&open(&unsigned), &id, &opts()).unwrap(); + let second = pdfcraft_sign::sign(&open(&first), &id, &SignOptions { rect: Some([20.0, 160.0, 220.0, 210.0]), ..opts() }).unwrap(); + assert_eq!(annots_ref(&second), Some(annots), "the signer kept the page's array indirect"); + let doc = open(&second); + let sigs = signatures(&doc, &second, &trust); + let s = sigs.iter().find(|s| s.field == "Signature1").unwrap(); + assert_eq!(s.modification, Modification::Allowed(vec![kind::SIGNATURE]), "{:?}", s.details); + assert_eq!(s.status, Status::Valid); + // A comment appended to the indirect array. + let comment = |doc: &mut Document| { + let mut d = pdfcraft_cos::Dict::new(); + d.set(b"Type".to_vec(), Object::name("Annot")); + d.set(b"Subtype".to_vec(), Object::name("Text")); + d.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(10), Object::Int(10), Object::Int(30), Object::Int(30)])); + let r = doc.add(Object::Dict(d)); + let mut a = doc.get(annots).as_array().cloned().unwrap(); + a.push(Object::Ref(r)); + doc.set(annots, Object::Array(a)); + }; + let check = |bytes: &[u8]| signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed).unwrap(); + let s = check(&edit_after(&first, comment)); + assert_eq!((s.status, s.modification), (Status::Valid, Modification::Allowed(vec![kind::COMMENTS]))); + let certified = pdfcraft_sign::sign(&open(&unsigned), &id, &SignOptions { certify: Some(2), ..opts() }).unwrap(); + let s = check(&edit_after(&certified, comment)); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::COMMENTS]))); +} + /// Issue #1328: a signature field whose /V is null, directly or through a reference, is unsigned: /// it is listed as unsigned and can be signed by name. #[test] From 4ac5165a8671cbf37283ebd41faeabdc6a3e066b Mon Sep 17 00:00:00 2001 From: ffishman <20211677+ffishman@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:57:31 -0300 Subject: [PATCH 07/26] fix(sign): let an LTV update declare the ESIC extension in the catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PAdES LTV writers add the /DSS and, in the same revision, declare the ESIC developer extension in the catalog's /Extensions (ISO 32000-2 §7.12). Any catalog change outside a short key list was "document structure", so the signature such an update protects showed as invalid. /Extensions may now gain declarations after signing; one that was signed may still not change or go. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/sign/src/pdf.rs | 18 +++++++++++++++++- crates/sign/tests/it/pdf.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 82a801dee..1e03c6ff3 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -1685,6 +1685,18 @@ fn page_change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Di } } +/// Whether the catalog `d` keeps every developer extension the signed catalog `b` declared, each +/// with the same value: it may only declare more. +fn extensions_kept(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> bool { + let now = d.get(b"Extensions").map(|e| doc.resolve(e)).and_then(|e| e.as_dict().cloned()); + let was = b.get(b"Extensions").map(|e| old.resolve(e)).and_then(|e| e.as_dict().cloned()); + match (was, now) { + (None, _) => true, + (Some(was), Some(now)) => was.iter().all(|(k, v)| now.get(k) == Some(v)), + (Some(_), None) => false, + } +} + fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) -> &'static str { let ty = d.name(b"Type"); // A later revision cannot make an existing object a signature by relabelling it: a signature @@ -1694,9 +1706,13 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) // Only a new signature value or policy: rewriting a signed one is not a signature (#1788). Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams") if before.is_none() => return kind::SIGNATURE, Some(b"Catalog") => { - let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering"]; + let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering", b"Extensions"]; return match before { Some(b) if without(d, keys) != without(b, keys) => kind::STRUCTURE, + // Developer extensions (ISO 32000-2 §7.12) only declare what the file uses. A + // writer adding a /DSS also declares the ESIC extension it belongs to, so a + // declaration may be added, but none that was signed may change or go. + Some(b) if !extensions_kept(doc, old, d, b) => kind::STRUCTURE, Some(b) => { if d.get(b"AcroForm") != b.get(b"AcroForm") { // Compare the forms themselves, inline or not. diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 61517b53d..0b0f91654 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -544,6 +544,35 @@ fn cached_discovery_still_validates_current_timestamp_bytes_ranges_and_trust() { assert_eq!(check(&doc, &tampered, &trusted).status, Status::Invalid); } +#[test] +fn ltv_writers_may_declare_the_esic_extension_but_not_change_a_signed_one() { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let check = |bytes: &[u8]| signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed).unwrap(); + let ext = |base: &[u8], level: i64| { + let mut esic = pdfcraft_cos::Dict::new(); + esic.set(b"BaseVersion".to_vec(), Object::name("1.7")); + esic.set(b"ExtensionLevel".to_vec(), Object::Int(level)); + let mut e = pdfcraft_cos::Dict::new(); + e.set(b"ESIC".to_vec(), Object::Dict(esic)); + edit_after(base, |doc| { + let root = doc.root().unwrap(); + doc.update_dict(root, |c| c.set(b"Extensions".to_vec(), Object::Dict(e))).unwrap(); + }) + }; + // As PAdES LTV writers save it: the store and the ESIC declaration in one revision. + let signed = pdfcraft_sign::sign(&open(&fixture()), &id, &opts()).unwrap(); + let evidence = pdfcraft_sign::dss::Evidence { certs: vec![id.certificate.raw.clone()], ocsps: Vec::new(), crls: Vec::new() }; + let ltv = pdfcraft_sign::dss::embed(&open(&ext(&signed, 2)), &evidence).unwrap(); + let s = check(<v); + assert_eq!(s.status, Status::Valid, "{:?}", s.details); + assert!(matches!(&s.modification, Modification::Allowed(k) if !k.contains(&kind::STRUCTURE)), "{:?}", s.modification); + // A declaration that was signed may not change afterwards. + let signed_with = pdfcraft_sign::sign(&open(&ext(&fixture(), 2)), &id, &opts()).unwrap(); + let s = check(&ext(&signed_with, 3)); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::STRUCTURE]))); +} + #[test] fn embedding_ltv_evidence_adds_a_dss_and_keeps_signatures_valid() { use pdfcraft_sign::dss::{self, Evidence}; From c829a1ad44cb4128d68b71895238ae1e8941219d Mon Sep 17 00:00:00 2001 From: ffishman <20211677+ffishman@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:03:37 -0300 Subject: [PATCH 08/26] fix(sign): a removed annotation is not hidden behind an added signature Review follow-up: an indirect /Annots array that drops a signed annotation while gaining a signature widget was judged by the widget alone, so a form-fill certification let a signed comment be deleted. A removed comment is now a comment change, and a removed widget is a change no permission covers. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/sign/src/pdf.rs | 17 ++++++++++++++--- crates/sign/tests/it/pdf.rs | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index cd555a989..0cd58d7f9 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -1338,12 +1338,23 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod (Some(k), _) => k, // The Document Security Store's own arrays and dictionaries, when they only grew. (_, _) if dss.contains(&num) => kind::SECURITY_STORE, - // A signed page's indirect /Annots array, still an array: what it gained. + // A signed page's indirect /Annots array, still an array: what it lost, then what it + // gained. Dropping a signed annotation is never hidden behind an added signature. (_, Object::Array(now)) if signed_annots.contains(&r) && before.as_deref().is_some_and(|b| b.as_array().is_some()) => { let was: HashSet = before.as_deref().and_then(Object::as_array).map(|a| a.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); - let added: Vec = now.iter().filter_map(Object::as_ref).filter(|r| !was.contains(r)).collect(); - added_annots(doc, &added) + let kept: HashSet = now.iter().filter_map(Object::as_ref).collect(); + let removed: Vec = was.iter().filter(|r| !kept.contains(r)).copied().collect(); + if removed.is_empty() { + let added: Vec = now.iter().filter_map(Object::as_ref).filter(|r| !was.contains(r)).collect(); + added_annots(doc, &added) + } else if removed.iter().any(|r| old.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))) { + // A form field or signature widget taken off the page: no permission covers it. + kind::OTHER + } else { + // Deleting comments is a comment change (permitted at P 3 only). + kind::COMMENTS + } } (_, Object::Dict(d)) => { let was = before.as_deref().and_then(Object::as_dict); diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index cd0b713a8..b3a025fc2 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -178,6 +178,30 @@ fn an_indirect_annots_array_is_judged_by_the_annotations_it_gains() { let certified = pdfcraft_sign::sign(&open(&unsigned), &id, &SignOptions { certify: Some(2), ..opts() }).unwrap(); let s = check(&edit_after(&certified, comment)); assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::COMMENTS]))); + // Under that certification, a signed comment swapped for a new signature widget is still a + // deleted comment, and a form field taken off the page is permitted by no level. + let commented = pdfcraft_sign::sign(&open(&edit_after(&unsigned, comment)), &id, &SignOptions { certify: Some(2), ..opts() }).unwrap(); + let swap = |drop: &dyn Fn(&Document, &Object) -> bool| { + edit_after(&commented, |doc| { + let mut w = pdfcraft_cos::Dict::new(); + w.set(b"Type".to_vec(), Object::name("Annot")); + w.set(b"Subtype".to_vec(), Object::name("Widget")); + w.set(b"FT".to_vec(), Object::name("Sig")); + w.set(b"T".to_vec(), PdfString::text("Later")); + w.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(0); 4])); + let w = doc.add(Object::Dict(w)); + let mut a = doc.get(annots).as_array().cloned().unwrap(); + a.retain(|o| !drop(doc, o)); + a.push(Object::Ref(w)); + doc.set(annots, Object::Array(a)); + }) + }; + let subtype = + |doc: &Document, o: &Object, t: &[u8]| o.as_ref().is_some_and(|r| doc.get(r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(t))); + let s = check(&swap(&|doc, o| subtype(doc, o, b"Text"))); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::COMMENTS]))); + let s = check(&swap(&|doc, o| subtype(doc, o, b"Widget") && o.as_ref().is_some_and(|r| r.num == 6))); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER]))); } /// Issue #1328: a signature field whose /V is null, directly or through a reference, is unsigned: From 5f042c691f3456cd7c601db782a3b5a54b861cfd Mon Sep 17 00:00:00 2001 From: Juan Jurado Date: Thu, 22 Oct 2026 16:45:39 -0600 Subject: [PATCH 09/26] M10: every paper size reaches the printer, from the dialog and doc_print (#3752) A CUPS job that names no paper gets the queue's PPD default (A4 on the reporter's Xerox queue), whatever the sheets were laid out on. Two gaps remained after #1583: - cups_media had no name for 13 of the 21 PAPERS sizes (A6, ARCH A-E1, ANSI C-E, A0-A2), so the Print dialog sent no media for them. They now use PWG 5101.1 names (the IANA-registered na_arch-e1_30x42in for ARCH E1), which carry their dimensions: CUPS feeds the driver's size of those dimensions whatever the driver calls it, and driverless IPP printers use them natively. Checked with cupsfilter against a synthetic PPD whose sizes use vendor keywords (ARCHD, AnsiD, A0...): each name produced its size's bounding box. The 8 sizes that already worked keep their PPD keywords. - doc_print to a printer (CLI run, MCP) sent no paper at all. It now resolves the paper to its PAPERS index and sends the same media option as the dialog, after the driver options. The job is built in spool_job so it can be tested without printing. print::media_option is the one place both get the option from. Both new tests were seen failing without the fix. The macOS tray half of #3752 (Auto Select instead of the PPD's default tray) is a craft-print change. --- ROADMAP.md | 1 + crates/automation/src/printing.rs | 111 ++++++++++++++++++++---------- crates/print/src/lib.rs | 50 ++++++++++---- crates/print/src/tests.rs | 28 ++++++++ crates/ui-egui/src/print_ui.rs | 2 +- parity/acrobat-features.toml | 4 +- 6 files changed, 145 insertions(+), 51 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 27c8e1b6b..ac34f9ae0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -190,6 +190,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-22 (M10, #3752 paper):** CUPS jobs ask for the chosen paper in every size. The 13 sizes the Print dialog sent no paper for (A6, A0–A2, ARCH, ANSI C–E) use PWG 5101.1 names, checked against a synthetic PPD with `cupsfilter`, and `doc_print` to a printer sends the paper too. The macOS tray half (Auto Select instead of the PPD's default tray) is a craft-print change that needs a pin bump here. Estimates unchanged. - **2026-10-22 (M14, macOS association recovery candidate):** Successful desktop open/save/sign can queue recovery of an existing per-file binding to the running PDFCraft bundle through NSWorkspace. Metadata-only watcher changes retain undo and avoid false overwrite prompts; real same-length/same-mtime edits remain protected. Rebased on 0.12.0 and reuses #3042's shared atomic metadata preservation; the bounded recovery parser does not narrow save limits. Prior native Preview TEST-signature and Finder workflows passed on macOS 27.0.1; other versions remain unqualified. Estimates unchanged. - **2026-10-13 (M7, free image rotation):** A selected page image has a rotate knob (Shift: 15 degree steps) and `image_edit` a `rotate_by` action; the angle is written as one undoable `cm`. Snapping to page edges while moving is in #1740; resize snapping is not done. Estimates unchanged. diff --git a/crates/automation/src/printing.rs b/crates/automation/src/printing.rs index 39cd45760..e43c3c6e9 100644 --- a/crates/automation/src/printing.rs +++ b/crates/automation/src/printing.rs @@ -26,6 +26,54 @@ fn bad(m: impl Into) -> ToolError { ToolError::InvalidArgs(m.into()) } +/// `doc_print`'s `paper`, by its name or a short form ("Letter", "ARCH D"), as its `PAPERS` +/// index and size; US Letter when none is given. +fn paper_named(name: Option<&str>) -> Result<(usize, (f64, f64))> { + let mut papers = PAPERS.iter().enumerate(); + let found = match name { + None => papers.next(), + Some(p) => papers.find(|(_, (n, _))| { + n.eq_ignore_ascii_case(p) + || n.replace("US ", "").eq_ignore_ascii_case(p) + || n.split(" (").next().is_some_and(|name| name.eq_ignore_ascii_case(p)) + }), + }; + found.map(|(index, (_, size))| (index, *size)).ok_or_else(|| { + bad(format!( + "unknown paper {:?}; choose one of {}", + name.unwrap_or_default(), + PAPERS.iter().map(|(name, _)| *name).collect::>().join(", ") + )) + }) +} + +/// The spooler job for `doc_print` to `printer`. It feeds the paper the PDF was laid out on +/// (`PAPERS[paper]`, as `media` after the driver's `options`) rather than the queue's default +/// paper, which a job naming none got (A4 under Letter sheets on the queue in #3752). +fn spool_job(a: &Args, printer: &str, title: String, duplex: Option, paper: usize) -> Result { + let options = match a.get("options") { + None => Vec::new(), + Some(Value::Object(m)) => m + .iter() + .map(|(k, v)| match v.as_str() { + Some(v) => Ok((k.clone(), v.to_string())), + None => Err(bad(format!("options.{k} must be a string (a choice from printer_options)"))), + }) + .collect::>>()?, + Some(_) => return Err(bad("options must be an object of option keys and choices from printer_options")), + }; + Ok(spool::Job { + printer: if printer == "default" { None } else { Some(printer.to_string()) }, + copies: a.opt_int("copies")?.unwrap_or(1).clamp(1, 999) as u32, + collate: a.opt_bool("collate")?.unwrap_or(true), + duplex, + scaling: spool::Scaling::Exact, + grayscale: a.opt_bool("grayscale")?.unwrap_or(false), + title, + options: options.into_iter().chain(print::media_option(paper)).collect(), + }) +} + impl Automation { pub(crate) fn printers(&self) -> Result { let list: Vec = spool::printers() @@ -125,20 +173,7 @@ impl Automation { "form-fields-only" => Content::FormFieldsOnly, c => return Err(bad(format!("unknown comments_forms {c:?}"))), }; - let paper = match a.opt_str("paper")? { - None => PAPERS[0].1, - Some(p) => PAPERS - .iter() - .find(|(n, _)| { - n.eq_ignore_ascii_case(p) - || n.replace("US ", "").eq_ignore_ascii_case(p) - || n.split(" (").next().is_some_and(|name| name.eq_ignore_ascii_case(p)) - }) - .map(|x| x.1) - .ok_or_else(|| { - bad(format!("unknown paper {p:?}; choose one of {}", PAPERS.iter().map(|(name, _)| *name).collect::>().join(", "))) - })?, - }; + let (paper_index, paper) = paper_named(a.opt_str("paper")?)?; // Print as image (Acrobat's Advanced Print Setup): pictures at `dpi`, grey when // `grayscale` is also asked for. let raster = if a.opt_bool("as_image")?.unwrap_or(false) { @@ -162,27 +197,7 @@ impl Automation { out["path"] = json!(path.to_string_lossy()); } (None, Some(printer)) => { - let name = self.doc(a)?.name.clone(); - let job = spool::Job { - printer: if printer == "default" { None } else { Some(printer.to_string()) }, - copies: a.opt_int("copies")?.unwrap_or(1).clamp(1, 999) as u32, - collate: a.opt_bool("collate")?.unwrap_or(true), - duplex, - scaling: spool::Scaling::Exact, - grayscale: a.opt_bool("grayscale")?.unwrap_or(false), - title: name, - options: match a.get("options") { - None => Vec::new(), - Some(Value::Object(m)) => m - .iter() - .map(|(k, v)| match v.as_str() { - Some(v) => Ok((k.clone(), v.to_string())), - None => Err(bad(format!("options.{k} must be a string (a choice from printer_options)"))), - }) - .collect::>>()?, - Some(_) => return Err(bad("options must be an object of option keys and choices from printer_options")), - }, - }; + let job = spool_job(a, printer, self.doc(a)?.name.clone(), duplex, paper_index)?; out["job"] = json!(spool::submit(&bytes, &job).map_err(|e| failed(e.to_string()))?); } _ => return Err(bad("pass either path (save the print-ready PDF) or printer (a name from printers, or \"default\")")), @@ -207,4 +222,30 @@ mod tests { assert!(duplex("long-edge", true).is_err()); assert!(duplex("sideways", false).is_err()); } + + /// #3752: `doc_print` to a printer sent no paper, so the queue's default paper stood in for + /// the one the sheets were laid out on (A4 under Letter sheets on the report's queue). + #[test] + fn printing_to_a_printer_feeds_the_laid_out_paper() { + let job = |args: Value, paper_name: Option<&str>| { + let (index, _) = paper_named(paper_name).unwrap(); + spool_job(&Args(&args), "Office", "a.pdf".into(), None, index).unwrap() + }; + let media = |m: &str| ("media".to_string(), m.to_string()); + // US Letter when no paper is asked for, after the driver's own options. + let letter = job(json!({ "options": { "InputSlot": "Tray3" } }), None); + assert_eq!(letter.options, [("InputSlot".to_string(), "Tray3".to_string()), media("Letter")]); + assert_eq!((letter.printer.as_deref(), letter.title.as_str()), (Some("Office"), "a.pdf")); + // Every paper, by its full name, goes out as its own media name. + for (i, (name, size)) in PAPERS.iter().enumerate() { + assert_eq!(paper_named(Some(name)).ok(), Some((i, *size)), "{name}"); + assert_eq!(job(json!({}), Some(name)).options, print::media_option(i).into_iter().collect::>(), "{name}"); + } + // Short forms find the same paper. + assert_eq!(job(json!({}), Some("letter")).options, [media("Letter")]); + assert_eq!(job(json!({}), Some("ARCH D")).options, [media("arch_d_24x36in")]); + assert!(matches!(paper_named(Some("Quarto")), Err(ToolError::InvalidArgs(m)) if m.contains("US Letter"))); + assert_eq!(spool_job(&Args(&json!({})), "default", String::new(), None, 0).ok().map(|j| j.printer), Some(None)); + assert!(spool_job(&Args(&json!({ "options": { "InputSlot": 3 } })), "Office", String::new(), None, 0).is_err()); + } } diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 6839e5f6b..80b7be27a 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -307,20 +307,44 @@ pub const PAPERS: [(&str, (f64, f64)); 21] = [ pub const LABEL_PAPER: usize = 19; /// The CUPS media name for `PAPERS[index]`, so the spooler feeds the sheet the PDF was laid out -/// on rather than the queue's default (PPD keywords; `w288h432` is the usual 4×6 label, `Env10` -/// the US #10 envelope). +/// on rather than the queue's default; `None` only past the end of `PAPERS`. The common sizes +/// use their PPD keywords (`w288h432` is the usual 4×6 label, `Env10` the US #10 envelope). +/// The rest use PWG 5101.1 names (the IANA registry for ARCH E1), which carry their dimensions: +/// CUPS feeds the PPD size of those dimensions whatever the driver calls it (`ARCHD`, `AnsiD`), +/// and driverless IPP printers use these names themselves (#3752). pub fn cups_media(index: usize) -> Option<&'static str> { - match PAPERS.get(index)?.0 { - "US Letter" => Some("Letter"), - "US Legal" => Some("Legal"), - "Tabloid" => Some("Tabloid"), - "A3" => Some("A3"), - "A4" => Some("A4"), - "A5" => Some("A5"), - "US #10 Envelope (4.125 x 9.5 in)" => Some("Env10"), - _ if index == LABEL_PAPER => Some("w288h432"), - _ => None, - } + Some(match PAPERS.get(index)?.0 { + "US Letter" => "Letter", + "US Legal" => "Legal", + "Tabloid" => "Tabloid", + "A3" => "A3", + "A4" => "A4", + "A5" => "A5", + "A6" => "iso_a6_105x148mm", + "ARCH A (9 x 12 in)" => "arch_a_9x12in", + "ARCH B (12 x 18 in)" => "arch_b_12x18in", + "ARCH C (18 x 24 in)" => "arch_c_18x24in", + "ARCH D (24 x 36 in)" => "arch_d_24x36in", + "ARCH E1 (30 x 42 in)" => "na_arch-e1_30x42in", + "ARCH E (36 x 48 in)" => "arch_e_36x48in", + "ANSI C (17 x 22 in)" => "na_c_17x22in", + "ANSI D (22 x 34 in)" => "na_d_22x34in", + "ANSI E (34 x 44 in)" => "na_e_34x44in", + "A2" => "iso_a2_420x594mm", + "A1" => "iso_a1_594x841mm", + "A0" => "iso_a0_841x1189mm", + "US #10 Envelope (4.125 x 9.5 in)" => "Env10", + _ if index == LABEL_PAPER => "w288h432", + // A size added to PAPERS without a name here: its tests fail, and meanwhile the queue's + // paper stands, as it did for every size before #1583. + _ => return None, + }) +} + +/// The job option that feeds `PAPERS[index]` (`media`, see [`cups_media`]), for the Print +/// dialog's jobs and `doc_print`'s alike. +pub fn media_option(index: usize) -> Option<(String, String)> { + cups_media(index).map(|m| ("media".to_string(), m.to_string())) } /// Whether most of page `index`'s text runs bottom-to-top as the page displays (a label laid diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index 99634f1aa..a17adef15 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -674,6 +674,34 @@ fn paper_matching_accepts_rounded_metric_sizes_but_not_custom_or_invalid_sizes() } } +#[test] +fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { + // #3752: 13 of the 21 sizes had no media name, so their jobs went out on the queue's default + // paper (A4 on the report's queue) whatever the sheets were laid out on. + let ppd_keywords = ["Letter", "Legal", "Tabloid", "A3", "A4", "A5", "w288h432", "Env10"]; + for (i, (name, (w, h))) in PAPERS.iter().enumerate() { + let media = cups_media(i).unwrap_or_else(|| panic!("{name} has no media name")); + assert_eq!(media_option(i), Some(("media".to_string(), media.to_string()))); + let args = lp_args(&Job { options: media_option(i).into_iter().collect(), ..Job::default() }); + assert!(args.contains(&format!("media={media}")), "{name}: lp is given it: {args:?}"); + if ppd_keywords.contains(&media) { + continue; + } + // The rest are PWG 5101.1 names, which end in their dimensions: CUPS feeds the driver's + // size of those dimensions, so they must be the paper's. + let dims = media.rsplit('_').next().unwrap_or_default(); + let (dims, points_per_unit) = match (dims.strip_suffix("mm"), dims.strip_suffix("in")) { + (Some(mm), _) => (mm, 72.0 / 25.4), + (_, Some(inches)) => (inches, 72.0), + _ => panic!("{name}: {media} is not a PWG name"), + }; + let (pw, ph) = dims.split_once('x').unwrap_or_else(|| panic!("{name}: {media} has no dimensions")); + let (pw, ph) = (pw.parse::().unwrap() * points_per_unit, ph.parse::().unwrap() * points_per_unit); + assert!((pw - w).abs() < 0.5 && (ph - h).abs() < 0.5, "{name}: {media} is {pw:.2} x {ph:.2} pt, not {w} x {h}"); + } + assert_eq!((cups_media(PAPERS.len()), media_option(PAPERS.len())), (None, None)); +} + #[test] fn us_number_10_envelope_is_offered_and_fed_by_name() { // 4.125 × 9.5 in (#2734). diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index cab6bfc23..d5d95c881 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -240,7 +240,7 @@ impl PrintDraft { } .into_iter() // The sheet the PDF was laid out on, not the queue's default paper. - .chain(print::cups_media(self.paper).map(|m| ("media".to_string(), m.to_string()))) + .chain(print::media_option(self.paper)) .collect(), } } diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 45dda3982..fc56e5f4e 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -7965,8 +7965,8 @@ milestone = "M10" status = "shipped" commands = ["print.dialog"] tools = ["doc_print", "printers"] -evidence = ["crates/ui-egui/tests/it/print.rs::paper_choice_survives_cancel_and_settings_restore", "crates/ui-egui/tests/it/print.rs::restored_paper_keeps_preview_and_saved_sheet_dimensions", "crates/ui-egui/tests/it/print.rs::print_dialog_lays_out_sheets_and_saves_a_pdf", "crates/engine/src/tests.rs::printing_lays_out_sheets_that_render", "crates/ui-egui/src/web_print.rs::a_job_draws_every_sheet_at_its_planned_resolution_then_builds_the_page", "crates/ui-egui/src/web_print.rs::the_print_page_turns_sideways_sheets_names_other_sizes_and_ends_without_a_blank_page"] -notes = "⌘P; settings and live sheet preview; on the web the same dialog prints through the browser's print dialog; last paper selection, including cancelled Print dialogs, persists by canonical name; this change does not alter persistence of other print preferences" +evidence = ["crates/ui-egui/tests/it/print.rs::paper_choice_survives_cancel_and_settings_restore", "crates/ui-egui/tests/it/print.rs::restored_paper_keeps_preview_and_saved_sheet_dimensions", "crates/ui-egui/tests/it/print.rs::print_dialog_lays_out_sheets_and_saves_a_pdf", "crates/engine/src/tests.rs::printing_lays_out_sheets_that_render", "crates/ui-egui/src/web_print.rs::a_job_draws_every_sheet_at_its_planned_resolution_then_builds_the_page", "crates/ui-egui/src/web_print.rs::the_print_page_turns_sideways_sheets_names_other_sizes_and_ends_without_a_blank_page", "crates/print/src/tests.rs::every_paper_reaches_the_spooler_by_a_name_of_its_size", "crates/automation/src/printing.rs::printing_to_a_printer_feeds_the_laid_out_paper"] +notes = "⌘P; settings and live sheet preview; on the web the same dialog prints through the browser's print dialog; last paper selection, including cancelled Print dialogs, persists by canonical name; this change does not alter persistence of other print preferences. CUPS jobs from the dialog and doc_print ask for the chosen paper (media) in every size: PPD keywords for the common ones, PWG 5101.1 names for A6, A0–A2, ARCH and ANSI C–E, checked against a synthetic PPD with cupsfilter, not on a physical large-format printer (#3752). On macOS jobs still take the PPD's default paper tray until craft-print sends Auto Select (#3752)" [[feature]] id = "print.printer-selection" From 75947139379cf3c1a00fd180d71b6f999e210134 Mon Sep 17 00:00:00 2001 From: Juan Jurado Date: Thu, 22 Oct 2026 18:24:54 -0600 Subject: [PATCH 10/26] M10: spell the ARCH media names as PWG 5101.1 does (#3752) ARCH A-E went out as arch_a_9x12in .. arch_e_36x48in; PWG 5101.1 names them na_arch-a_9x12in .. na_arch-e_36x48in (ARCH E1's IANA-registered na_arch-e1_30x42in already had the prefix). CUPS parses either from the dimensions, which is why the cupsfilter check passed, but a driverless printer may accept only the names it advertises. Re-checked the corrected names with cupsfilter against the synthetic PPD. The paper test now pins every size's exact name, and fails on the old spelling. Doc comments for doc_print and PrintDraft::job. Review: storytold/pdfcraft#4161 (CodeRabbit). --- crates/automation/src/printing.rs | 4 +++- crates/print/src/lib.rs | 17 ++++++++-------- crates/print/src/tests.rs | 34 +++++++++++++++++++++++++++++-- crates/ui-egui/src/print_ui.rs | 2 ++ 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/crates/automation/src/printing.rs b/crates/automation/src/printing.rs index e43c3c6e9..111d27eec 100644 --- a/crates/automation/src/printing.rs +++ b/crates/automation/src/printing.rs @@ -101,6 +101,8 @@ impl Automation { Ok(json!({ "printer": printer, "count": list.len(), "options": list })) } + /// `doc_print`: lay the document out with the Print dialog's options, then save the + /// print-ready PDF to `path` or send it to `printer` with the chosen paper. pub(crate) fn doc_print(&mut self, a: &Args) -> Result { let doc = self.doc(a)?; let (id, count) = (doc.id, doc.info.pages.len()); @@ -243,7 +245,7 @@ mod tests { } // Short forms find the same paper. assert_eq!(job(json!({}), Some("letter")).options, [media("Letter")]); - assert_eq!(job(json!({}), Some("ARCH D")).options, [media("arch_d_24x36in")]); + assert_eq!(job(json!({}), Some("ARCH D")).options, [media("na_arch-d_24x36in")]); assert!(matches!(paper_named(Some("Quarto")), Err(ToolError::InvalidArgs(m)) if m.contains("US Letter"))); assert_eq!(spool_job(&Args(&json!({})), "default", String::new(), None, 0).ok().map(|j| j.printer), Some(None)); assert!(spool_job(&Args(&json!({ "options": { "InputSlot": 3 } })), "Office", String::new(), None, 0).is_err()); diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 80b7be27a..75b6fdf1f 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -309,9 +309,10 @@ pub const LABEL_PAPER: usize = 19; /// The CUPS media name for `PAPERS[index]`, so the spooler feeds the sheet the PDF was laid out /// on rather than the queue's default; `None` only past the end of `PAPERS`. The common sizes /// use their PPD keywords (`w288h432` is the usual 4×6 label, `Env10` the US #10 envelope). -/// The rest use PWG 5101.1 names (the IANA registry for ARCH E1), which carry their dimensions: -/// CUPS feeds the PPD size of those dimensions whatever the driver calls it (`ARCHD`, `AnsiD`), -/// and driverless IPP printers use these names themselves (#3752). +/// The rest use PWG 5101.1 names (for ARCH E1, which 5101.1 lacks, the IANA registry's), spelled +/// exactly: a driverless printer may take only the names it advertises. They carry their +/// dimensions, so CUPS feeds the PPD size of those dimensions whatever the driver calls it +/// (`ARCHD`, `AnsiD`) (#3752). pub fn cups_media(index: usize) -> Option<&'static str> { Some(match PAPERS.get(index)?.0 { "US Letter" => "Letter", @@ -321,12 +322,12 @@ pub fn cups_media(index: usize) -> Option<&'static str> { "A4" => "A4", "A5" => "A5", "A6" => "iso_a6_105x148mm", - "ARCH A (9 x 12 in)" => "arch_a_9x12in", - "ARCH B (12 x 18 in)" => "arch_b_12x18in", - "ARCH C (18 x 24 in)" => "arch_c_18x24in", - "ARCH D (24 x 36 in)" => "arch_d_24x36in", + "ARCH A (9 x 12 in)" => "na_arch-a_9x12in", + "ARCH B (12 x 18 in)" => "na_arch-b_12x18in", + "ARCH C (18 x 24 in)" => "na_arch-c_18x24in", + "ARCH D (24 x 36 in)" => "na_arch-d_24x36in", "ARCH E1 (30 x 42 in)" => "na_arch-e1_30x42in", - "ARCH E (36 x 48 in)" => "arch_e_36x48in", + "ARCH E (36 x 48 in)" => "na_arch-e_36x48in", "ANSI C (17 x 22 in)" => "na_c_17x22in", "ANSI D (22 x 34 in)" => "na_d_22x34in", "ANSI E (34 x 44 in)" => "na_e_34x44in", diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index a17adef15..e7777fa1a 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -674,10 +674,10 @@ fn paper_matching_accepts_rounded_metric_sizes_but_not_custom_or_invalid_sizes() } } +/// #3752: 13 of the 21 sizes had no media name, so their jobs went out on the queue's default +/// paper (A4 on the report's queue) whatever the sheets were laid out on. #[test] fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { - // #3752: 13 of the 21 sizes had no media name, so their jobs went out on the queue's default - // paper (A4 on the report's queue) whatever the sheets were laid out on. let ppd_keywords = ["Letter", "Legal", "Tabloid", "A3", "A4", "A5", "w288h432", "Env10"]; for (i, (name, (w, h))) in PAPERS.iter().enumerate() { let media = cups_media(i).unwrap_or_else(|| panic!("{name} has no media name")); @@ -700,6 +700,36 @@ fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { assert!((pw - w).abs() < 0.5 && (ph - h).abs() < 0.5, "{name}: {media} is {pw:.2} x {ph:.2} pt, not {w} x {h}"); } assert_eq!((cups_media(PAPERS.len()), media_option(PAPERS.len())), (None, None)); + // The names themselves, spelled as PWG 5101.1 has them: a driverless printer may take only the + // names it advertises, so one CUPS can merely parse (ARCH A was `arch_a_9x12in`) is not enough. + // ARCH E1 isn't in 5101.1; its name is the IANA registry's. + let names: Vec<(&str, &str)> = PAPERS.iter().enumerate().map(|(i, (name, _))| (*name, cups_media(i).unwrap_or_default())).collect(); + assert_eq!( + names, + [ + ("US Letter", "Letter"), + ("US Legal", "Legal"), + ("Tabloid", "Tabloid"), + ("A3", "A3"), + ("A4", "A4"), + ("A5", "A5"), + ("A6", "iso_a6_105x148mm"), + ("ARCH A (9 x 12 in)", "na_arch-a_9x12in"), + ("ARCH B (12 x 18 in)", "na_arch-b_12x18in"), + ("ARCH C (18 x 24 in)", "na_arch-c_18x24in"), + ("ARCH D (24 x 36 in)", "na_arch-d_24x36in"), + ("ARCH E1 (30 x 42 in)", "na_arch-e1_30x42in"), + ("ARCH E (36 x 48 in)", "na_arch-e_36x48in"), + ("ANSI C (17 x 22 in)", "na_c_17x22in"), + ("ANSI D (22 x 34 in)", "na_d_22x34in"), + ("ANSI E (34 x 44 in)", "na_e_34x44in"), + ("A2", "iso_a2_420x594mm"), + ("A1", "iso_a1_594x841mm"), + ("A0", "iso_a0_841x1189mm"), + ("4 x 6 in label (101.6 x 152.4 mm)", "w288h432"), + ("US #10 Envelope (4.125 x 9.5 in)", "Env10"), + ] + ); } #[test] diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index d5d95c881..12a941b15 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -223,6 +223,8 @@ impl PrintDraft { self.handling == Handling::Multiple && self.order == PageOrder::CutStack } + /// The spooler job for this print: the dialog's copies, two-sided and colour settings, the + /// driver options changed from the printer's defaults, and the chosen paper. pub fn job(&self, title: &str) -> spool::Job { spool::Job { printer: self.printer.clone(), From 635b42d9899a671b8e82d50033d970b16643c40f Mon Sep 17 00:00:00 2001 From: Juan Jurado Date: Thu, 22 Oct 2026 18:42:49 -0600 Subject: [PATCH 11/26] M10: ARCH E1 goes out as PWG 5101.1's na_wide-format_30x42in (#3752) 30 x 42 in is `na_wide-format_30x42in` in PWG 5101.1-2023 and the IANA IPP registry (checked against both). `na_arch-e1_30x42in` was asked for on the IPP list in 2021 but never registered. CUPS maps the new name to the PPD's 30 x 42 in size (cupsfilter, synthetic PPD), and the paper test pins it and fails on the old name. Doc comments for bad, printers and two paper tests, for the review's docstring check. Review: storytold/pdfcraft#4161 (CodeRabbit). --- crates/automation/src/printing.rs | 3 +++ crates/print/src/lib.rs | 10 +++++----- crates/print/src/tests.rs | 10 +++++++--- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/automation/src/printing.rs b/crates/automation/src/printing.rs index 111d27eec..74e732814 100644 --- a/crates/automation/src/printing.rs +++ b/crates/automation/src/printing.rs @@ -22,6 +22,7 @@ fn duplex(value: &str, cut_stack: bool) -> Result> { }) } +/// An invalid-arguments error saying `m`. fn bad(m: impl Into) -> ToolError { ToolError::InvalidArgs(m.into()) } @@ -75,6 +76,8 @@ fn spool_job(a: &Args, printer: &str, title: String, duplex: Option Result { let list: Vec = spool::printers() .into_iter() diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 75b6fdf1f..d85e766f1 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -309,10 +309,10 @@ pub const LABEL_PAPER: usize = 19; /// The CUPS media name for `PAPERS[index]`, so the spooler feeds the sheet the PDF was laid out /// on rather than the queue's default; `None` only past the end of `PAPERS`. The common sizes /// use their PPD keywords (`w288h432` is the usual 4×6 label, `Env10` the US #10 envelope). -/// The rest use PWG 5101.1 names (for ARCH E1, which 5101.1 lacks, the IANA registry's), spelled -/// exactly: a driverless printer may take only the names it advertises. They carry their -/// dimensions, so CUPS feeds the PPD size of those dimensions whatever the driver calls it -/// (`ARCHD`, `AnsiD`) (#3752). +/// The rest use PWG 5101.1 names (ARCH E1 is its `wide-format` size), spelled exactly as the +/// spec and the IANA registry have them: a driverless printer may take only the names it +/// advertises. They carry their dimensions, so CUPS feeds the PPD size of those dimensions +/// whatever the driver calls it (`ARCHD`, `AnsiD`) (#3752). pub fn cups_media(index: usize) -> Option<&'static str> { Some(match PAPERS.get(index)?.0 { "US Letter" => "Letter", @@ -326,7 +326,7 @@ pub fn cups_media(index: usize) -> Option<&'static str> { "ARCH B (12 x 18 in)" => "na_arch-b_12x18in", "ARCH C (18 x 24 in)" => "na_arch-c_18x24in", "ARCH D (24 x 36 in)" => "na_arch-d_24x36in", - "ARCH E1 (30 x 42 in)" => "na_arch-e1_30x42in", + "ARCH E1 (30 x 42 in)" => "na_wide-format_30x42in", "ARCH E (36 x 48 in)" => "na_arch-e_36x48in", "ANSI C (17 x 22 in)" => "na_c_17x22in", "ANSI D (22 x 34 in)" => "na_d_22x34in", diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index e7777fa1a..d93bdf745 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -661,6 +661,8 @@ fn architectural_sheets_preserve_full_scale() { assert!(fit[0].placed[0].matrix.0[0] < 1.0, "Fit adds margins and must not masquerade as full scale"); } +/// A sheet matches a preset either way round and rounded to whole points, but not half a point off; +/// empty, negative, infinite, NaN and custom sizes match none. #[test] fn paper_matching_accepts_rounded_metric_sizes_but_not_custom_or_invalid_sizes() { for (i, (_, (w, h))) in PAPERS.iter().enumerate() { @@ -702,7 +704,8 @@ fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { assert_eq!((cups_media(PAPERS.len()), media_option(PAPERS.len())), (None, None)); // The names themselves, spelled as PWG 5101.1 has them: a driverless printer may take only the // names it advertises, so one CUPS can merely parse (ARCH A was `arch_a_9x12in`) is not enough. - // ARCH E1 isn't in 5101.1; its name is the IANA registry's. + // ARCH E1 is 5101.1's `wide-format` size: `na_arch-e1_30x42in` was asked for once and never + // registered. let names: Vec<(&str, &str)> = PAPERS.iter().enumerate().map(|(i, (name, _))| (*name, cups_media(i).unwrap_or_default())).collect(); assert_eq!( names, @@ -718,7 +721,7 @@ fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { ("ARCH B (12 x 18 in)", "na_arch-b_12x18in"), ("ARCH C (18 x 24 in)", "na_arch-c_18x24in"), ("ARCH D (24 x 36 in)", "na_arch-d_24x36in"), - ("ARCH E1 (30 x 42 in)", "na_arch-e1_30x42in"), + ("ARCH E1 (30 x 42 in)", "na_wide-format_30x42in"), ("ARCH E (36 x 48 in)", "na_arch-e_36x48in"), ("ANSI C (17 x 22 in)", "na_c_17x22in"), ("ANSI D (22 x 34 in)", "na_d_22x34in"), @@ -732,9 +735,10 @@ fn every_paper_reaches_the_spooler_by_a_name_of_its_size() { ); } +/// #2734: the US #10 envelope (4.125 × 9.5 in) is offered, fed by its PPD name and laid out at +/// its own size. #[test] fn us_number_10_envelope_is_offered_and_fed_by_name() { - // 4.125 × 9.5 in (#2734). let index = matching_paper((684.0, 297.0)).unwrap(); assert_eq!(PAPERS[index].0, "US #10 Envelope (4.125 x 9.5 in)"); assert_eq!(PAPERS[index].1, (297.0, 684.0)); From 12b5e670e40ff321c7a00231a09b333205b8a15f Mon Sep 17 00:00:00 2001 From: so-sai Date: Thu, 22 Oct 2026 13:31:02 +0700 Subject: [PATCH 12/26] feat(ui): add 'Split document...' to page thumbnail context menus --- crates/ui-egui/src/canvas.rs | 6 +++ crates/ui-egui/src/editing.rs | 4 ++ crates/ui-egui/src/panels.rs | 7 +++ crates/ui-egui/tests/it/editing.rs | 82 ++++++++++++++++++++++++++++++ crates/ui-egui/tests/it/viewer.rs | 2 +- parity/acrobat-features.toml | 6 +-- 6 files changed, 103 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index f8df86fb3..df0c89130 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -665,6 +665,8 @@ pub enum ViewAction { Save, Extract, Split, + /// Split before each selected page: opens Split on that mode. + SplitBeforeSelected, /// Copy the selected pages (Cut also deletes them). CopyPages { cut: bool, @@ -5488,6 +5490,10 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool view.pending_action = Some(ViewAction::Extract); ui.close(); } + if ui.button(tl!("Split document…")).clicked() { + view.pending_action = Some(ViewAction::SplitBeforeSelected); + ui.close(); + } if ui.add_enabled(editable, egui::Button::new(tl!("Replace pages…"))).clicked() { view.pending_action = Some(ViewAction::ReplacePages); ui.close(); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 5b821b638..99f0534ab 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -265,6 +265,10 @@ impl PdfCraftApp { } Some(crate::canvas::ViewAction::Extract) => self.open_extract_dialog(), Some(crate::canvas::ViewAction::Split) => self.dialog = Some(crate::Dialog::Split), + Some(crate::canvas::ViewAction::SplitBeforeSelected) => { + self.split_draft.mode = crate::SplitMode::Selection; + self.dialog = Some(crate::Dialog::Split); + } Some(crate::canvas::ViewAction::CopyPages { cut }) => self.copy_pages(cut), Some(crate::canvas::ViewAction::PastePages) => self.paste_pages(), Some(crate::canvas::ViewAction::EnhanceCamera) => self.enhance_camera(i), diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index edfe63ebc..d36f4cf19 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -1363,6 +1363,13 @@ fn pages(ui: &mut egui::Ui, t: &Tokens, info: &DocInfo, view: &mut crate::DocVie view.pending_action = Some(crate::canvas::ViewAction::Extract); ui.close(); } + if ui.button(tl!("Split document…")).clicked() { + if view.selected.is_empty() { + view.select_pages(&[i]); + } + view.pending_action = Some(crate::canvas::ViewAction::SplitBeforeSelected); + ui.close(); + } if ui.add_enabled(editable, egui::Button::new(tl!("Replace pages…"))).clicked() { view.pending_action = Some(crate::canvas::ViewAction::ReplacePages); ui.close(); diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 71ba9ccda..94e6d62bc 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -2241,6 +2241,88 @@ fn split_before_selected_pages() { let _ = std::fs::remove_dir_all(dir); } +#[test] +fn split_from_organize_grid_context_menu() { + let dir = temp_path("split-org-ctx"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut h = organize(4); + h.state_mut().export_dir_override = Some(dir.to_string_lossy().into_owned()); + h.get_by_label("Page 3").click_button(egui::PointerButton::Secondary); + h.run_steps(2); + h.get_all_by_label("Split document…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(pdfcraft_ui_egui::Dialog::Split)); + assert_eq!(h.state().split_draft.mode, pdfcraft_ui_egui::SplitMode::Selection); + h.get_by_label_contains("Creates 2 files from 4 pages"); + h.get_all_by_label("Split").last().unwrap().click(); + h.run_steps(3); + let mut names: Vec = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect(); + names.sort(); + assert_eq!(names, ["doc (pages 1-2).pdf", "doc (pages 3-4).pdf"]); + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn split_from_pages_panel_context_menu() { + let mut h = harness(4, |app| app.right = Some(pdfcraft_ui_egui::RightPanel::Pages)); + let thumb = h.get_all_by_label("Page 3").max_by(|a, b| a.rect().left().total_cmp(&b.rect().left())).unwrap(); + thumb.click_button(egui::PointerButton::Secondary); + h.run_steps(2); + h.get_all_by_label("Split document…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(pdfcraft_ui_egui::Dialog::Split)); + assert_eq!(h.state().split_draft.mode, pdfcraft_ui_egui::SplitMode::Selection); + h.get_by_label_contains("Creates 2 files from 4 pages"); +} + +#[test] +fn split_from_pages_panel_context_menu_on_current_page_without_selection() { + let mut h = harness(4, |app| app.right = Some(pdfcraft_ui_egui::RightPanel::Pages)); + // Navigate to Page 2 (0-indexed 1) so it is current, but keep selected empty + h.state_mut().views[0].go_to_page(1); + h.state_mut().views[0].selected.clear(); + h.run_steps(2); + assert_eq!(h.state().views[0].current, 1); + assert!(h.state().views[0].selected.is_empty()); + let thumb = h.get_all_by_label("Page 2").max_by(|a, b| a.rect().left().total_cmp(&b.rect().left())).unwrap(); + thumb.click_button(egui::PointerButton::Secondary); + h.run_steps(2); + h.get_all_by_label("Split document…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(pdfcraft_ui_egui::Dialog::Split)); + assert_eq!(h.state().split_draft.mode, pdfcraft_ui_egui::SplitMode::Selection); + h.get_by_label_contains("Creates 2 files from 4 pages"); +} + +#[test] +fn split_from_pages_panel_context_menu_on_page_one_falls_back_to_pages() { + let mut h = harness(4, |app| app.right = Some(pdfcraft_ui_egui::RightPanel::Pages)); + let thumb = h.get_all_by_label("Page 1").max_by(|a, b| a.rect().left().total_cmp(&b.rect().left())).unwrap(); + thumb.click_button(egui::PointerButton::Secondary); + h.run_steps(2); + h.get_all_by_label("Split document…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(pdfcraft_ui_egui::Dialog::Split)); + assert_eq!(h.state().split_draft.mode, pdfcraft_ui_egui::SplitMode::Pages); +} + +#[test] +fn split_from_pages_panel_context_menu_with_multi_selection() { + let mut h = harness(4, |app| app.right = Some(pdfcraft_ui_egui::RightPanel::Pages)); + // Select pages 2 and 4 (indices 1 and 3) + h.state_mut().views[0].select_pages(&[1, 3]); + h.run_steps(2); + let thumb = h.get_all_by_label("Page 2").max_by(|a, b| a.rect().left().total_cmp(&b.rect().left())).unwrap(); + thumb.click_button(egui::PointerButton::Secondary); + h.run_steps(2); + h.get_all_by_label("Split document…").last().unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(pdfcraft_ui_egui::Dialog::Split)); + assert_eq!(h.state().split_draft.mode, pdfcraft_ui_egui::SplitMode::Selection); + h.get_by_label_contains("Creates 3 files from 4 pages"); +} + // ── Encrypted documents ─────────────────────────────────────────────────────────────────────── fn protected(user: &str, owner: &str, permissions: i32) -> Vec { diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index a3c86fc52..e4a18ba5e 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -1393,7 +1393,7 @@ fn pages_panel_thumbnails_have_a_context_menu() { h.run_steps(2); // The page right-clicked becomes the selection the menu acts on. assert_eq!(h.state().views[0].target_pages(), vec![2]); - for item in ["Extract pages", "Cut", "Paste after"] { + for item in ["Extract pages", "Split document…", "Cut", "Paste after"] { h.get_by_label(item); } h.get_by_label("Copy").click(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 6a2d0ccf9..23c6d5fed 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1274,8 +1274,8 @@ milestone = "M4" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/it/editing.rs::dragging_pages_panel_thumbnails_reorders_pages", "crates/ui-egui/tests/it/viewer.rs::pages_panel_thumbnails_have_a_context_menu", "crates/ui-egui/tests/it/editing.rs::deleting_pages_from_the_pages_panel_menu", "crates/ui-egui/tests/it/editing.rs::the_delete_key_deletes_pages_picked_in_the_pages_panel_but_never_every_page"] -notes = "Drag the selection (or the page grabbed) to reorder, with a drop bar and edge scrolling; right-click offers Extract pages, Cut, Copy, Paste after and Delete pages (also the Delete key over the panel). Not yet: Rotate, Insert and Page labels in the menu, dragging pages between documents, or dropping files onto the panel." +evidence = ["crates/ui-egui/tests/it/editing.rs::dragging_pages_panel_thumbnails_reorders_pages", "crates/ui-egui/tests/it/viewer.rs::pages_panel_thumbnails_have_a_context_menu", "crates/ui-egui/tests/it/editing.rs::split_from_pages_panel_context_menu", "crates/ui-egui/tests/it/editing.rs::deleting_pages_from_the_pages_panel_menu", "crates/ui-egui/tests/it/editing.rs::the_delete_key_deletes_pages_picked_in_the_pages_panel_but_never_every_page"] +notes = "Drag the selection (or the page grabbed) to reorder, with a drop bar and edge scrolling; right-click offers Split document…, Extract pages, Cut, Copy, Paste after and Delete pages (also the Delete key over the panel). Not yet: Rotate, Insert and Page labels in the menu, dragging pages between documents, or dropping files onto the panel." [[feature]] id = "view.bookmarks-panel" @@ -2920,7 +2920,7 @@ milestone = "M4" status = "shipped" commands = ["page.split"] tools = ["doc_split"] -evidence = ["crates/ui-egui/tests/it/editing.rs::split_before_selected_pages"] +evidence = ["crates/ui-egui/tests/it/editing.rs::split_before_selected_pages", "crates/ui-egui/tests/it/editing.rs::split_from_organize_grid_context_menu"] notes = "" [[feature]] From 64d8c20584f2cf08c91374ba399a289ecdb1f4fd Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Thu, 22 Oct 2026 06:01:23 +0300 Subject: [PATCH 13/26] Leave the page grid when the Crop tool is chosen Crop is only listed in the Organize pages panel, which is shown together with the thumbnail grid; activating it there must bring the document view back, otherwise the tool is selected but no page is on screen to drag a crop rectangle on. In the "page.crop" command arm, close the organize grid of the active view before arming the Crop tool, the way opening a page from the grid does. The left panel then falls back to the workspace panel on the next frame, as it already does when the grid closes by other means. Test: the_crop_tool_leaves_the_page_grid_so_a_page_can_be_cropped failed before the change and passes after. --- crates/ui-egui/src/commands.rs | 3 +++ crates/ui-egui/tests/it/pageboxes.rs | 29 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 92bc0b360..f908cd423 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -669,6 +669,9 @@ impl PdfCraftApp { } "page.crop" => { self.quick_tool = crate::QuickTool::Crop; + if let Some(v) = active.and_then(|i| self.views.get_mut(i)) { + v.organize = false; + } self.notify_tr("Drag a rectangle on a page to crop it; double-click a page for Set Page Boxes"); } "page.boxes" => { diff --git a/crates/ui-egui/tests/it/pageboxes.rs b/crates/ui-egui/tests/it/pageboxes.rs index 865f12770..2e2829e05 100644 --- a/crates/ui-egui/tests/it/pageboxes.rs +++ b/crates/ui-egui/tests/it/pageboxes.rs @@ -90,3 +90,32 @@ fn duplicate_pages_copies_the_current_page() { let s = h.state(); assert_eq!(s.session.get(s.views[0].id).unwrap().info.pages.len(), 3); } + +#[test] +fn the_crop_tool_leaves_the_page_grid_so_a_page_can_be_cropped() { + let mut h = harness(); + // The Organize panel's thumbnail grid is on screen for this view: choosing + // "Crop pages" from it must leave the grid so the canvas handles a drag on + // a page instead of returning early to organize_grid. + h.state_mut().views[0].organize = true; + assert!(h.state_mut().execute("page.crop")); + assert_eq!(h.state().quick_tool, QuickTool::Crop); + assert!(!h.state().views[0].organize, "\"page.crop\" must clear view.organize"); + h.run_steps(2); + let r = h.state().views[0].page_screen_rect(0).unwrap(); + // The middle half of the page in both directions. + let (a, b) = (r.min + r.size() * 0.25, r.min + r.size() * 0.75); + h.hover_at(a); + h.run_steps(1); + h.drag_at(a); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(a + (b - a) * (k as f32 / 4.0)); + h.run_steps(1); + } + h.drop_at(b); + h.run_steps(4); + let (w, hh) = size(&h, 0); + assert!((w - 150.0).abs() < 2.0 && (hh - 200.0).abs() < 2.0, "{w}×{hh}"); + assert_eq!(size(&h, 1), (300.0, 400.0)); +} From 3b9cfe1d52aa1053199a784260b42be82fae0b1a Mon Sep 17 00:00:00 2001 From: Jayesh-Prajapati9 Date: Fri, 23 Oct 2026 18:32:33 +0530 Subject: [PATCH 14/26] Tests: UI tests wait for background work by time, not frame count (#4186) A frame count is not a time: on a busy runner the worker had not finished the open, or the page text had not arrived, within 30 frames or 50 hover rounds. support::step_until (#[must_use]) steps frames until a condition holds or 30 s pass, sleeping briefly between frames, and says whether it held; each test asserts on that with its own message. - control_open_replies_pending_while_the_worker_opens_and_focuses_once_it_has and mode_tabs_keep_the_hand_over_page_text_scrolled_under_them wait with it, and so does control.rs call() for its reply. - support::settle waits for every pending render and fails if the pages are not rendered within 30 s. It replaces all the render-pending settle loops (40 to 3000 frames) and the local settle/wait_for_pages wrappers, except text_select's double-click wait, which is held to 8 frames on purpose to stay inside egui's multi-click window, and view.rs's loops that already wait by a deadline. - wait_until is built on step_until, keeping its diagnostic message. - tabbing_back_to_the_same_field_then_saving_keeps_the_committed_value already waited 30 s for its file. It failed because its "same-frame" case shared its scratch directory with typing_and_saving_in_the_same_frame_saves_the_last_keystrokes, which runs in the same test process (since ee1ee704, #1045) and deleted the directory mid-save. forms' and pickers' scratch() now give every call a directory of its own. Fixes #4186 --- crates/ui-egui/tests/it/bookmark_position.rs | 21 ++---- crates/ui-egui/tests/it/comments.rs | 24 ++----- crates/ui-egui/tests/it/content.rs | 34 ++------- crates/ui-egui/tests/it/control.rs | 27 ++++---- crates/ui-egui/tests/it/editing.rs | 72 ++++++-------------- crates/ui-egui/tests/it/forms.rs | 55 ++++----------- crates/ui-egui/tests/it/object_selection.rs | 8 +-- crates/ui-egui/tests/it/ocr.rs | 8 +-- crates/ui-egui/tests/it/pickers.rs | 7 +- crates/ui-egui/tests/it/prepare.rs | 8 +-- crates/ui-egui/tests/it/redact.rs | 8 +-- crates/ui-egui/tests/it/support/mod.rs | 44 +++++++++--- crates/ui-egui/tests/it/ui.rs | 30 +++----- crates/ui-egui/tests/it/view.rs | 48 ++----------- crates/ui-egui/tests/it/viewer.rs | 16 +---- crates/ui-egui/tests/it/xfa.rs | 15 +--- 16 files changed, 127 insertions(+), 298 deletions(-) diff --git a/crates/ui-egui/tests/it/bookmark_position.rs b/crates/ui-egui/tests/it/bookmark_position.rs index 03fd8b50a..09fd1d4be 100644 --- a/crates/ui-egui/tests/it/bookmark_position.rs +++ b/crates/ui-egui/tests/it/bookmark_position.rs @@ -55,21 +55,10 @@ fn harness(rotate: u16) -> Harness<'static, PdfCraftApp> { app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); h } -/// Run frames until rendering has finished and the layout holds still. -fn settle(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..200 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } -} - /// Make a bookmark with ⌘B, as the Bookmarks button does, and say what it goes to. fn new_bookmark(h: &mut Harness<'static, PdfCraftApp>) -> (Option, DestView) { h.key_press_modifiers(Modifiers::COMMAND, Key::B); @@ -103,7 +92,7 @@ fn a_new_bookmark_goes_to_the_top_of_the_window_not_the_top_of_the_page() { h.state_mut().apply_edit(Edit::AddBookmark { parent: vec![], index: 0, title: "Mid".into(), page: 1, left: None, top: Some(250.0) }); h.run_steps(3); h.get_by_label("Mid").click(); - settle(&mut h); + crate::support::settle(&mut h); let (page, view) = new_bookmark(&mut h); assert_eq!(page, Some(1), "the page at the top of the window"); let (left, top) = xyz(view); @@ -151,7 +140,7 @@ fn a_bookmark_made_on_a_turned_page_pins_the_axis_the_window_scrolls_along() { fn rotate_view(h: &mut Harness<'static, PdfCraftApp>, degrees: u16) { h.state_mut().views[0].rotation = degrees; h.run_steps(3); - settle(h); + crate::support::settle(h); } #[test] @@ -216,12 +205,12 @@ fn a_bookmark_in_a_spread_of_unequal_pages_goes_to_the_page_crossing_the_top_of_ }); h.state_mut().views[0].layout = PageLayout::TwoUp; h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); // 20 pt below page 2's top is at the window's top, where page 1 has not started. h.state_mut().apply_edit(Edit::AddBookmark { parent: vec![], index: 0, title: "Right".into(), page: 1, left: None, top: Some(380.0) }); h.run_steps(3); h.get_by_label("Right").click(); - settle(&mut h); + crate::support::settle(&mut h); let (page, view) = new_bookmark(&mut h); assert_eq!(page, Some(1), "page 2 crosses the top of the window; page 1 starts below it"); close(xyz(view).1, 380.0, "top"); diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 6194c7516..68b41ac98 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -40,20 +40,10 @@ fn harness_with_bytes(bytes: Vec, setup: impl FnOnce(&mut PdfCraftApp) + 'st app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); h } -fn settle(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..200 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } -} - /// A point on page 1 in PDF user space → screen (no rotation; MediaBox 300×200). fn at(h: &Harness<'static, PdfCraftApp>, x: f32, y: f32) -> Pos2 { let r = h.state().views[0].page_screen_rect(0).expect("page 1 on screen"); @@ -146,7 +136,7 @@ fn sidebar_comment_centres_the_annotation_at_high_zoom() { && let Ok(dir) = std::env::var("PDFCRAFT_SHOTS") { let _gpu = crate::gpu(); - settle(&mut h); + crate::support::settle(&mut h); h.render().unwrap().save(format!("{dir}/comment-centred.png")).unwrap(); } } @@ -754,7 +744,7 @@ fn a_dragged_text_box_follows_a_rotated_view() { } h.state_mut().set_option("zoom", "150").unwrap(); h.run_steps(3); - settle(&mut h); + crate::support::settle(&mut h); let screen = |h: &Harness<'static, PdfCraftApp>, x: f32, y: f32| { let r = h.state().views[0].page_screen_rect(0).expect("page 1 on screen"); let (u, v) = (x / 300.0, (200.0 - y) / 200.0); @@ -1726,7 +1716,7 @@ fn attachment_harness(setup: impl FnOnce(&mut PdfCraftApp) + 'static) -> Harness app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); h } @@ -1915,7 +1905,7 @@ fn hovering_a_comment_shows_its_author_and_text() { app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); assert!(h.query_by_label_contains("Check this figure").is_none(), "no popup before hovering"); let p = at(&h, 100.0, 85.0); h.hover_at(p); @@ -1945,7 +1935,7 @@ fn preference_hides_author_only_highlight_hover_previews() { app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); let p = at(&h, 100.0, 85.0); // By default, author-only hover is shown. @@ -1992,7 +1982,7 @@ fn markup_hover_harness(pdf: Vec, view_rotation: u16) -> Harness<'static, Pd app }); h.run_steps(4); - settle(&mut h); + crate::support::settle(&mut h); h } diff --git a/crates/ui-egui/tests/it/content.rs b/crates/ui-egui/tests/it/content.rs index 515e3e04f..dc13bdb61 100644 --- a/crates/ui-egui/tests/it/content.rs +++ b/crates/ui-egui/tests/it/content.rs @@ -28,13 +28,7 @@ fn harness_bytes_with_step_dt(bytes: Vec, step_dt: f32) -> Harness<'static, app.set_option("zoom", "150").unwrap(); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } @@ -161,13 +155,7 @@ fn text_tool_deleting_a_draft_hides_the_old_added_glyphs_and_cancel_restores_the click(&mut h, point); h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::A); h.key_press(egui::Key::Backspace); - for _ in 0..100 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert!(h.state().views[0].content.draft.as_ref().unwrap().text.is_empty()); let shot = h.render().unwrap(); for x in 105..140 { @@ -795,18 +783,6 @@ fn red(pixels: &image::RgbaImage, r: egui::Rect) -> usize { .count() } -/// Steps until no render is pending. -fn settle(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..200 { - h.run_steps(1); - if !h.state().render_pending() { - return; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } - panic!("the page did not finish rendering"); -} - /// An added picture resized by a corner is drawn at its new size while dragged and after the /// release until the page is rendered again. It used to stay at its old size under the outline for /// the whole drag, and to show there again after the release. @@ -826,7 +802,7 @@ fn a_resized_added_picture_shows_only_at_its_new_size() { h.state_mut().add_image("red.png".into(), png); h.run_steps(3); assert_eq!(h.state().views[0].content.selected, Some((0, 0)), "the picture is selected"); - settle(&mut h); + crate::support::settle(&mut h); let rect = added(&h)[0].content.rect(); let r = item_on_screen(&h, rect); // The lower right corner to the middle: half the size, the upper left corner kept. @@ -841,7 +817,7 @@ fn a_resized_added_picture_shows_only_at_its_new_size() { h.hover_at(grip + (to - grip) * (k as f32 / 4.0)); h.run_steps(1); } - settle(&mut h); + crate::support::settle(&mut h); let pixels = h.render().unwrap(); assert!(red(&pixels, kept) > 100, "while dragged, the picture is drawn at its new size"); assert_eq!(red(&pixels, gone), 0, "while dragged, the picture shows at its old size"); @@ -856,7 +832,7 @@ fn a_resized_added_picture_shows_only_at_its_new_size() { } assert_ne!(added(&h)[0].content.rect(), rect, "the resize is applied"); h.state_mut().views[0].hold_rasters = false; - settle(&mut h); + crate::support::settle(&mut h); h.run_steps(2); let pixels = h.render().unwrap(); assert!(red(&pixels, kept) > 100 && red(&pixels, gone) == 0, "the page raster has the picture at its new size"); diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index 54bfe2fa3..ec29ecc97 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/crates/ui-egui/tests/it/control.rs @@ -73,13 +73,15 @@ fn harness_pages_at(pages: usize, width: f32, scale: f32) -> (Harness<'static, P /// Send a request and run frames until it is answered. fn call(h: &mut Harness<'static, PdfCraftApp>, c: &ControlClient, method: &str, params: Value) -> Reply { let rx = c.send(method, params); - for _ in 0..30 { - h.step(); - if let Ok(r) = rx.try_recv() { - return r; - } + let mut reply = None; + let answered = crate::support::step_until(h, |_| { + reply = rx.try_recv().ok(); + reply.is_some() + }); + match reply { + Some(r) if answered => r, + _ => panic!("{method}: no reply in 30 s"), } - panic!("{method}: no reply after 30 frames"); } fn ok(h: &mut Harness<'static, PdfCraftApp>, c: &ControlClient, method: &str, params: Value) -> Value { @@ -1674,15 +1676,12 @@ fn control_open_replies_pending_while_the_worker_opens_and_focuses_once_it_has() // The worker completes the open on later frames; the tab shows, and only then does a // repeated open focus it instead of reporting another pending open. let mut documents = 0; - for _ in 0..30 { - h.run_steps(1); - let state = ok(&mut h, &c, "ui.state", json!({})); + let opened = crate::support::step_until(&mut h, |h| { + let state = ok(h, &c, "ui.state", json!({})); documents = state["documents"].as_array().map(|d| d.len()).unwrap_or_default(); - if documents == 1 { - break; - } - } - assert_eq!(documents, 1, "the pending open completed into a tab"); + documents == 1 + }); + assert!(opened, "the pending open completed into a tab: {documents} tabs"); let again = ok(&mut h, &c, "ui.open", json!({ "path": a })); assert_eq!(again["focused"], true, "{again}"); diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 71ba9ccda..7713a8e2f 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -2826,13 +2826,7 @@ fn deleting_a_draft_does_not_show_the_original_text_underneath() { h.drop_at(at); h.run_steps(3); h.state_mut().views[0].line_editor.as_mut().unwrap().text.clear(); - for _ in 0..100 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let shot = h.render().unwrap(); if let Ok(path) = std::env::var("PDFCRAFT_TEXT_PREVIEW_SHOT") { shot.save(path).unwrap(); @@ -2931,13 +2925,7 @@ fn opening_a_paragraph_leaves_the_page_as_it_is() { }); h.run_steps(4); assert!(h.state_mut().execute("edit.edit_text")); - for _ in 0..100 { - h.run_steps(1); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let page = h.state().views[0].page_screen_rect(0).expect("on screen"); let k = page.width() / 300.0; let block = h.state().session.get(h.state().views[0].id).unwrap().text_blocks(0)[0].rect; @@ -3008,13 +2996,7 @@ fn a_quick_click_into_another_paragraph_keeps_both_in_place() { }; let (top, bottom) = (screen(&blocks[0]), screen(&blocks[1])); click_at(&mut h, top.center()); - for _ in 0..100 { - h.run_steps(1); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert_eq!(h.state().views[0].line_editor.as_ref().map(|e| e.text.as_str()), Some("Top")); let ink = |pixels: &image::RgbaImage, r: egui::Rect| { (r.top() as u32..r.bottom() as u32) @@ -3974,14 +3956,13 @@ fn save_paragraph_drag(h: &mut Harness<'static, PdfCraftApp>, name: &str, origin fn paragraph_drag_shot(h: &mut Harness<'static, PdfCraftApp>, name: &str) { if let Ok(directory) = std::env::var("PDFCRAFT_PARAGRAPH_DRAG_SHOTS") { - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } - assert!(!h.state().render_pending(), "the screenshot must show the current document generation"); + assert!( + crate::support::step_until(h, |h| { + h.run_steps(1); + !h.state().render_pending() + }), + "the screenshot must show the current document generation" + ); let directory = std::path::Path::new(&directory); std::fs::create_dir_all(directory).unwrap(); h.render().unwrap().save(directory.join(name)).unwrap(); @@ -5131,14 +5112,13 @@ fn image_pixels_follow_the_frame_while_a_corner_is_dragged() { h.run_steps(4); // The dragged pixels are the page raster's: wait for it, as a slow runner may not have it yet // (the drag then drew no picture at all). - for _ in 0..400 { - h.run_steps(1); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } - assert!(!h.state().render_pending(), "the page raster arrives"); + assert!( + crate::support::step_until(&mut h, |h| { + h.run_steps(1); + !h.state().render_pending() + }), + "the page raster arrives" + ); let page = h.state().views[0].page_screen_rect(0).unwrap(); let at = |x: f32, y: f32| egui::pos2(page.left() + x / 200.0 * page.width(), page.top() + (300.0 - y) / 300.0 * page.height()); let p = at(95.0, 195.0); @@ -5837,13 +5817,7 @@ fn a_paragraph_running_off_its_page_edits_without_covering_the_next_page() { h.drag_at(p); h.run_steps(1); h.drop_at(p); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert!( h.state().views[0].line_editor.as_ref().is_some_and(|e| e.page == 0 && e.text.contains("line 50")), "the paragraph's off-page lines are in it" @@ -5932,15 +5906,13 @@ fn mode_tabs_keep_the_hand_over_page_text_scrolled_under_them() { h.run_steps(4); let top = h.state().views[0].page_screen_rect(0).expect("on screen").top(); let mut cursor = egui::CursorIcon::Default; - for _ in 0..50 { + let text_cursor = crate::support::step_until(&mut h, |h| { h.hover_at(text.center() + egui::vec2(0.0, top)); h.run_steps(2); cursor = h.output().platform_output.cursor_icon; - if cursor == egui::CursorIcon::Text { - break; - } - } - assert_eq!(cursor, egui::CursorIcon::Text); + cursor == egui::CursorIcon::Text + }); + assert!(text_cursor, "the I-beam over page text, not {cursor:?}"); assert_eq!(mode_tab_cursors(&mut h, text), HAND_ON_EVERY_TAB); } diff --git a/crates/ui-egui/tests/it/forms.rs b/crates/ui-egui/tests/it/forms.rs index 230addd65..676a16db4 100644 --- a/crates/ui-egui/tests/it/forms.rs +++ b/crates/ui-egui/tests/it/forms.rs @@ -14,13 +14,7 @@ fn harness() -> Harness<'static, PdfCraftApp> { app.set_option("zoom", "150").unwrap(); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } @@ -500,8 +494,13 @@ fn wait_for_file(h: &mut Harness<'static, PdfCraftApp>, path: &std::path::Path) h.run_steps(1); } +/// A fresh directory of its own for each call. The tests share one process and run in parallel, +/// so a name alone is not enough: two tests that both used "same-frame" deleted each other's +/// directory mid-save, and the save then never wrote its file (#4186). fn scratch(name: &str) -> std::path::PathBuf { - let dir = std::env::temp_dir().join(format!("pdfcraft-forms-{name}-{}", std::process::id())); + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("pdfcraft-forms-{name}-{}-{n}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); dir @@ -1029,13 +1028,7 @@ fn calculated_field_and_script_side_effect_on_another_page_re_render() { event: "calculate".into(), script: Some("event.value = 'from_' + getField('name').value;".into()), })); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); click_field(&mut h, "name", 0); h.event(egui::Event::Text("hello".into())); h.run_steps(2); @@ -1043,13 +1036,7 @@ fn calculated_field_and_script_side_effect_on_another_page_re_render() { h.run_steps(2); assert_eq!(value(&h, "page2_field"), ["from_hello"]); // Wait for all re-renders to complete. - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert_eq!(value(&h, "name"), ["hello"]); assert_eq!(value(&h, "page2_field"), ["from_hello"]); } @@ -1065,13 +1052,7 @@ fn overlay_clears_after_fresh_tiles_zoom_then_edit_and_rejected_edit() { h.key_press(egui::Key::Tab); h.run_steps(2); assert_eq!(value(&h, "name"), ["GoodVal"]); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert!(!h.state().views[0].forms.unbaked.contains(&("name".to_string(), 0))); // 2. Rejected edit: typing rejected by validate script leaves unbaked clean. @@ -1091,13 +1072,7 @@ fn overlay_clears_after_fresh_tiles_zoom_then_edit_and_rejected_edit() { // 3. Zoom then edit: zoom to another scale, let tiles settle, then edit. h.state_mut().views[0].zoom_step(true); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); click_field(&mut h, "name", 0); h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::A); h.event(egui::Event::Text("ZoomedVal".into())); @@ -1105,13 +1080,7 @@ fn overlay_clears_after_fresh_tiles_zoom_then_edit_and_rejected_edit() { h.key_press(egui::Key::Tab); h.run_steps(2); assert_eq!(value(&h, "name"), ["ZoomedVal"]); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); assert!(!h.state().views[0].forms.unbaked.contains(&("name".to_string(), 0))); } diff --git a/crates/ui-egui/tests/it/object_selection.rs b/crates/ui-egui/tests/it/object_selection.rs index 6e428ad74..fc2a8e4fe 100644 --- a/crates/ui-egui/tests/it/object_selection.rs +++ b/crates/ui-egui/tests/it/object_selection.rs @@ -73,13 +73,7 @@ fn harness_document(clipping: bool, background: bool) -> Harness<'static, PdfCra assert!(app.execute("edit.edit_text")); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } diff --git a/crates/ui-egui/tests/it/ocr.rs b/crates/ui-egui/tests/it/ocr.rs index 20fb73344..7f2e3bdc5 100644 --- a/crates/ui-egui/tests/it/ocr.rs +++ b/crates/ui-egui/tests/it/ocr.rs @@ -133,13 +133,7 @@ fn an_ocr_word_opens_for_editing_and_stays_open() { assert!(app.execute("edit.edit_text")); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let (r, k) = { let app = h.state(); let d = app.session.get(app.views[0].id).unwrap(); diff --git a/crates/ui-egui/tests/it/pickers.rs b/crates/ui-egui/tests/it/pickers.rs index e4ce56d7c..bbab087d9 100644 --- a/crates/ui-egui/tests/it/pickers.rs +++ b/crates/ui-egui/tests/it/pickers.rs @@ -115,9 +115,12 @@ fn a_pick_for_a_document_that_is_no_longer_active_is_dropped() { let _ = std::fs::remove_dir_all(&dir); } -/// A scratch folder for one test. +/// A scratch folder for one test, of its own for each call: the tests share one process and run +/// in parallel, so two that picked the same name would delete each other's folder (#4186). fn scratch(name: &str) -> std::path::PathBuf { - let dir = std::env::temp_dir().join(format!("pdfcraft-pickers-{name}-{}", std::process::id())); + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("pdfcraft-pickers-{name}-{}-{n}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); dir diff --git a/crates/ui-egui/tests/it/prepare.rs b/crates/ui-egui/tests/it/prepare.rs index 106d2d539..bf0f6c68d 100644 --- a/crates/ui-egui/tests/it/prepare.rs +++ b/crates/ui-egui/tests/it/prepare.rs @@ -17,13 +17,7 @@ fn harness() -> Harness<'static, PdfCraftApp> { app.set_option("zoom", "150").unwrap(); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } diff --git a/crates/ui-egui/tests/it/redact.rs b/crates/ui-egui/tests/it/redact.rs index c2d150e11..3e11861cb 100644 --- a/crates/ui-egui/tests/it/redact.rs +++ b/crates/ui-egui/tests/it/redact.rs @@ -14,13 +14,7 @@ fn harness() -> Harness<'static, PdfCraftApp> { app.set_option("zoom", "150").unwrap(); app }); - for _ in 0..60 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } diff --git a/crates/ui-egui/tests/it/support/mod.rs b/crates/ui-egui/tests/it/support/mod.rs index c4c0ee060..2e1e11ffb 100644 --- a/crates/ui-egui/tests/it/support/mod.rs +++ b/crates/ui-egui/tests/it/support/mod.rs @@ -2,21 +2,43 @@ use egui_kittest::Harness; use pdfcraft_ui_egui::PdfCraftApp; -pub fn wait_until(h: &mut Harness<'static, PdfCraftApp>, what: &str, ready: impl Fn(&PdfCraftApp) -> bool) { +/// Step frames until `ready` holds or 30 seconds pass, and say whether it held. For background work +/// (a worker-thread open, page text arriving) that a test then asserts on itself: a frame count is +/// not a time, and a busy runner may not have finished the work within one. Sleeps briefly between +/// frames so the worker threads get the CPU. +#[must_use] +pub fn step_until(h: &mut Harness<'static, PdfCraftApp>, mut ready: impl FnMut(&mut Harness<'static, PdfCraftApp>) -> bool) -> bool { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); loop { h.step(); - if ready(h.state()) { - return; + if ready(h) { + return true; + } + if std::time::Instant::now() >= deadline { + return false; } - assert!( - std::time::Instant::now() < deadline, - "timed out waiting for {what}: views={}, active={:?}, password={:?}, toast={:?}", - h.state().views.len(), - h.state().active, - h.state().password_prompt.as_ref().map(|p| (&p.name, &p.error)), - h.state().toast - ); std::thread::sleep(std::time::Duration::from_millis(5)); } } + +/// Step frames until nothing is waiting to be rendered, and fail the test if that takes over 30 +/// seconds: what a test checks next is on the rendered page. +pub fn settle(h: &mut Harness<'static, PdfCraftApp>) { + let settled = step_until(h, |h| { + h.run_steps(1); + !h.state().render_pending() + }); + assert!(settled, "pages rendered"); +} + +pub fn wait_until(h: &mut Harness<'static, PdfCraftApp>, what: &str, ready: impl Fn(&PdfCraftApp) -> bool) { + let held = step_until(h, |h| ready(h.state())); + assert!( + held, + "timed out waiting for {what}: views={}, active={:?}, password={:?}, toast={:?}", + h.state().views.len(), + h.state().active, + h.state().password_prompt.as_ref().map(|p| (&p.name, &p.error)), + h.state().toast + ); +} diff --git a/crates/ui-egui/tests/it/ui.rs b/crates/ui-egui/tests/it/ui.rs index 6472b7c66..275cdab76 100644 --- a/crates/ui-egui/tests/it/ui.rs +++ b/crates/ui-egui/tests/it/ui.rs @@ -451,7 +451,7 @@ fn overflowing_tabs_can_be_reached_by_scrolling() { assert!(h.state().active.is_some_and(|i| i < 19)); if let Ok(dir) = std::env::var("PDFCRAFT_TAB_SHOTS") { let _gpu = crate::gpu(); - settle(&mut h); + crate::support::settle(&mut h); h.render().unwrap().save(format!("{dir}/overflow-tabs.png")).unwrap(); } } @@ -565,7 +565,7 @@ fn narrow_tab_strips_accept_vertical_wheel_scrolling() { assert!(h.get_by_label_contains("Display theme:").rect().right() <= width); if let Ok(dir) = std::env::var("PDFCRAFT_TAB_SHOTS") { let _gpu = crate::gpu(); - settle(&mut h); + crate::support::settle(&mut h); h.render().unwrap().save(format!("{dir}/tabs-{width}.png")).unwrap(); } } @@ -612,23 +612,13 @@ endstream endobj trailer << /Root 1 0 R >> %%EOF"; -fn settle(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..200 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } -} - #[test] fn find_counts_matches_across_pages() { let mut h = harness(|app| { app.open_bytes("text.pdf", None, TEXT_FIXTURE.to_vec()).expect("opens"); app.set_option("find", "brown").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); h.get_by_label_contains("of 2"); let v = &h.state().views[0]; let f = v.find.as_ref().expect("find open"); @@ -641,7 +631,7 @@ fn find_reports_no_matches() { app.open_bytes("text.pdf", None, TEXT_FIXTURE.to_vec()).expect("opens"); app.set_option("find", "zebra").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); h.get_by_label("No matches"); } @@ -651,7 +641,7 @@ fn drag_selects_text_and_copy_returns_it() { app.open_bytes("text.pdf", None, TEXT_FIXTURE.to_vec()).expect("opens"); app.set_option("left", "closed").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); // "The quick brown fox": glyph 4 = 'q', glyph 14 = 'n' of "brown". let (a, b) = { let v = &h.state().views[0]; @@ -738,7 +728,7 @@ fn zoom_keeps_the_point_under_the_cursor_still() { app.set_option("left", "closed").unwrap(); app.set_option("panel", "none").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); let before = h.state().views[0].glyph_screen_pos(0, 4).expect("glyph on screen"); let z = h.state().views[0].zoom; h.state_mut().views[0].zoom_at(z * 2.0, before); @@ -754,7 +744,7 @@ fn trackpad_pinch_zooms_the_pdf_under_the_pointer() { app.set_option("left", "closed").unwrap(); app.set_option("panel", "none").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); let before = h.state().views[0].glyph_screen_pos(0, 4).expect("glyph on screen"); let zoom = h.state().views[0].zoom; h.hover_at(before); @@ -781,7 +771,7 @@ fn selection_works_in_every_view_rotation() { // Keep the whole rotated page on screen. app.set_option("zoom", "60").unwrap(); }); - settle(&mut h); + crate::support::settle(&mut h); let (a, b) = { let v = &h.state().views[0]; (v.glyph_screen_pos(0, 4).expect("glyph"), v.glyph_screen_pos(0, 14).expect("glyph")) @@ -1321,7 +1311,7 @@ fn document_tabs_expose_selection_and_activate_from_keyboard() { } if let Ok(dir) = std::env::var("PDFCRAFT_TAB_SHOTS") { let _gpu = crate::gpu(); - settle(&mut h); + crate::support::settle(&mut h); h.render().unwrap().save(format!("{dir}/focused-tab-{key:?}.png")).unwrap(); } } @@ -1547,7 +1537,7 @@ fn short_window_rail_buttons_scroll_clear_of_page_navigation() { app.open_bytes("short.pdf", None, FIXTURE.to_vec()).unwrap(); app }); - settle(&mut h); + crate::support::settle(&mut h); let rail = |h: &Harness<'static, PdfCraftApp>, label: &str| { h.query_all_by_label(label).map(|n| n.rect()).find(|r| r.left() >= width - 48.0).unwrap_or_else(|| panic!("{label} in the rail")) }; diff --git a/crates/ui-egui/tests/it/view.rs b/crates/ui-egui/tests/it/view.rs index dc1d5585d..3520f0a98 100644 --- a/crates/ui-egui/tests/it/view.rs +++ b/crates/ui-egui/tests/it/view.rs @@ -1266,13 +1266,7 @@ fn highlight_fields_tints_the_field_area() { 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() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let r = rect(&h, 0).expect("page 1"); // The field spans x 50..250, y 300..330 (PDF space, y up) on a 300×400 page; sample inside it. let at = egui::pos2(r.min.x + r.width() * (60.0 / 300.0), r.min.y + r.height() * (1.0 - 305.0 / 400.0)); @@ -1409,13 +1403,7 @@ fn required_radio_buttons_get_a_round_red_border() { app.set_option("fields", "on").unwrap(); app }); - for _ in 0..100 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let r = rect(&h, 0).expect("page 1"); let ppp = h.ctx.pixels_per_point(); let img = h.render().expect("renders"); @@ -1560,13 +1548,7 @@ fn required_fields_get_a_red_border_when_highlighting() { h.state_mut().set_option("fields", "on").unwrap(); let props = pdfcraft_engine::FieldProps { required: Some(true), ..Default::default() }; h.state_mut().apply_edit(pdfcraft_engine::Edit::SetFieldProps { name: "fullname".into(), props: Box::new(props) }); - for _ in 0..100 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); let r = rect(&h, 0).expect("page 1"); // Just inside the left edge of the field (x 50, y 300..330 on a 300×400 page). let ppp = h.ctx.pixels_per_point(); @@ -1622,13 +1604,7 @@ trailer << /Root 1 0 R >> /// Mean and largest difference between page 1 on screen and the page rendered directly at the /// view's exact device scale, over the lined area of `page_raster_maps_one_to_one_onto_screen_pixels`. fn compare_page_with_raster(h: &mut Harness<'static, PdfCraftApp>, pdf: &[u8], zoom: f32) -> (f64, u8) { - for _ in 0..100 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(h); let ppp = h.ctx.pixels_per_point(); let r = rect(h, 0).expect("page 1"); let img = h.render().expect("renders"); @@ -1693,16 +1669,6 @@ fn grid_harness(pdf: Vec, zoom: &str) -> Harness<'static, PdfCraftApp> { }) } -fn settle(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..400 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } -} - /// Detail in the middle of the window: the summed difference between neighbouring pixels. fn centre_detail(h: &mut Harness<'static, PdfCraftApp>) -> u64 { let img = h.render().expect("renders"); @@ -1720,7 +1686,7 @@ fn centre_detail(h: &mut Harness<'static, PdfCraftApp>) -> u64 { fn zooming_a_tiled_page_shows_its_earlier_tiles_until_new_ones_arrive() { let _gpu = gpu(); let mut h = grid_harness(fine_grid_pdf(), "300"); - settle(&mut h); + crate::support::settle(&mut h); let sharp = centre_detail(&mut h); // The first frame at a new zoom has no tiles for it yet (they are requested at its end). h.state_mut().set_option("zoom", "310").unwrap(); @@ -1736,13 +1702,13 @@ fn zooming_a_tiled_page_shows_its_earlier_tiles_until_new_ones_arrive() { fn returning_to_a_visited_zoom_reuses_its_cached_tiles() { let _gpu = gpu(); let mut h = grid_harness(fine_grid_pdf(), "300"); - settle(&mut h); + crate::support::settle(&mut h); let before = h.state().views[0].tiles_debug(0); assert!(!before.is_empty(), "the 300 % page should be tiled, but it cached no tiles"); // Zoom right out, where the 1200 pt page fits in one raster and is not tiled at all, so // none of its tiles is demanded... h.state_mut().set_option("zoom", "50").unwrap(); - settle(&mut h); + crate::support::settle(&mut h); // ...then back to the first zoom, which was already rendered sharp. h.state_mut().set_option("zoom", "300").unwrap(); h.step(); diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index a3c86fc52..8052e6a31 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -40,13 +40,7 @@ fn harness() -> Harness<'static, PdfCraftApp> { app }); // Background rendering and text extraction can take seconds on a loaded CI runner (FreeBSD VM). - for _ in 0..3000 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h } @@ -1732,13 +1726,7 @@ fn a_picture_inside_form_artwork_under_text_is_selected_and_copied() { app.open_bytes("scan.pdf", None, pdf).unwrap(); app }); - for _ in 0..40 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } + crate::support::settle(&mut h); h.state_mut().system_clipboard = false; let i = h.state().active.unwrap(); let r = h.state().views[i].page_screen_rect(0).expect("on screen"); diff --git a/crates/ui-egui/tests/it/xfa.rs b/crates/ui-egui/tests/it/xfa.rs index e66658b0e..ac03ebd69 100644 --- a/crates/ui-egui/tests/it/xfa.rs +++ b/crates/ui-egui/tests/it/xfa.rs @@ -103,17 +103,6 @@ fn ordinary_forms_and_documents_have_no_xfa_notice() { assert_eq!(xfa(&h), Some(pdfcraft_render::Xfa::Dynamic)); } -/// Run frames until the pages are drawn (fields can be clicked then). -fn wait_for_pages(h: &mut Harness<'static, PdfCraftApp>) { - for _ in 0..40 { - h.run_steps(2); - if !h.state().render_pending() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(5)); - } -} - /// Click the centre of a field's widget. fn click_field(h: &mut Harness<'static, PdfCraftApp>, name: &str) { let p = { @@ -133,7 +122,7 @@ fn click_field(h: &mut Harness<'static, PdfCraftApp>, name: &str) { #[test] fn xfa_buttons_run_their_scripts_in_the_app() { let mut h = open(pdfcraft_xfa::fixtures::shell(&pdfcraft_xfa::fixtures::scripted_template())); - wait_for_pages(&mut h); + crate::support::settle(&mut h); let names = |h: &Harness<'static, PdfCraftApp>| { let s = h.state(); s.session.get(s.views[0].id).unwrap().form.iter().map(|f| f.name.clone()).collect::>() @@ -189,7 +178,7 @@ fn a_form_whose_scripts_were_turned_off_at_open_says_so() { fn scripts_turned_off_by_a_click_bring_a_closed_notice_bar_back() { let tpl = pdfcraft_xfa::fixtures::scripted_template().replace(r#"xfa.host.messageBox("Hello " + qty.rawValue);"#, RUNAWAY); let mut h = open(pdfcraft_xfa::fixtures::shell(&tpl)); - wait_for_pages(&mut h); + crate::support::settle(&mut h); // The form opens with its scripts on, and the user closes the notice bar. let id = h.state().views[0].id; assert!(!h.state().session.get(id).unwrap().xfa_scripts_off()); From 866a28a93e7bb801d87227c3771b82be73738ef9 Mon Sep 17 00:00:00 2001 From: Jayesh-Prajapati9 Date: Fri, 23 Oct 2026 18:15:48 +0530 Subject: [PATCH 15/26] M14: update ctutils to 0.4.5 for RUSTSEC-2026-0385 The dependency-advisory gate rejects ctutils 0.4.2 (non-constant-time mask generation and u128 comparisons, fixed in 0.4.4). It comes in through crypto-bigint for the signing crates. Only its version and checksum change in Cargo.lock. (cherry picked from commit 1f8811761a08d853be4af0c36d52a551bd274054) --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47d73be44..9129e85d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1291,9 +1291,9 @@ dependencies = [ [[package]] name = "ctutils" -version = "0.4.2" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +checksum = "38d28fb6aa7dd680b3ffd1a3e64e115c177bdba63c0ae7f9cd48129ff05c4fa0" dependencies = [ "cmov", "subtle", From 1481eb1ddab1bea1468da2a10c1bcf77c0abac17 Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:39:36 +0300 Subject: [PATCH 16/26] Update ctutils to fix RUSTSEC-2026-0385 --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47d73be44..2a655a3df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1291,9 +1291,9 @@ dependencies = [ [[package]] name = "ctutils" -version = "0.4.2" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +checksum = "12218662ccac37d9714f2191252553d67585b97df8c839cbe49e0bde278dc865" dependencies = [ "cmov", "subtle", From 81e054231cc04124068a95420c050bb9142966fc Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:36:12 +0300 Subject: [PATCH 17/26] organize: format field widget splitting changes --- crates/organize/src/import.rs | 21 ++++++++++++++++++++- crates/organize/src/tests.rs | 17 +++++++++-------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index ef18a121f..ab8c15076 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -38,7 +38,26 @@ const MAX_COPY_DEPTH: usize = 256; /// appearance, placement and page. Field state (name, value, flags, default appearance) moves to /// the new parent field, where the widgets inherit it. const WIDGET_KEYS: &[&[u8]] = &[ - b"Type", b"Subtype", b"Rect", b"P", b"AP", b"AS", b"MK", b"BS", b"Border", b"C", b"CA", b"F", b"H", b"R", b"NM", b"M", b"StructParent", b"StructParents", b"OC", b"Tabs", + b"Type", + b"Subtype", + b"Rect", + b"P", + b"AP", + b"AS", + b"MK", + b"BS", + b"Border", + b"C", + b"CA", + b"F", + b"H", + b"R", + b"NM", + b"M", + b"StructParent", + b"StructParents", + b"OC", + b"Tabs", ]; struct Copier<'a> { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index c1e20a132..00eebaedc 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -681,7 +681,10 @@ fn repeated_page_splits_a_combined_field_widget() { let ps = pages(&out).unwrap(); let widget = |i: usize| -> ObjRef { let list = page_dict(&out, i).get(b"Annots").map(|a| out.resolve(a).as_array().cloned().unwrap_or_default()).unwrap_or_default(); - list.iter().filter_map(|a| a.as_ref()).find(|r| out.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))).expect("a widget annotation") + list.iter() + .filter_map(|a| a.as_ref()) + .find(|r| out.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))) + .expect("a widget annotation") }; let (w1, w2) = (widget(0), widget(1)); assert_ne!(w1, w2, "each copy of the page has its own widget"); @@ -692,7 +695,8 @@ fn repeated_page_splits_a_combined_field_widget() { let f = out.get(parent).as_dict().unwrap().clone(); assert_eq!(f.get(b"T").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("name")); assert_eq!(f.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); - let kids: Vec = f.get(b"Kids").and_then(|k| out.resolve(k).as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); + let kids: Vec = + f.get(b"Kids").and_then(|k| out.resolve(k).as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); assert!(kids.contains(&w1) && kids.contains(&w2), "one field holds both widgets: {kids:?}"); let fields: Vec = catalog(&out) .get(b"AcroForm") @@ -747,7 +751,8 @@ fn form_fields_come_along_and_are_registered() { let field = out.resolve(&fields[0]).as_dict().cloned().unwrap(); assert_eq!(field.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); // The widget child, not the field, names the page (#1810). - let kid = field.get(b"Kids").map(|k| out.resolve(k)).and_then(|k| k.as_array().cloned()).and_then(|k| k.first().and_then(|k| k.as_ref())).unwrap(); + let kid = + field.get(b"Kids").map(|k| out.resolve(k)).and_then(|k| k.as_array().cloned()).and_then(|k| k.first().and_then(|k| k.as_ref())).unwrap(); assert_eq!(out.get(kid).as_dict().unwrap().reference(b"P"), Some(pages(&out).unwrap()[0].obj)); } @@ -2600,11 +2605,7 @@ fn a_repeated_page_joins_the_renamed_field() { let list = page_dict(&dst, page).get(b"Annots").map(|a| dst.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap(); let widget = list.iter().find_map(|a| a.as_ref()).expect("a widget on the page"); widgets.push(widget); - assert_eq!( - dst.get(widget).as_dict().unwrap().reference(b"Parent"), - renamed.as_ref(), - "page {page}'s widget belongs to the renamed field" - ); + assert_eq!(dst.get(widget).as_dict().unwrap().reference(b"Parent"), renamed.as_ref(), "page {page}'s widget belongs to the renamed field"); } assert_ne!(widgets[0], widgets[1], "each copy has its own widget"); } From 1f41c0ccaaabebf2bdc97c616e6426bfa5ac0674 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:31:13 +0300 Subject: [PATCH 18/26] organize: preserve widget actions and inherited field state --- crates/organize/src/import.rs | 44 ++++++++++++++++++++++++++++------- crates/organize/src/tests.rs | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index ab8c15076..405dc8a5f 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -58,6 +58,7 @@ const WIDGET_KEYS: &[&[u8]] = &[ b"StructParents", b"OC", b"Tabs", + b"A", ]; struct Copier<'a> { @@ -435,7 +436,8 @@ fn import_pages_mapped( { let r = dst.add(Object::Null); let c = copier.copy_value(dst, &resolved, Some(r))?; - let Object::Dict(mut d) = c else { continue }; + let Object::Dict(d) = c else { continue }; + let (_, mut d) = field_and_widget(dst, &d); d.set(b"Parent".to_vec(), Object::Ref(parent)); // The new widget is not a field root of its own. copier.fields.retain(|f| *f != r); @@ -864,14 +866,7 @@ fn add_kid(dst: &mut Document, field: ObjRef, kid: ObjRef) -> Result<(), Organiz /// field stays one. fn split_own_field(dst: &mut Document, fields: &mut [ObjRef], widget: ObjRef) { let Some(d) = dst.get(widget).as_dict().cloned() else { return }; - let (mut field, mut child) = (Dict::new(), Dict::new()); - for (k, v) in d.iter() { - if WIDGET_KEYS.contains(&k.as_slice()) { - child.set(k.clone(), v.clone()); - } else { - field.set(k.clone(), v.clone()); - } - } + let (mut field, mut child) = field_and_widget(dst, &d); let parent = dst.add(Object::Null); field.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(widget)])); child.set(b"Parent".to_vec(), Object::Ref(parent)); @@ -882,6 +877,37 @@ fn split_own_field(dst: &mut Document, fields: &mut [ObjRef], widget: ObjRef) { } } +/// A combined field/widget has two additional-action namespaces (ISO 32000-1 tables 194/196). +/// Partition them as well as the ordinary entries, including when copying a later widget. +fn field_and_widget(doc: &Document, d: &Dict) -> (Dict, Dict) { + let (mut field, mut child) = (Dict::new(), Dict::new()); + for (k, v) in d.iter() { + if k.as_slice() == b"AA" + && let Some(actions) = doc.resolve(v).as_dict().cloned() + { + let (mut field_actions, mut widget_actions) = (Dict::new(), Dict::new()); + for (trigger, action) in actions.iter() { + if [b"E".as_slice(), b"X", b"D", b"U", b"Fo", b"Bl", b"PO", b"PC", b"PV", b"PI"].contains(&trigger.as_slice()) { + widget_actions.set(trigger.clone(), action.clone()); + } else { + field_actions.set(trigger.clone(), action.clone()); + } + } + if field_actions.iter().next().is_some() { + field.set(k.clone(), Object::Dict(field_actions)); + } + if widget_actions.iter().next().is_some() { + child.set(k.clone(), Object::Dict(widget_actions)); + } + } else if WIDGET_KEYS.contains(&k.as_slice()) { + child.set(k.clone(), v.clone()); + } else { + field.set(k.clone(), v.clone()); + } + } + (field, child) +} + /// Add top-level fields to the destination's `/AcroForm /Fields` (creating the form if needed). fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), OrganizeError> { if fields.is_empty() { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 00eebaedc..ad4688e25 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -690,6 +690,9 @@ fn repeated_page_splits_a_combined_field_widget() { assert_ne!(w1, w2, "each copy of the page has its own widget"); for (i, w) in [(0, w1), (1, w2)] { let d = out.get(w).as_dict().unwrap().clone(); + for key in [b"T".as_slice(), b"V", b"FT"] { + assert!(!d.contains(key), "a widget inherits field entries: {key:?}"); + } assert_eq!(d.reference(b"P"), Some(ps[i].obj), "/P names the page holding the widget"); let parent = d.reference(b"Parent").expect("the widget has a parent field"); let f = out.get(parent).as_dict().unwrap().clone(); @@ -712,6 +715,46 @@ fn repeated_page_splits_a_combined_field_widget() { } } +#[test] +fn splitting_a_combined_widget_keeps_annotation_actions_and_field_triggers() { + let mut src = doc_a(); + let page = pages(&src).unwrap()[1].obj; + let list = page_dict(&src, 1).get(b"Annots").and_then(|a| a.as_array()).cloned().unwrap(); + let widget = + list.iter().filter_map(Object::as_ref).find(|r| src.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))).unwrap(); + let mut action = Dict::new(); + action.set(b"S".to_vec(), Object::name("GoTo")); + action.set(b"D".to_vec(), Object::Array(vec![Object::Ref(page), Object::name("Fit")])); + let mut named = Dict::new(); + named.set(b"S".to_vec(), Object::name("Named")); + named.set(b"N".to_vec(), Object::name("NextPage")); + let mut aa = Dict::new(); + aa.set(b"E".to_vec(), Object::Dict(named.clone())); + aa.set(b"K".to_vec(), Object::Dict(named)); + let aa = src.add(Object::Dict(aa)); + src.update_dict(widget, |d| { + d.set(b"A".to_vec(), Object::Dict(action)); + d.set(b"AA".to_vec(), Object::Ref(aa)); + }) + .unwrap(); + let out = full_roundtrip(&extract_pages(&src, &[1, 1]).unwrap()); + let ps = pages(&out).unwrap(); + for i in 0..2 { + let w = annots(&out, i).into_iter().find(|d| d.name(b"Subtype") == Some(b"Widget")).unwrap(); + let action = out.resolve(w.get(b"A").expect("the widget keeps its action")); + let dest = action.as_dict().unwrap().get(b"D").and_then(|a| a.as_array()).unwrap()[0].as_ref().unwrap(); + assert!(ps.iter().any(|p| p.obj == dest), "the action targets a copied page"); + let waa = out.resolve(w.get(b"AA").expect("widget triggers")); + assert!(waa.as_dict().unwrap().contains(b"E")); + assert!(!waa.as_dict().unwrap().contains(b"K")); + let parent = out.get(w.reference(b"Parent").unwrap()).as_dict().unwrap().clone(); + assert!(!parent.contains(b"A")); + let faa = out.resolve(parent.get(b"AA").expect("field triggers")); + assert!(faa.as_dict().unwrap().contains(b"K")); + assert!(!faa.as_dict().unwrap().contains(b"E")); + } +} + #[test] fn repeated_page_gets_independent_annotations() { // A1 has a link, A2 a link and a form field, A3 a note with a popup; each is listed twice. From ba37d412e81482d567452d80c4f56d8996755b0d Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:02:14 -0400 Subject: [PATCH 19/26] test(ui): recovery idle tests make folders that never share a name --- crates/ui-egui/src/recovery.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/recovery.rs b/crates/ui-egui/src/recovery.rs index 17a3410d1..c21e2366b 100644 --- a/crates/ui-egui/src/recovery.rs +++ b/crates/ui-egui/src/recovery.rs @@ -424,9 +424,14 @@ mod idle_tests { struct TestDir(PathBuf); impl TestDir { + /// A new folder of this test's own. The tests run on parallel threads of one process, and + /// two can read the same clock time (it moves in microseconds on macOS), so a counter + /// tells their folders apart; the time keeps runs of the process apart. fn new() -> Self { + static NEXT: AtomicU64 = AtomicU64::new(0); + let n = NEXT.fetch_add(1, Ordering::Relaxed); let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); - let path = std::env::temp_dir().join(format!("pdfcraft-idle-recovery-{}-{nonce}", std::process::id())); + let path = std::env::temp_dir().join(format!("pdfcraft-idle-recovery-{}-{nonce}-{n}", std::process::id())); std::fs::create_dir(&path).unwrap(); Self(path) } @@ -441,6 +446,26 @@ mod idle_tests { } } + /// Folders made at the same moment on parallel threads, as the tests make theirs, are each + /// their own: none fails because another test's folder took its name. + #[test] + fn test_folders_made_at_once_on_parallel_threads_are_each_their_own() { + let start = std::sync::Barrier::new(8); + let dirs: Vec = std::thread::scope(|s| { + let threads: Vec<_> = (0..8) + .map(|_| { + s.spawn(|| { + start.wait(); + (0..25).map(|_| TestDir::new()).collect::>() + }) + }) + .collect(); + threads.into_iter().flat_map(|t| t.join().unwrap()).map(|d| d.path().to_path_buf()).collect() + }); + let distinct: std::collections::HashSet<&PathBuf> = dirs.iter().collect(); + assert_eq!(distinct.len(), 200); + } + fn frame(app: &mut PdfCraftApp, ctx: &egui::Context, now: f64) -> Duration { let mut out = ctx.run_ui(egui::RawInput { time: Some(now), ..Default::default() }, |ui| { app.autosave_tick(ui.ctx(), now); From a8839bc5b1a4ecd92d955647ef6b97fd70b00329 Mon Sep 17 00:00:00 2001 From: Kyle Kassen <85040622+KyleKassen@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:58:26 -0500 Subject: [PATCH 20/26] M10: preserve page artwork when printing annotations --- ROADMAP.md | 7 +++- crates/print/README.md | 1 + crates/print/src/lib.rs | 13 ++++-- crates/print/src/tests/appearances.rs | 60 +++++++++++++++++++++++++++ docs/roadmap.md | 5 ++- parity/acrobat-features.toml | 4 +- 6 files changed, 82 insertions(+), 8 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 5634d6bf2..c442c0608 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away -> **Last reviewed:** 2026-10-22 · **Last updated:** 2026-10-22 · **Change:** minor (M14, #3558: rebase guarded macOS per-file association recovery onto current main; estimates unchanged) +> **Last reviewed:** 2026-10-23 · **Last updated:** 2026-10-23 · **Change:** minor (M10: vector printing preserves existing page XObjects when adding annotation appearances; estimates unchanged) > **Last reviewed:** 2026-10-21 · **Last updated:** 2026-10-21 · **Change:** minor (M3, #3808: the Pages inspector has an independent 50–150% thumbnail size slider and buttons; selection and current-page following are retained, with headless ui.set coverage. Estimates unchanged.) > **Last reviewed:** 2026-10-20 | **Last updated:** 2026-10-20 | **Change:** minor (M11 #3507: image merging preserves content stream boundaries; estimates unchanged) > **Last reviewed:** 2026-10-20 | **Last updated:** 2026-10-20 | **Change:** minor (M10, #3532 partial: vector printing retains existing appearances with absent Subtype; estimates unchanged) @@ -191,6 +191,10 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-23 (M10, print resources):** Vector printing preserves original page artwork when existing XObjects use annotation resource names. + The regression checks two distinct appearances, inherited resources, print/reopen pixels and unchanged source data. + Estimates unchanged. + - **2026-10-22 (M4, #3916 partial):** Document edits and Undo/Redo cancel an Organize page drag. Pointer-release regressions check exact page order and save/reopen. - **2026-10-22 (M14, macOS association recovery candidate):** Successful desktop open/save/sign can queue recovery of an existing per-file binding to the running PDFCraft bundle through NSWorkspace. Metadata-only watcher changes retain undo and avoid false overwrite prompts; real same-length/same-mtime edits remain protected. Rebased on 0.12.0 and reuses #3042's shared atomic metadata preservation; the bounded recovery parser does not narrow save limits. Prior native Preview TEST-signature and Finder workflows passed on macOS 27.0.1; other versions remain unqualified. Estimates unchanged. - **2026-10-13 (M7, free image rotation):** A selected page image has a rotate knob (Shift: 15 degree steps) and `image_edit` a `rotate_by` action; the angle is written as one undoable `cm`. Snapping to page edges while moving is in #1740; resize snapping is not done. Estimates unchanged. @@ -640,6 +644,7 @@ Newest first: the date, what moved, and the new overall estimate. | Date | Change | Summary | |---|---|---| +| 2026-10-23 | minor | M10: annotation appearances use unoccupied XObject names, preserving original page artwork and distinct appearances through inherited resources and print/reopen. Estimates unchanged. | | 2026-10-22 | minor | M4, #3916 partial: document edits and Undo/Redo cancel Organize drags. Pointer-release and save/reopen regressions check page order. Estimates unchanged. | | 2026-10-20 | minor | M4: initial-view window and interface options resolve indirect boolean ViewerPreferences values; synthetic true/false and save/reopen regressions retain defaults for malformed entries. Estimates unchanged. | | 2026-10-20 | minor | M6, #3502: form detection preserves comments across content streams; estimates unchanged. | diff --git a/crates/print/README.md b/crates/print/README.md index ae60e1a2c..1379be2b4 100644 --- a/crates/print/README.md +++ b/crates/print/README.md @@ -35,6 +35,7 @@ spool::submit(&pdf, &spool::Job { printer: None, copies: 2, ..Default::default() Each source page becomes a Form XObject (its content wrapped in q/Q, plus the printable annotations); sheets place them with a clip. The result is a fresh, unencrypted, garbage-collected file (callers check the print permission). +Annotation appearances receive unused XObject names, preserving existing page resources even when they use the same naming prefix. The GUI Print dialog remembers its last selected paper size across application sessions, including when Print is cancelled. The preference stores a canonical name such as `A4`; diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index db65492c1..98d0fd662 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -801,7 +801,7 @@ fn page_form( } let annots = page.dict.get(b"Annots").and_then(|a| doc.resolve(a).as_array().cloned()).unwrap_or_default(); let mut xobjects = res.get(b"XObject").and_then(|x| doc.resolve(x).as_dict().cloned()).unwrap_or_default(); - let mut k = 0; + let mut k = 0usize; for a in annots { let Some(d) = doc.resolve(&a).as_dict().cloned() else { continue }; if !prints(doc, &d, content) { @@ -833,8 +833,15 @@ fn page_form( } else { ap }; - let name = format!("PCPrA{k}"); - k += 1; + // Keep the page's own XObjects, including names previous print operations generated. + // Check the growing map so each appearance also keeps the name just assigned to it. + let name = loop { + let name = format!("PCPrA{k}"); + k = k.checked_add(1).ok_or_else(|| PrintError::Invalid("too many annotation appearance resources".into()))?; + if !xobjects.contains(name.as_bytes()) { + break name; + } + }; let [a0, b0, c0, d0, e0, f0] = m.0; body.extend(format!("q {} {} {} {} {} {} cm /{name} Do Q\n", n(a0), n(b0), n(c0), n(d0), n(e0), n(f0)).bytes()); xobjects.set(name.into_bytes(), Object::Ref(ap)); diff --git a/crates/print/src/tests/appearances.rs b/crates/print/src/tests/appearances.rs index acf8ece6f..325841987 100644 --- a/crates/print/src/tests/appearances.rs +++ b/crates/print/src/tests/appearances.rs @@ -85,6 +85,66 @@ fn xobject(doc: &Document, owner: &Dict, name: &[u8]) -> ObjRef { objects.as_dict().unwrap().get(name).unwrap().as_ref().unwrap() } +#[test] +fn annotation_appearances_do_not_replace_page_xobjects() { + for inherited in [false, true] { + let (mut doc, ap, widget) = widget_fixture(Some(Object::name("Form")), None); + let page = pdfcraft_model::pages(&doc)[0].obj; + let mut xobjects = Dict::new(); + for (name, color) in [("PCPrA0", "1 0 0"), ("PCPrA1", "0 0 1"), ("PCPrA3", "1 0 0")] { + let dict = Dict::from_iter([ + (b"Type".to_vec(), Object::name("XObject")), + (b"Subtype".to_vec(), Object::name("Form")), + (b"BBox".to_vec(), numbers(&[0.0, 0.0, 10.0, 10.0])), + ]); + let form = doc.add(Object::Stream(Stream::flate(dict, format!("{color} rg 0 0 10 10 re f").as_bytes()))); + xobjects.set(name.as_bytes().to_vec(), Object::Ref(form)); + } + let xobjects = doc.add(Object::Dict(xobjects)); + let resources = doc.add(Object::Dict(Dict::from_iter([(b"XObject".to_vec(), Object::Ref(xobjects))]))); + let contents = doc.add(Object::Stream(Stream::flate( + Dict::new(), + b"q 1 0 0 1 60 20 cm /PCPrA0 Do Q\nq 1 0 0 1 70 20 cm /PCPrA1 Do Q\nq 1 0 0 1 80 20 cm /PCPrA3 Do Q", + ))); + let second_ap = doc.add(Object::Stream(Stream::flate(doc.get(ap).as_dict().unwrap().clone(), b"/Ink cs 1 1 0 scn 0 0 20 10 re f"))); + let mut second_widget = doc.get(widget).as_dict().unwrap().clone(); + second_widget.set(b"Rect".to_vec(), numbers(&[10.0, 50.0, 50.0, 70.0])); + second_widget.set(b"AP".to_vec(), Object::Dict(Dict::from_iter([(b"N".to_vec(), Object::Ref(second_ap))]))); + let second_widget = doc.add(Object::Dict(second_widget)); + doc.update_dict(page, |d| { + d.set(b"Resources".to_vec(), Object::Ref(resources)); + d.set(b"Contents".to_vec(), Object::Ref(contents)); + d.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(widget), Object::Ref(second_widget)])); + }) + .unwrap(); + if inherited { + let parent = doc.get(page).as_dict().unwrap().reference(b"Parent").unwrap(); + doc.update_dict(parent, |d| d.set(b"Resources".to_vec(), Object::Ref(resources))).unwrap(); + doc.update_dict(page, |d| { + d.remove(b"Resources"); + }) + .unwrap(); + } + let before = saved(&doc); + let source = render(before.clone()); + for (x, color) in [(65, [255, 0, 0, 255]), (75, [0, 0, 255, 255]), (85, [255, 0, 0, 255])] { + assert_eq!(pixel(&source, x, 75), color, "source page artwork"); + } + for (y, color) in [(40, [255, 255, 0, 255]), (70, [0, 255, 0, 255])] { + assert_eq!(pixel(&source, 30, y), color, "source widget appearance"); + } + let printed = render(impose(&doc, &print_settings(Content::Document)).unwrap()); + for (x, color) in [(65, [255, 0, 0, 255]), (75, [0, 0, 255, 255]), (85, [255, 0, 0, 255])] { + assert_eq!(pixel(&printed, x, 75), color, "printed page artwork, inherited={inherited}"); + } + for (y, color) in [(40, [255, 255, 0, 255]), (70, [0, 255, 0, 255])] { + assert_eq!(pixel(&printed, 30, y), color, "printed widget appearance, inherited={inherited}"); + } + assert_eq!(printed.rgba, source.rgba, "printing must retain page artwork and both appearances, inherited={inherited}"); + assert_eq!(saved(&doc), before, "printing must leave the source resources unchanged"); + } +} + #[test] fn missing_subtype_appearance_survives_vector_print() { let (control, _, _) = widget_fixture(Some(Object::name("Form")), None); diff --git a/docs/roadmap.md b/docs/roadmap.md index bb542aefc..c609f5882 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # PdfCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-22 · **Last updated:** 2026-10-22 · **Change:** minor (M14, #3558: rebase guarded macOS per-file association recovery onto current main; estimates unchanged) +> **Last reviewed:** 2026-10-23 · **Last updated:** 2026-10-23 · **Change:** minor (M10: vector printing preserves existing page XObjects when adding annotation appearances; estimates unchanged) > **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M12 #3673: Character encoding accepts Identity-H/V with Adobe CJK collections; estimates unchanged) > **Last reviewed:** 2026-10-20 | **Last updated:** 2026-10-20 | **Change:** minor (M10, #3532 partial: vector printing retains existing appearances with absent Subtype; estimates unchanged) @@ -184,7 +184,7 @@ total matches the parity estimate. | M7 | Content editing (text, images, header/footer, watermark) | 250–500 | 20% | 100–180 | Done: header & footer, watermarks, backgrounds, Bates; added text and images that stay editable (move, resize, format, rotate, flip, crop, replace); links (Link tool, Link Properties, create from URLs, remove all). Missing: editing existing text and images in place (the longest pole), image/PDF watermarks | | M8 | Security + redaction | 100–180 | 68% | 15–30 | Done: opening protected documents, permissions, Protect Using Password, Remove security; redaction (mark text/areas/pages, Search & Redact with patterns, apply removing glyphs, image pixels, vectors, XObject content, annotations and fields, verification, full rewrite on save); Remove hidden information and Sanitize.
Done since: redaction codes (U.S. FOIA and Privacy Act sets as overlay text, in Redaction properties and `redact_mark`). Redaction cleans indirect and form structure references, preserving surviving form tags. Redaction retires inherited resource dictionaries, so a parent `/Pages` can't keep a removed XObject reachable; unredacted pages are left untouched. Image redaction removes alternate images and unsupported masks only on affected images. Changed marked-content spans lose replacement descriptions; untouched spans keep theirs. Hidden-text cleanup retires unused named descriptions across inherited/shared page and form resources (64 focused tests and scoped Rust 1.96 Clippy/wasm checks pass; publication CI pending). The redaction audit counts a filled cover that hides half of any one word of a long line (#2662), applied or not. Missing: certificate security, custom redaction code sets and pattern locales, DCT re-encoding | | M9 | Signatures (PAdES, validation) | 160–280 | 60% | 40–70 | Done: new `sign` crate (DER, X.509, CMS, PKCS #12 on RustCrypto; aws-lc-rs for RSA private keys); PAdES B-B signing (visible, invisible, existing fields, certification with DocMDP); validation with trust store and changes-after-signing classification; self-signed digital IDs; Signatures panel, message bar, sign dialogs; agent tools. Checked with pdfsig and OpenSSL.
Done since: macOS Keychain and Windows Current User Personal store signing (software-backed CNG keys), certificate viewer, signed documents protected from rewrites, page validation overlays separate from original signature artwork (baked Not Verified captions remain; #79 open), snapshot tabs that reuse instead of multiplying. Missing: timestamps (B-T), LTV (DSS, OCSP, CRL), FieldMDP, smart cards, PKCS #11 | -| M10 | OCR, create, export, print | 200–350 | 33% | 120–215 | Done: create from blank/text/PNG/JPEG/TIFF (multi-page)/GIF/BMP; export PNG/JPEG/TIFF and text; Print (Acrobat's sizing, n-up, booklet, poster, comments & forms, preview, the CUPS and Windows spoolers, print-ready PDF).
Done since: embedded/72/custom DPI choices for image imports; export all images; OCR (searchable image for pages, ranges and multiple files); single-sided cut-and-stack imposition with cut marks, through Print and doc_print; architectural/ANSI/large ISO sheet presets, Match page at 100% and PDF-scale preview (#599 follow-up); Create PDF from multiple files (PDFs, images and text, edited as pages in the grid); OCR can also read pictures on pages that have text, without adding that text twice (#1028). Done since: Word invoice export uses borderless inferred tables, floating positioned text/artwork only where text overlaps an image or is rotated, and per-page sizes, covered by synthetic invoice automation regressions (#773); printing on the web through the browser's print dialog, sheets drawn by PdfCraft (gap 4). Lossless image export applies per-component Decode, including Indexed mapping (#1201 partial). Existing 16-bit precision and color-management limits remain. #3532 partial: vector printing retains existing appearances with absent Subtype through a print-owned Form copy; source artwork stays unchanged. #3120: Office export reads form XObjects 50 levels deep, as the renderer does, and up to 50 000 a page; pages past that are listed in `truncated_pages`. Missing: OCR languages beyond Latin, editable-text OCR output, full Office layout fidelity/import. Done since: dotted-leader contents entries stay in separate complete paragraphs (#1757) #2156 candidate: last selected print paper persists across sessions; other print preferences unchanged. | +| M10 | OCR, create, export, print | 200–350 | 33% | 120–215 | Done: create from blank/text/PNG/JPEG/TIFF (multi-page)/GIF/BMP; export PNG/JPEG/TIFF and text; Print (Acrobat's sizing, n-up, booklet, poster, comments & forms, preview, the CUPS and Windows spoolers, print-ready PDF).
Done since: embedded/72/custom DPI choices for image imports; export all images; OCR (searchable image for pages, ranges and multiple files); single-sided cut-and-stack imposition with cut marks, through Print and doc_print; architectural/ANSI/large ISO sheet presets, Match page at 100% and PDF-scale preview (#599 follow-up); Create PDF from multiple files (PDFs, images and text, edited as pages in the grid); OCR can also read pictures on pages that have text, without adding that text twice (#1028). Done since: Word invoice export uses borderless inferred tables, floating positioned text/artwork only where text overlaps an image or is rotated, and per-page sizes, covered by synthetic invoice automation regressions (#773); printing on the web through the browser's print dialog, sheets drawn by PdfCraft (gap 4). Lossless image export applies per-component Decode, including Indexed mapping (#1201 partial). Existing 16-bit precision and color-management limits remain. #3532 partial: vector printing retains existing appearances with absent Subtype through a print-owned Form copy; source artwork stays unchanged. #3120: Office export reads form XObjects 50 levels deep, as the renderer does, and up to 50 000 a page; pages past that are listed in `truncated_pages`. Missing: OCR languages beyond Latin, editable-text OCR output, full Office layout fidelity/import. Done since: dotted-leader contents entries stay in separate complete paragraphs (#1757) #2156 candidate: last selected print paper persists across sessions; other print preferences unchanged. Vector printing gives annotation appearances unoccupied XObject names so existing page artwork survives. | | M11 | Optimize, preflight, PDF/A/X/UA, print production | 200–350 | 18% | 80–140 | Done: new `optimize` crate: Reduce File Size and the PDF Optimizer (images measured where drawn, bicubic downsampling, JPEG/ZIP recompression only when smaller, discard objects and user data, Flate clean-up, resource merging, object streams). Content stream boundaries preserve comments, split operands and graphics state when image fragments merge. Done since: reject invalid image and soft-mask dimensions with checked sizing (#2178). Missing: fonts and transparency panels, space audit, preflight, PDF/A/X/UA | | M12 | Accessibility, compare, measure, search, XFA | 200–380 | 30% | 90–160 | Done: new `a11y` crate with the Accessibility Checker (all 32 rules, report, Fix/Skip/Explain, options dialog and results panel, agent tools). Locatable figure, annotation and form findings highlight their page bounds. Character encoding accepts Identity-H/V with Adobe CJK collections and predefined CJK CMaps independently (#3673); per-glyph validation and embedded-font extraction remain separate.
Done since: 2D distance, perimeter and area measurements with persistent viewport calibration, snapping, live information and CSV export; a bounded, text-only auto-tag proposal tool for one page. Marking a figure decorative rewrites only that stream (#1324). An XFA radio item whose value contains a space is written back unchanged (#1319). Done since: #2569: Growable multiline XFA text reflows after edits, with undo/redo, save/reopen and print coverage; oversized fields still become read-only page text. Structured-data synchronization warnings retain accepted whitespace without relayout from stale data. Done since: #2572: Every repeated page-master button resolves its own field to the shared XFA click script, including print buttons. Done since: #2570: Dynamic XFA rich-text fields carry exData/maxLength into AcroForm MaxLen for reported and enforced Unicode character limits, retained after save/reopen. Missing: writing proposed tags and parent tree, field autotag, visual Tags/Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA
Reading colours: pending page backgrounds follow the configured colour (#2059). | | M13 | Automation (MCP, Action Wizard, CLI) + AI providers | 60–120 | 45% | 25–45 | Done: MCP resources (document info, text, page images); headless tool table (123 tools incl. signing, optimizing, initial view, links, stamps, data exchange, comment review, forms authoring and scripts, redaction, sanitize, print, add content), opt-in MCP server over stdio, CLI `run`/`tools` (closed stdout pipes exit cleanly), UI control channel with drag. Missing: Action Wizard, AI providers
Done since: CLI run and MCP reject missing, empty, repeated and option-valued --root arguments before any tool/script execution or MCP initialization. A single-hyphen directory name remains valid; synthetic CLI regressions preserve a would-be overwritten file. `text_find` also caps the per-page match ranges it materializes at the remaining result limit plus one continuation match. CLI robustness scans visit each directory once through overlapping roots and symlink aliases. | @@ -239,6 +239,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st | Date | Change | Summary | |---|---|---| +| 2026-10-23 | minor | M10: annotation appearances use unoccupied XObject names, preserving original page artwork and distinct appearances through inherited resources and print/reopen. Estimates unchanged. | | 2026-10-22 | minor | M4, #3916 partial: edits and Undo/Redo cancel Organize page drags. Pointer-release and save/reopen regressions check page order. Estimates unchanged. | | 2026-10-22 | minor | M14 #3558: rebased macOS association recovery; reuse #3042 shared metadata preservation instead of a duplicate save wrapper. Estimates unchanged. | | 2026-10-20 | minor | M4: initial-view window and interface options resolve indirect boolean ViewerPreferences values; synthetic true/false and save/reopen regressions retain defaults for malformed entries. Estimates unchanged. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 4ec39848c..41f97c616 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -8109,8 +8109,8 @@ milestone = "M10" status = "shipped" commands = ["print.dialog"] tools = ["doc_print"] -evidence = ["crates/print/src/tests.rs::imposed_pdf_has_the_sheets_and_honours_comments_and_forms"] -notes = "Print flag honoured" +evidence = ["crates/print/src/tests.rs::imposed_pdf_has_the_sheets_and_honours_comments_and_forms", "crates/print/src/tests/appearances.rs::annotation_appearances_do_not_replace_page_xobjects"] +notes = "Print flag honoured; appearance resource names preserve existing page XObjects and distinct annotations, including inherited resources" [[feature]] id = "print.print-with-comment-summary" From 1a69e508261ccb992b1cc34774be073cbea0aced Mon Sep 17 00:00:00 2001 From: Kyle Kassen <85040622+KyleKassen@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:32:46 -0500 Subject: [PATCH 21/26] M10: document annotation resource collision regression --- crates/print/src/tests/appearances.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/print/src/tests/appearances.rs b/crates/print/src/tests/appearances.rs index 325841987..0d3107d1d 100644 --- a/crates/print/src/tests/appearances.rs +++ b/crates/print/src/tests/appearances.rs @@ -85,6 +85,8 @@ fn xobject(doc: &Document, owner: &Dict, name: &[u8]) -> ObjRef { objects.as_dict().unwrap().get(name).unwrap().as_ref().unwrap() } +/// Printing keeps occupied page resource names and distinct annotation appearances intact, +/// whether the page resources are direct or inherited from the page tree. #[test] fn annotation_appearances_do_not_replace_page_xobjects() { for inherited in [false, true] { From 184e9f30ffa1b67e36f6b417d66a3788a3257098 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 11:22:46 +0800 Subject: [PATCH 22/26] M10: a booklet decides two-sided itself, as Acrobat does (#2707) dialog hides Two-sided while Booklet is chosen and prints the booklet two-sided itself (Using Acrobat X Pro, Print dialog box options: "Acrobat automatically enables duplex printing, if available, for booklet printing"; later manuals and the current help no longer say so, and on a current Acrobat the Two-sided control is shown for Size and hidden for Booklet with the same duplex-capable printer). print::booklet_duplex gives a booklet's setting: both sides flip on the short edge, the fold of a 2-up sheet turned sideways; front or back only is one pass of a manual duplex and prints one-sided. The Print dialog hides Two-sided while Booklet is chosen and sends that setting, leaving the Two-sided choice as it was for the other layouts (and as #2001 remembers it); doc_print applies the same rule. This replaces the first version, which rewrote Two-sided to the short edge when Booklet was chosen. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/printing.rs | 25 +++++++++++++++- crates/automation/src/tools.rs | 2 +- crates/print/src/lib.rs | 13 +++++++++ crates/print/src/tests.rs | 8 ++++++ crates/ui-egui/src/print_ui.rs | 47 +++++++++++++++++++------------ crates/ui-egui/tests/it/print.rs | 28 ++++++++++++++++++ parity/acrobat-features.toml | 4 +-- 7 files changed, 105 insertions(+), 22 deletions(-) diff --git a/crates/automation/src/printing.rs b/crates/automation/src/printing.rs index 39cd45760..6c122feb6 100644 --- a/crates/automation/src/printing.rs +++ b/crates/automation/src/printing.rs @@ -22,6 +22,16 @@ fn duplex(value: &str, cut_stack: bool) -> Result> { }) } +/// [`duplex`] for a job laid out as `layout`: the name is checked either way, and a booklet +/// decides two-sided itself (#2707), as the Print dialog does (`print::booklet_duplex`). +fn job_duplex(value: &str, layout: &Layout) -> Result> { + let chosen = duplex(value, matches!(layout, Layout::Multiple { order: PageOrder::CutStack, .. }))?; + Ok(match layout { + Layout::Booklet { subset, .. } => Some(print::booklet_duplex(*subset)), + _ => chosen, + }) +} + fn bad(m: impl Into) -> ToolError { ToolError::InvalidArgs(m.into()) } @@ -108,7 +118,7 @@ impl Automation { }, l => return Err(bad(format!("unknown layout {l:?}"))), }; - let duplex = duplex(a.opt_str("duplex")?.unwrap_or("off"), matches!(layout, Layout::Multiple { order: PageOrder::CutStack, .. }))?; + let duplex = job_duplex(a.opt_str("duplex")?.unwrap_or("off"), &layout)?; if a.opt_str("order")? == Some("cut-stack") && !matches!(layout, Layout::Multiple { .. }) { return Err(bad("cut-stack order needs layout multiple")); } @@ -195,6 +205,19 @@ impl Automation { mod tests { use super::*; + /// #2707: a booklet sets two-sided itself whatever `duplex` asks; a bad name is still an error. + #[test] + fn a_booklet_decides_two_sided_itself() { + let booklet = |subset| Layout::Booklet { subset, binding: Binding::Left }; + for asked in ["printer", "off", "long-edge", "short-edge"] { + assert_eq!(job_duplex(asked, &booklet(BookletSubset::BothSides)).ok(), Some(Some(spool::Duplex::ShortEdge)), "{asked}"); + assert_eq!(job_duplex(asked, &booklet(BookletSubset::FrontOnly)).ok(), Some(Some(spool::Duplex::Off)), "{asked}"); + assert_eq!(job_duplex(asked, &booklet(BookletSubset::BackOnly)).ok(), Some(Some(spool::Duplex::Off)), "{asked}"); + } + assert!(job_duplex("sideways", &booklet(BookletSubset::BothSides)).is_err()); + assert_eq!(job_duplex("long-edge", &Layout::Size(SizeMode::Fit)).ok(), Some(Some(spool::Duplex::LongEdge)), "other layouts keep it"); + } + /// `printer` leaves two-sided to the printer's own default (#2422); cut and stack still asks /// for single-sided sheets, and refuses an explicit edge. #[test] diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 1e009c117..a4c9b2019 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -543,7 +543,7 @@ pub fn tools() -> Vec { t( "doc_print", "Print", - "Print with Acrobat's Print dialog options, or save the print-ready PDF. pages: a range such as \"1-3, 6, 9-\" (page labels allowed; default all); subset odd/even (by page number, as Acrobat counts them: pages 2, 7-10 with even prints 2, 8 and 10, not every second page of the range); reverse. layout: fit (default), actual, shrink, custom (scale %), multiple (per_sheet 2/4/6/9/16, order, border, auto_rotate; cut-stack order arranges single-sided sheets for cutting into piles and stacking left to right, top to bottom, keeping sheet order within each pile; duplex must be off), booklet (booklet_subset both/front/back, binding left/right), poster (scale %, overlap pt, cut_marks). orientation auto/portrait/landscape; comments_forms document / document-and-markups (default) / document-and-stamps / form-fields-only; paper Letter/Legal/Tabloid, ARCH A/B/C/D/E1/E, ANSI C/D/E, A0/A1/A2/A3/A4/A5/A6, or US #10 Envelope (short names or full preset labels); use layout actual for 100% in the print-ready PDF; printer drivers may still substitute paper or scale. Then path (save) or printer (a name or \"default\") with copies, collate, duplex off/long-edge/short-edge, grayscale, and options (driver options from printer_options, CUPS).", + "Print with Acrobat's Print dialog options, or save the print-ready PDF. pages: a range such as \"1-3, 6, 9-\" (page labels allowed; default all); subset odd/even (by page number, as Acrobat counts them: pages 2, 7-10 with even prints 2, 8 and 10, not every second page of the range); reverse. layout: fit (default), actual, shrink, custom (scale %), multiple (per_sheet 2/4/6/9/16, order, border, auto_rotate; cut-stack order arranges single-sided sheets for cutting into piles and stacking left to right, top to bottom, keeping sheet order within each pile; duplex must be off), booklet (booklet_subset both/front/back, binding left/right; a booklet sets two-sided itself, as Acrobat does: both flips on the short edge, front or back prints one-sided), poster (scale %, overlap pt, cut_marks). orientation auto/portrait/landscape; comments_forms document / document-and-markups (default) / document-and-stamps / form-fields-only; paper Letter/Legal/Tabloid, ARCH A/B/C/D/E1/E, ANSI C/D/E, A0/A1/A2/A3/A4/A5/A6, or US #10 Envelope (short names or full preset labels); use layout actual for 100% in the print-ready PDF; printer drivers may still substitute paper or scale. Then path (save) or printer (a name or \"default\") with copies, collate, duplex off/long-edge/short-edge, grayscale, and options (driver options from printer_options, CUPS).", ) .with(schema( json!({ diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index db65492c1..a753768e0 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -149,6 +149,19 @@ pub enum BookletSubset { BackOnly, } +/// How a booklet goes through the printer (#2707): it decides two-sided printing itself, as +/// Acrobat does (its Print dialog hides Two-sided while Booklet is chosen; Using Acrobat X Pro, +/// Print dialog box options: "Acrobat automatically enables duplex printing, if available, for +/// booklet printing"). Both sides flip on the short edge, the fold of a 2-up sheet turned +/// sideways; front side only or back side only is one pass of a manual duplex and prints +/// single-sided. +pub fn booklet_duplex(subset: BookletSubset) -> spool::Duplex { + match subset { + BookletSubset::BothSides => spool::Duplex::ShortEdge, + BookletSubset::FrontOnly | BookletSubset::BackOnly => spool::Duplex::Off, + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] pub enum Binding { #[default] diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index 84ab248c7..9ca376cb8 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -961,3 +961,11 @@ fn odd_and_even_pages_are_page_numbers_not_places_in_the_range() { // A range with no page of the kind prints nothing. assert_eq!(select_pages(12, Some("3, 5"), &[], Subset::Even, false), Err(PrintError::NoPages)); } + +/// #2707: a booklet's two-sided setting follows its subset. +#[test] +fn a_booklet_prints_both_sides_on_the_short_edge() { + assert_eq!(booklet_duplex(BookletSubset::BothSides), spool::Duplex::ShortEdge); + assert_eq!(booklet_duplex(BookletSubset::FrontOnly), spool::Duplex::Off); + assert_eq!(booklet_duplex(BookletSubset::BackOnly), spool::Duplex::Off); +} diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index cab6bfc23..ac60fb54c 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -228,8 +228,15 @@ impl PrintDraft { printer: self.printer.clone(), copies: self.copies.max(1), collate: self.collate, - // Cut and stack needs single-sided sheets, whatever the printer's default. - duplex: if self.cut_and_stack() { Some(spool::Duplex::Off) } else { self.duplex }, + // Cut and stack needs single-sided sheets, whatever the printer's default; a booklet + // decides two-sided itself (#2707), leaving the Two-sided choice as it was. + duplex: if self.cut_and_stack() { + Some(spool::Duplex::Off) + } else if self.handling == Handling::Booklet { + Some(print::booklet_duplex(self.booklet_subset)) + } else { + self.duplex + }, scaling: spool::Scaling::Exact, grayscale: self.grayscale, title: title.to_string(), @@ -1017,22 +1024,26 @@ pub(crate) fn body( }); }); ui.end_row(); - ui.label(tl!("Two-sided:")); - ui.add_enabled_ui(!browser, |ui| { - combo( - ui, - "duplex", - &mut d.duplex, - &[ - (None, tl!("Printer default")), - (Some(spool::Duplex::Off), tl!("Off")), - (Some(spool::Duplex::LongEdge), tl!("Flip on long edge")), - (Some(spool::Duplex::ShortEdge), tl!("Flip on short edge")), - ], - 160.0, - ); - }); - ui.end_row(); + // A booklet decides two-sided itself (#2707), so the choice is hidden meanwhile, as + // in Acrobat's Print dialog; it is kept for the other layouts. + if d.handling != Handling::Booklet { + ui.label(tl!("Two-sided:")); + ui.add_enabled_ui(!browser, |ui| { + combo( + ui, + "duplex", + &mut d.duplex, + &[ + (None, tl!("Printer default")), + (Some(spool::Duplex::Off), tl!("Off")), + (Some(spool::Duplex::LongEdge), tl!("Flip on long edge")), + (Some(spool::Duplex::ShortEdge), tl!("Flip on short edge")), + ], + 160.0, + ); + }); + ui.end_row(); + } ui.label(tl!("Paper:")); let paper_name = PAPERS.get(d.paper).map_or("US Letter", |p| p.0); let before = d.paper; diff --git a/crates/ui-egui/tests/it/print.rs b/crates/ui-egui/tests/it/print.rs index ae325eaaa..21aeca128 100644 --- a/crates/ui-egui/tests/it/print.rs +++ b/crates/ui-egui/tests/it/print.rs @@ -873,3 +873,31 @@ fn restored_paper_keeps_preview_and_saved_sheet_dimensions() { assert!((size.0 - a4.0).abs() < 0.01 && (size.1 - a4.1).abs() < 0.01, "saved sheet: {size:?}"); std::fs::remove_file(out).unwrap(); } + +/// #2707: a booklet decides two-sided itself, as Acrobat does (its Print dialog hides Two-sided +/// while Booklet is chosen): the job flips on the short edge for both sides and prints +/// one-sided for the front or back only, and the Two-sided choice is kept for other layouts. +#[test] +fn a_booklet_decides_two_sided_itself() { + use pdfcraft_engine::print::{BookletSubset, spool::Duplex}; + let mut h = harness(); + assert!(h.state_mut().execute("print.dialog")); + h.run_steps(3); + h.state_mut().print_draft.duplex = Some(Duplex::LongEdge); + h.run_steps(1); + assert!(h.query_by_label("Two-sided:").is_some()); + h.get_by_label("Booklet").click(); + h.run_steps(2); + assert!(h.query_by_label("Two-sided:").is_none(), "hidden while Booklet is chosen"); + let d = &h.state().print_draft; + assert_eq!(d.duplex, Some(Duplex::LongEdge), "the Two-sided choice is kept"); + assert_eq!(d.job("x").duplex, Some(Duplex::ShortEdge), "both sides flip on the short edge"); + for subset in [BookletSubset::FrontOnly, BookletSubset::BackOnly] { + h.state_mut().print_draft.booklet_subset = subset; + assert_eq!(h.state().print_draft.job("x").duplex, Some(Duplex::Off), "{subset:?} is one pass of a manual duplex"); + } + h.get_by_label("Size").click(); + h.run_steps(2); + assert!(h.query_by_label("Two-sided:").is_some()); + assert_eq!(h.state().print_draft.job("x").duplex, Some(Duplex::LongEdge), "back to the choice"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 4ec39848c..a72350e4b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -8073,8 +8073,8 @@ milestone = "M10" status = "shipped" commands = ["print.dialog"] tools = ["doc_print"] -evidence = ["crates/print/src/tests.rs::booklets_pair_pages_for_folding"] -notes = "both/front/back, left/right binding" +evidence = ["crates/print/src/tests.rs::booklets_pair_pages_for_folding", "crates/ui-egui/tests/print.rs::a_booklet_decides_two_sided_itself", "crates/automation/src/printing.rs::a_booklet_decides_two_sided_itself", "crates/print/src/tests.rs::a_booklet_prints_both_sides_on_the_short_edge"] +notes = "both/front/back, left/right binding; a booklet decides two-sided itself, as Acrobat does (its Print dialog hides Two-sided while Booklet is chosen; Using Acrobat X Pro: Acrobat automatically enables duplex printing for booklet printing): both sides flip on the short edge, front or back only prints one-sided, and the Two-sided choice is kept for the other layouts (#2707)" [[feature]] id = "print.poster" From d64a698825e649c37f4a34b86263d10aba6bd379 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:39:26 +0800 Subject: [PATCH 23/26] fix(parity): cite the booklet print test at its path under tests/it (#2707) main moved the ui-egui integration tests into tests/it, so xtask parity could no longer find print.booklet's ui-egui evidence. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC --- parity/acrobat-features.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index a72350e4b..4cc2c31dc 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -8073,7 +8073,7 @@ milestone = "M10" status = "shipped" commands = ["print.dialog"] tools = ["doc_print"] -evidence = ["crates/print/src/tests.rs::booklets_pair_pages_for_folding", "crates/ui-egui/tests/print.rs::a_booklet_decides_two_sided_itself", "crates/automation/src/printing.rs::a_booklet_decides_two_sided_itself", "crates/print/src/tests.rs::a_booklet_prints_both_sides_on_the_short_edge"] +evidence = ["crates/print/src/tests.rs::booklets_pair_pages_for_folding", "crates/ui-egui/tests/it/print.rs::a_booklet_decides_two_sided_itself", "crates/automation/src/printing.rs::a_booklet_decides_two_sided_itself", "crates/print/src/tests.rs::a_booklet_prints_both_sides_on_the_short_edge"] notes = "both/front/back, left/right binding; a booklet decides two-sided itself, as Acrobat does (its Print dialog hides Two-sided while Booklet is chosen; Using Acrobat X Pro: Acrobat automatically enables duplex printing for booklet printing): both sides flip on the short edge, front or back only prints one-sided, and the Two-sided choice is kept for the other layouts (#2707)" [[feature]] From 42910de473d5407b7d02337b0e15745e2c47fa0e Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:56:46 +0000 Subject: [PATCH 24/26] Reject open-ended page range on a document with no pages Found while testing crates/print/src/range.rs:69-76 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/print/src/range.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/print/src/range.rs b/crates/print/src/range.rs index d03f31a3c..f0d8939da 100644 --- a/crates/print/src/range.rs +++ b/crates/print/src/range.rs @@ -70,7 +70,7 @@ pub fn select_pages(count: usize, range: Option<&str>, labels: &[String], subset Some(_) if labels.iter().any(|l| l == part) => pages.push(page_of(part, count, labels)?), Some((a, b)) => { let from = if a.trim().is_empty() { 0 } else { page_of(a, count, labels)? }; - let to = if b.trim().is_empty() { count.saturating_sub(1) } else { page_of(b, count, labels)? }; + let to = if b.trim().is_empty() { count.checked_sub(1).ok_or(PrintError::NoPages)? } else { page_of(b, count, labels)? }; if from <= to { pages.extend(from..=to); } else { @@ -115,3 +115,18 @@ fn narrow(pages: Vec, subset: Subset, reverse: bool) -> Result } Ok(out) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn open_ended_range_of_empty_document_is_an_error() { + assert!(select_pages(0, Some("-"), &[], Subset::All, false).is_err()); + } + + #[test] + fn open_ended_range_covers_all_pages() { + assert_eq!(select_pages(5, Some("-"), &[], Subset::All, false).unwrap(), vec![0, 1, 2, 3, 4]); + } +} From 63510e29eb0e73d1aa1fed62c9c0a0f3848103a9 Mon Sep 17 00:00:00 2001 From: Eric Kaden <608341+EricKaden@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:12:33 -0400 Subject: [PATCH 25/26] test(ui): docstrings for the recovery tests' folder helper --- crates/ui-egui/src/recovery.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ui-egui/src/recovery.rs b/crates/ui-egui/src/recovery.rs index c21e2366b..8870878ed 100644 --- a/crates/ui-egui/src/recovery.rs +++ b/crates/ui-egui/src/recovery.rs @@ -435,12 +435,15 @@ mod idle_tests { std::fs::create_dir(&path).unwrap(); Self(path) } + + /// The folder. fn path(&self) -> &Path { &self.0 } } impl Drop for TestDir { + /// Remove the folder and what the test left in it. fn drop(&mut self) { let _ = std::fs::remove_dir_all(&self.0); } From 70d92a052bbc19ff60fa31578ad37275d499e5ee Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 06:21:39 +0000 Subject: [PATCH 26/26] Integration: #2289 keep Contents, BM, Lang and AF on the split widget --- crates/organize/src/import.rs | 4 ++++ crates/organize/src/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 15087a0c5..3e56c0d53 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -59,6 +59,10 @@ const WIDGET_KEYS: &[&[u8]] = &[ b"OC", b"Tabs", b"A", + b"Contents", + b"BM", + b"Lang", + b"AF", ]; struct Copier<'a> { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index a9364e83f..85c319915 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -1014,6 +1014,30 @@ fn splitting_a_combined_widget_keeps_annotation_actions_and_field_triggers() { } } +#[test] +fn splitting_a_combined_widget_keeps_its_annotation_only_keys() { + let mut src = doc_a(); + let list = page_dict(&src, 1).get(b"Annots").and_then(|a| a.as_array()).cloned().unwrap(); + let widget = + list.iter().filter_map(Object::as_ref).find(|r| src.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))).unwrap(); + src.update_dict(widget, |d| { + d.set(b"Contents".to_vec(), Object::String(PdfString::text("alt text"))); + d.set(b"BM".to_vec(), Object::name("Multiply")); + d.set(b"Lang".to_vec(), Object::String(PdfString::text("en-US"))); + d.set(b"AF".to_vec(), Object::Array(Vec::new())); + }) + .unwrap(); + let out = full_roundtrip(&extract_pages(&src, &[1, 1]).unwrap()); + for i in 0..2 { + let w = annots(&out, i).into_iter().find(|d| d.name(b"Subtype") == Some(b"Widget")).unwrap(); + let parent = out.get(w.reference(b"Parent").unwrap()).as_dict().unwrap().clone(); + for key in [b"Contents".as_slice(), b"BM", b"Lang", b"AF"] { + assert!(w.contains(key), "the widget keeps {key:?}"); + assert!(!parent.contains(key), "the parent field does not take {key:?}"); + } + } +} + #[test] fn repeated_page_gets_independent_annotations() { // A1 has a link, A2 a link and a form field, A3 a note with a popup; each is listed twice.