From 021812b6eeededdd343042be50acffc140b86558 Mon Sep 17 00:00:00 2001 From: Guillem Hermida Date: Fri, 16 Oct 2026 16:08:17 +0200 Subject: [PATCH 01/23] fix(ui): the password prompt takes typing on Linux again (#2818) The prompt's field requested focus on every frame. In egui 0.36 each request also interrupts the input method, and egui-winit answers with set_ime_allowed(false) then (true), which recreates the X11 input context (or re-enables Wayland text-input) every frame. On Linux typed keys go through the input method, so nothing typed reached the field. Request focus only when the field doesn't have it, as the command search does (#1638). The permissions password prompt shares the box. Tests type through synthetic IME pre-edits and commits after opening an encrypted file from the command line, File > Open, the start page and a drop, and into the permissions prompt; both fail without the fix. --- crates/ui-egui/src/dialogs.rs | 10 ++- crates/ui-egui/tests/pickers.rs | 104 ++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 6 +- 3 files changed, 114 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 2d2407629c..f68edd7490 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1696,12 +1696,16 @@ fn password_modal( ui.add(egui::Label::new(message).wrap()); ui.add_space(8.0); let r = ui.add(widgets::line(input).password(true).hint_text(tl!("Password")).desired_width(f32::INFINITY)); - // Enter submits. The field keeps focus (we request it every frame), so check the key - // while it is focused as well as on the frame focus is lost. + // Enter submits. The field keeps focus, so check the key while it is focused as well as + // on the frame focus is lost. if (r.has_focus() || r.lost_focus()) && ui.input(|i| i.key_pressed(egui::Key::Enter)) { answer = Some(PasswordAnswer::Submit); } - r.request_focus(); + // Only when it lost focus: each request interrupts the input method, and on Linux typed + // keys go through it, so asking on every frame lost everything typed (#2818). + if !r.has_focus() { + r.request_focus(); + } if let Some(e) = error { ui.add_space(4.0); // Known error text is translated; anything else passes through. diff --git a/crates/ui-egui/tests/pickers.rs b/crates/ui-egui/tests/pickers.rs index e7db8cffa6..b5115df5f0 100644 --- a/crates/ui-egui/tests/pickers.rs +++ b/crates/ui-egui/tests/pickers.rs @@ -691,6 +691,110 @@ fn unlocking_a_late_startup_url_keeps_it_in_the_background() { assert_eq!(h.state().active, Some(1)); } +/// How an encrypted file reaches the password prompt. +#[derive(Clone, Copy, Debug)] +enum OpenedFrom { + CommandLine, + FileOpen, + StartPage, + Drop, +} + +#[derive(Debug)] +struct Dropped(std::path::PathBuf, Vec); + +impl egui::DroppedFile for Dropped { + fn path(&self) -> &std::path::Path { + &self.0 + } + fn bytes(&self) -> Result, String> { + Ok(self.1.clone()) + } +} + +/// Types `text` the way keys arrive where egui-winit hands text fields to an input method, as on +/// Linux (X11 XIM, Wayland text-input): each character is with the input method (a pre-edit) for +/// a frame before it is committed. A frame that asks to interrupt the composition makes +/// egui-winit switch the input method off and on again, which drops what it holds, so the commit +/// never comes. +fn type_through_input_method(h: &mut Harness<'static, PdfCraftApp>, text: &str) { + for c in text.chars() { + let c = c.to_string(); + h.event(egui::Event::Ime(egui::ImeEvent::Preedit { text: c.clone(), active_range_chars: Some(0..1) })); + h.step(); + let interrupted = h.output().platform_output.ime.as_ref().is_none_or(|ime| ime.should_interrupt_composition); + let next = if interrupted { egui::ImeEvent::Preedit { text: String::new(), active_range_chars: None } } else { egui::ImeEvent::Commit(c) }; + h.event(egui::Event::Ime(next)); + h.step(); + } +} + +/// #2818: nothing typed into the password prompt arrived on Linux. The field asked for focus on +/// every frame, and each request also interrupts the input method. +#[test] +fn the_open_password_can_be_typed_through_an_input_method_however_the_file_is_opened() { + let bytes = encrypted_fixture(); + for from in [OpenedFrom::CommandLine, OpenedFrom::FileOpen, OpenedFrom::StartPage, OpenedFrom::Drop] { + let dir = scratch(&format!("password-{from:?}")); + let file = dir.join("encrypted.pdf"); + std::fs::write(&file, &bytes).unwrap(); + let path = file.to_string_lossy().into_owned(); + let arg = path.clone(); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + // As `main` opens the files named on the command line. + if matches!(from, OpenedFrom::CommandLine) { + app.open_path(&arg); + } + app + }); + h.run_steps(4); + match from { + OpenedFrom::CommandLine => {} + OpenedFrom::FileOpen => { + h.state_mut().pick_override = Some(vec![path.clone()]); + assert!(h.state_mut().execute("file.open")); + } + OpenedFrom::StartPage => { + h.state_mut().pick_override = Some(vec![path.clone()]); + h.get_by_label("Open file").click(); + } + OpenedFrom::Drop => h.input_mut().dropped_files.push(std::sync::Arc::new(Dropped(file.clone(), bytes.clone()))), + } + support::wait_until(&mut h, "password prompt", |app| app.password_prompt.is_some()); + h.run_steps(2); + type_through_input_method(&mut h, "test-password"); + assert_eq!(h.state().password_prompt.as_ref().map(|p| p.input.as_str()), Some("test-password"), "opened from {from:?}"); + h.key_press(egui::Key::Enter); + support::wait_until(&mut h, "unlocked document", |app| app.password_prompt.is_none() && app.views.len() == 1); + let doc = h.state().session.get(h.state().views[0].id).unwrap(); + assert_eq!((doc.path.as_deref(), doc.info.pages.len()), (Some(path.as_str()), 1), "opened from {from:?}"); + let _ = std::fs::remove_dir_all(&dir); + } +} + +/// The permissions password prompt is the same box (#2818). +#[test] +fn the_permissions_password_can_be_typed_through_an_input_method() { + use pdfcraft_engine::{Edit, Protection, Session}; + let mut s = Session::new(); + let id = s.open("locked.pdf", None, std::sync::Arc::new(fixture(1)), None).unwrap(); + s.apply(id, Edit::Protect(Protection { permissions_password: Some("owner".into()), ..Default::default() })).unwrap(); + let locked = s.save_bytes(id).unwrap().as_ref().clone(); + let mut h = harness(&[]); + h.state_mut().open_bytes("locked.pdf", None, locked).unwrap(); + h.run_steps(2); + assert!(h.state_mut().execute("protect.remove")); + h.run_steps(2); + assert!(h.state().unlock_prompt.is_some(), "Remove security asks for the permissions password"); + type_through_input_method(&mut h, "owner"); + assert_eq!(h.state().unlock_prompt.as_ref().map(|p| p.input.as_str()), Some("owner")); + h.key_press(egui::Key::Enter); + h.run_steps(2); + assert!(h.state().unlock_prompt.is_none(), "the typed password is accepted"); +} + #[test] fn a_late_startup_url_preserves_the_combine_files_tab() { let mut h = harness(&[]); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c3d319f94f..b2a1a2c938 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -178,8 +178,8 @@ milestone = "M1" status = "shipped" commands = ["file.open"] tools = ["doc_open"] -evidence = ["crates/ui-egui/tests/editing.rs::password_prompt_opens_and_security_tab_reports_the_details", "crates/crypt/src/tests.rs::empty_user_password_opens_without_asking", "crates/ui-egui/src/open_worker_tests.rs::a_chosen_tool_survives_wrong_password_retry_and_runs_after_unlock", "crates/ui-egui/src/open_worker_tests.rs::startup_intake_waits_for_local_authentication_and_keeps_its_own_background_password_prompt"] -notes = "" +evidence = ["crates/ui-egui/tests/editing.rs::password_prompt_opens_and_security_tab_reports_the_details", "crates/crypt/src/tests.rs::empty_user_password_opens_without_asking", "crates/ui-egui/src/open_worker_tests.rs::a_chosen_tool_survives_wrong_password_retry_and_runs_after_unlock", "crates/ui-egui/src/open_worker_tests.rs::startup_intake_waits_for_local_authentication_and_keeps_its_own_background_password_prompt", "crates/ui-egui/tests/pickers.rs::the_open_password_can_be_typed_through_an_input_method_however_the_file_is_opened"] +notes = "the field takes typing through the platform input method, which typed keys go through on Linux, however the file was opened (#2818); checked with synthetic input-method events, not on a Linux desktop" [[feature]] id = "core.unicode-passwords" @@ -202,7 +202,7 @@ milestone = "M1" status = "shipped" commands = ["protect.unlock"] tools = ["doc_open"] -evidence = ["crates/engine/src/tests.rs::restricted_documents_refuse_changes_unless_opened_by_the_owner", "crates/engine/src/tests.rs::owner_password_of_older_revisions_opens_the_viewer_too", "crates/crypt/src/tests.rs::owner_authentication_recovers_the_user_password", "crates/engine/src/tests.rs::the_permissions_password_unlocks_an_open_document", "crates/ui-egui/tests/editing.rs::the_security_notice_unlocks_with_the_permissions_password", "crates/ui-egui/tests/editing.rs::remove_security_asks_for_the_permissions_password_first"] +evidence = ["crates/engine/src/tests.rs::restricted_documents_refuse_changes_unless_opened_by_the_owner", "crates/engine/src/tests.rs::owner_password_of_older_revisions_opens_the_viewer_too", "crates/crypt/src/tests.rs::owner_authentication_recovers_the_user_password", "crates/engine/src/tests.rs::the_permissions_password_unlocks_an_open_document", "crates/ui-egui/tests/editing.rs::the_security_notice_unlocks_with_the_permissions_password", "crates/ui-egui/tests/editing.rs::remove_security_asks_for_the_permissions_password_first", "crates/ui-egui/tests/pickers.rs::the_permissions_password_can_be_typed_through_an_input_method"] notes = "at open (the password prompt takes either password), or after opening a restricted file that needed no password (#995, #1077): Unlock… on the security notice and in Properties ▸ Security, and changing security asks for it first. Not yet: an automation tool to unlock an open document (reopen with doc_open and the password instead)" [[feature]] From 5157e0421b26e794a2e5fb9ec4adbe048a39d841 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 02:24:39 +0000 Subject: [PATCH 02/23] Keep seconds in FormCalc Time2Num/IsoTime2Num for ISO 8601 times Found while testing crates/js/src/formcalc.rs:2348-2352 ("time2num" | "isotime2num") Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/js/src/formcalc.rs | 13 ++++++++++--- crates/js/src/tests.rs | 3 +++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/js/src/formcalc.rs b/crates/js/src/formcalc.rs index 059276e936..449954a7fe 100644 --- a/crates/js/src/formcalc.rs +++ b/crates/js/src/formcalc.rs @@ -2347,9 +2347,16 @@ impl Interp<'_> { } "time2num" | "isotime2num" => { let s = self.arg_str(args, 0)?; - let parts: Vec = s.split(':').filter_map(|p| p.trim().parse::().ok()).map(|v| v.min(1_000_000)).collect(); - let (h, m, sec) = (parts.first().copied().unwrap_or(0), parts.get(1).copied().unwrap_or(0), parts.get(2).copied().unwrap_or(0)); - Val::Num(((h * 3600 + m * 60 + sec) * 1000) as f64) + // ISO 8601 allows a leading "T", a trailing "Z" and fractional seconds. + let t = s.trim(); + let t = t.strip_prefix(['T', 't']).unwrap_or(t); + let t = t.strip_suffix(['Z', 'z']).unwrap_or(t); + let mut parts = t.split(':'); + let mut whole = || parts.next().and_then(|p| p.trim().parse::().ok()).unwrap_or(0).min(1_000_000); + let (h, m) = (whole(), whole()); + let sec = + parts.next().and_then(|p| p.trim().parse::().ok()).filter(|v| v.is_finite() && *v >= 0.0).unwrap_or(0.0).min(1_000_000.0); + Val::Num(((h * 3600 + m * 60) * 1000) as f64 + (sec * 1000.0).round()) } // ── financial "fv" => { diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 55fa45760d..877dce52bc 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -680,6 +680,9 @@ mod formcalc { assert_eq!(calc(r#"Num2Date(45350, "YYYY-MM-DD")"#), "2024-02-29"); assert_eq!(calc(r#"Num2Date(Date2Num("2024-02-29") + 1, "MMMM D, YYYY")"#), "March 1, 2024"); assert_eq!(calc(r#"Time2Num("01:00:00")"#), "3600000"); + assert_eq!(calc(r#"IsoTime2Num("12:30:45Z")"#), "45045000"); + assert_eq!(calc(r#"IsoTime2Num("12:30:45.250")"#), "45045250"); + assert_eq!(calc(r#"IsoTime2Num("T12:30:45")"#), "45045000"); assert_eq!(calc(r#"Num2Time(3661000)"#), "01:01:01"); assert_eq!(calc("Round(Pmt(10000, 0.01, 12), 2)"), "888.49"); assert_eq!(calc("Round(FV(100, 0.01, 12), 2)"), "1268.25"); From be2f96a8adb7a0e80baf37abf1192938ddd6b787 Mon Sep 17 00:00:00 2001 From: Guillem Hermida Date: Sat, 17 Oct 2026 11:52:51 +0200 Subject: [PATCH 03/23] contributors: credit Guillem Hermida by real name --- contributors/people.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index da3cfc6a77..0ac481a198 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -30,6 +30,9 @@ real_name = "Eduardo Soares" display_name = "Eduardo Soares" sync_github_name = false +[people.guillehr2] +real_name = "Guillem Hermida" + [people.jcl-bit] real_name = "James Locke" display_name = "James Locke" From b913c2851d6a10ff50ab35e4e938e258acb0af02 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 15:39:32 +0200 Subject: [PATCH 04/23] fix(ui): a second file dialog waits for the open one instead of being refused (fixes #2150) --- crates/ui-egui/src/pickers.rs | 115 ++++++++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 20 deletions(-) diff --git a/crates/ui-egui/src/pickers.rs b/crates/ui-egui/src/pickers.rs index 7661b71750..16f96ec82c 100644 --- a/crates/ui-egui/src/pickers.rs +++ b/crates/ui-egui/src/pickers.rs @@ -71,10 +71,25 @@ impl std::fmt::Debug for Picked { } } +/// A request that came while another picker was showing (#2150). +struct Waiting { + ask: Ask, + target: Option, + then: Then, +} + +impl std::fmt::Debug for Waiting { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Waiting").field("target", &self.target).finish_non_exhaustive() + } +} + /// Pickers in flight and the picks they finished. -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub struct Pickers { - /// A picker is showing, or its pick is waiting for the frame; a second request is refused + /// Requests that show, in order, once the picker before them is answered. + waiting: std::collections::VecDeque, + /// A picker is showing, or its pick is waiting for the frame; a second request waits /// until the pick's closure starts. showing: Arc, done: Arc>>, @@ -169,8 +184,9 @@ impl PdfCraftApp { } /// Show `ask` without blocking the frame, and call `then` with the chosen paths on a later - /// frame ([`Self::process_picked`]). Returns false, after telling the user, when no picker - /// could be shown: another one is still open, or its worker couldn't start. + /// frame ([`Self::process_picked`]). While another picker is open, `ask` waits and shows + /// when that one is answered (the user is told). Returns false, after telling the user, + /// when no picker could be shown because its worker couldn't start. /// /// `then` is not called when the picker is cancelled, or when `target` is given and by then /// that document is no longer the active one or has been edited (the user is told). Use @@ -180,12 +196,19 @@ impl PdfCraftApp { /// `pick_override` answers instead of a native panel (tests and automation); the answer /// still arrives on a later frame, like a real pick. pub(crate) fn ask(&mut self, ask: Ask, target: Option, then: impl FnOnce(&mut PdfCraftApp, Vec) + Send + 'static) -> bool { - if self.pickers.showing.swap(true, Ordering::SeqCst) { - self.notify_tr("Another file dialog is still open. Finish with it first."); - return false; - } let target = self.pick_target(target); let then: Then = Box::new(then); + if self.pickers.showing.swap(true, Ordering::SeqCst) { + // Refusing would drop what asked, such as a finished Reduce File Size copy. + self.pickers.waiting.push_back(Waiting { ask, target, then }); + self.notify_tr("Another file dialog is still open. This one will open when it closes."); + return true; + } + self.show_picker(ask, target, then) + } + + /// [`Self::ask`] once `showing` is claimed for it. + fn show_picker(&mut self, ask: Ask, target: Option, then: Then) -> bool { if let Some(paths) = self.pick_override.clone() { self.pickers.deliver(target, paths.into_iter().map(PathBuf::from).collect(), then); return true; @@ -256,6 +279,13 @@ impl PdfCraftApp { self.notify_fmt("That didn't work: an internal error stopped it ({m}).", &[("m", m.as_str())]); } } + // After the closures, so a picker one of them asked for comes before the waiting ones. + if !self.pickers.waiting.is_empty() + && !self.pickers.showing.swap(true, Ordering::SeqCst) + && let Some(Waiting { ask, target, then }) = self.pickers.waiting.pop_front() + { + self.show_picker(ask, target, then); + } } } @@ -359,34 +389,79 @@ mod tests { } #[test] - fn a_second_picker_is_refused_while_one_is_showing() { + fn a_second_picker_waits_for_the_one_showing_and_opens_when_it_closes() { + let mut app = PdfCraftApp::new(); + app.pick_override = Some(vec!["c.pdf".into()]); + let got = Arc::new(Mutex::new(Vec::new())); + let first = got.clone(); + assert!(app.ask(Ask::Save(rfd::AsyncFileDialog::new()), None, move |_, _| first.lock().unwrap().push("first"))); + let second = got.clone(); + assert!(app.ask(Ask::Folder(rfd::AsyncFileDialog::new()), None, move |_, _| second.lock().unwrap().push("second"))); + assert!(app.toast.as_ref().is_some_and(|(m, _)| m.contains("will open when it closes")), "the user is told it will follow"); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec!["first"], "the waiting picker shows only once the first one is answered"); + assert!(app.pickers.showing.load(Ordering::SeqCst), "the waiting picker owns the flag now"); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec!["first", "second"]); + assert!(!app.pickers.showing.load(Ordering::SeqCst)); + } + + #[test] + fn a_waiting_picker_still_opens_when_the_one_showing_is_cancelled() { let mut app = PdfCraftApp::new(); app.pick_override = Some(vec!["c.pdf".into()]); app.pickers.showing.store(true, Ordering::SeqCst); let got = Arc::new(Mutex::new(Vec::new())); let into = got.clone(); - assert!(!app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, move |_, paths| into.lock().unwrap().extend(paths))); + assert!(app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, move |_, paths| into.lock().unwrap().extend(paths))); app.process_picked(); - assert!(got.lock().unwrap().is_empty()); - assert!(app.pickers.showing.load(Ordering::SeqCst), "the showing picker still owns the flag"); - assert!(app.toast.as_ref().is_some_and(|(m, _)| m.contains("Another file dialog")), "the user is told"); + assert!(got.lock().unwrap().is_empty(), "the showing picker still owns the flag"); + app.pickers.deliver(None, vec![], Box::new(|_, _| {})); + app.process_picked(); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("c.pdf")]); + } + + /// #2150: Reduce File Size finishing under an open Save As panel used to drop its bytes. + #[test] + fn a_reduced_copy_finished_under_another_dialog_is_saved_once_it_closes() { + let dir = std::env::temp_dir().join(format!("pdfcraft-ui-pickers-{}-reduce", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let out = dir.join("notes (reduced).pdf"); + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("notes.txt", None, b"hello".to_vec()).unwrap(); + let (_, doc) = app.active_ids().unwrap(); + app.pick_override = Some(vec![out.to_string_lossy().into_owned()]); + // Save As is still open on another tab when the optimizer finishes. + app.pickers.showing.store(true, Ordering::SeqCst); + let reduced = Arc::new(b"%PDF-1.7 reduced".to_vec()); + app.save_optimized(doc, crate::optimize_ui::OptimizeKind::Reduce, Ok((reduced.clone(), String::new()))); + app.pickers.deliver(None, vec![], Box::new(|_, _| {})); + app.process_picked(); + app.process_picked(); + assert_eq!(std::fs::read(&out).ok(), Some(reduced.to_vec()), "the reduced copy is saved where the user chose"); + let _ = std::fs::remove_dir_all(&dir); } #[test] - fn a_follow_up_that_asks_again_gets_its_picker_even_if_another_request_came_first() { + fn a_follow_up_that_asks_again_gets_its_picker_before_a_request_that_came_first() { let mut app = PdfCraftApp::new(); app.pick_override = Some(vec!["first.fdf".into()]); let got = Arc::new(Mutex::new(Vec::new())); let into = got.clone(); assert!(app.ask(Ask::Files(rfd::AsyncFileDialog::new()), None, move |app, _| { // Merge data: the spreadsheet's save picker, asked from the first pick's closure. - assert!(app.ask(Ask::Save(rfd::AsyncFileDialog::new()), None, move |_, paths| into.lock().unwrap().extend(paths))); + assert!(app.ask(Ask::Save(rfd::AsyncFileDialog::new()), None, move |_, _| into.lock().unwrap().push("chained"))); })); - // Another command asks for a picker before the frame uses the first pick: refused. - assert!(!app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, |_, _| panic!("must not run"))); - app.process_picked(); - app.process_picked(); - assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("first.fdf")], "the chained picker answered"); + // Another command asks for a picker before the frame uses the first pick: it waits. + let other = got.clone(); + assert!(app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, move |_, _| other.lock().unwrap().push("other"))); + for _ in 0..4 { + app.process_picked(); + } + assert_eq!(*got.lock().unwrap(), vec!["chained", "other"], "the chained picker answers first"); } #[test] From 6ee8329ec0d518395d4756c0c42a2d9eea2849e2 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Sun, 18 Oct 2026 17:03:20 +0200 Subject: [PATCH 05/23] test(ui): the reduced-copy regression saves a real PDF and reopens its text (#2150) --- crates/ui-egui/src/pickers.rs | 39 ++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/pickers.rs b/crates/ui-egui/src/pickers.rs index 16f96ec82c..da24b5f4dc 100644 --- a/crates/ui-egui/src/pickers.rs +++ b/crates/ui-egui/src/pickers.rs @@ -422,6 +422,32 @@ mod tests { assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("c.pdf")]); } + /// A one-page document showing `text`, with an unencoded content stream long enough that + /// Reduce File Size makes a smaller copy (it saves none otherwise). + fn text_pdf(text: &str) -> Vec { + let content = format!("BT /F1 12 Tf 20 250 Td ({text}) Tj ET\n{}", "q Q\n".repeat(500)); + let objs = [ + "<< /Type /Catalog /Pages 2 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 300] >>".to_string(), + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >>".to_string(), + format!("<< /Length {} >>\nstream\n{content}endstream", content.len()), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_string(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out + } + /// #2150: Reduce File Size finishing under an open Save As panel used to drop its bytes. #[test] fn a_reduced_copy_finished_under_another_dialog_is_saved_once_it_closes() { @@ -431,17 +457,24 @@ mod tests { let out = dir.join("notes (reduced).pdf"); let mut app = PdfCraftApp::new(); app.set_option("language", "en").unwrap(); - app.open_bytes("notes.txt", None, b"hello".to_vec()).unwrap(); + let source = text_pdf("Minutes of the harbour board"); + app.open_bytes("notes.pdf", None, source.clone()).unwrap(); let (_, doc) = app.active_ids().unwrap(); + let (reduced, _) = app.session.optimize_job(doc, &pdfcraft_engine::optimize::Settings::default(), &[]).unwrap().run(|_| true).unwrap(); + assert!(reduced.len() < source.len(), "a copy that isn't smaller would not be saved at all"); app.pick_override = Some(vec![out.to_string_lossy().into_owned()]); // Save As is still open on another tab when the optimizer finishes. app.pickers.showing.store(true, Ordering::SeqCst); - let reduced = Arc::new(b"%PDF-1.7 reduced".to_vec()); app.save_optimized(doc, crate::optimize_ui::OptimizeKind::Reduce, Ok((reduced.clone(), String::new()))); app.pickers.deliver(None, vec![], Box::new(|_, _| {})); app.process_picked(); app.process_picked(); - assert_eq!(std::fs::read(&out).ok(), Some(reduced.to_vec()), "the reduced copy is saved where the user chose"); + let written = std::fs::read(&out).unwrap_or_default(); + assert!(written == *reduced, "the reduced copy is saved where the user chose, byte for byte"); + let mut again = pdfcraft_engine::Session::new(); + let id = again.open("notes (reduced).pdf", None, Arc::new(written), None).unwrap(); + let text = pdfcraft_engine::export::Exporter::new(again.get(id).unwrap()).text(0).unwrap(); + assert!(text.contains("Minutes of the harbour board"), "the saved copy reopens with its text: {text:?}"); let _ = std::fs::remove_dir_all(&dir); } From 31373819f2ab2c009a28e1a4d6dd7ba1b4dedf9b Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 11:42:35 +0200 Subject: [PATCH 06/23] Zero-pad required integer digits in FormCalc Format() Found while testing crates/js/src/formcalc.rs:2240-2276 ("format" builtin, number branch) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/js/src/formcalc.rs | 5 ++++- crates/js/src/tests.rs | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/js/src/formcalc.rs b/crates/js/src/formcalc.rs index cf82eff466..0b12347bd0 100644 --- a/crates/js/src/formcalc.rs +++ b/crates/js/src/formcalc.rs @@ -2261,8 +2261,11 @@ impl Interp<'_> { } let num = self.to_num(&v); let decimals = pic.split_once('.').map_or(0, |(_, f)| f.chars().filter(|c| matches!(c, '9' | 'z' | 'Z')).count()).min(20); + // `9` in the integer part forces a digit; `z` does not. + let min_int = pic.split('.').next().map_or(0, |i| i.chars().filter(|c| *c == '9').count()).min(20); + let width = min_int + if decimals > 0 { decimals + 1 } else { 0 }; let grouped = pic.contains(','); - let mut text = fixed(num.abs(), decimals); + let mut text = format!("{:0>width$}", fixed(num.abs(), decimals)); if grouped { let (int, frac) = text.split_once('.').map_or((text.as_str(), ""), |(a, b)| (a, b)); let mut g = String::new(); diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 4fe5fd2906..3f4647d169 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -705,6 +705,15 @@ mod formcalc { assert!(go("Nope(1)", "form1[0].page1[0].total[0]", "calculate").error.unwrap().contains("unknown function")); } + #[test] + fn format_nine_pads_the_integer_part() { + assert_eq!(calc(r#"Format("99999", 42)"#), "00042"); + assert_eq!(calc(r#"Format("9999", 5)"#), "0005"); + assert_eq!(calc(r#"Format("999.99", 4.5)"#), "004.50"); + assert_eq!(calc(r#"Format("zzz9", 42)"#), "42"); + assert_eq!(calc(r#"Format("99", 12345)"#), "12345"); + } + #[test] fn date2num_rejects_days_past_the_end_of_the_month() { assert_eq!(calc(r#"Date2Num("2023-02-30")"#), "0"); From 031532b8a563f0dee20a2ec284f63ed15640fccb Mon Sep 17 00:00:00 2001 From: PerchAnton <101743951+PerchAnton@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:01:43 +0300 Subject: [PATCH 07/23] M4: bound named-destination pruning during page deletion (#3309) --- ROADMAP.md | 3 +- crates/automation/README.md | 7 ++ crates/automation/tests/automation.rs | 84 +++++++++++++++++++++++ crates/organize/src/lib.rs | 69 ++++++++++++------- crates/organize/src/tests.rs | 96 +++++++++++++++++++++++++++ docs/roadmap.md | 6 +- parity/acrobat-features.toml | 4 +- 7 files changed, 240 insertions(+), 29 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 907beb98a4..a3def841d0 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-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; M4 #3309: bounded iterative destination pruning; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M3, #3149: discard render demand after page geometry changes and check the GPU texture limit before upload; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M4, #1919: repeated page indices accumulate rotations; M3, #3149: discard render demand after page geometry changes and check the GPU texture limit before upload; estimates unchanged) @@ -169,6 +169,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-18 (M4, #3309):** Deleting a page prunes named destinations without recursive calls. A shared 100,000-entry budget bounds nodes, names and child edges, including repeated edges; exceeding it refuses the engine edit without losing unsaved work or history. Synthetic deep/cyclic graph, save/reopen and automation regressions cover the change. Estimates unchanged. - **2026-10-15 (M7, Edit text caret):** In a text region that mixes sizes (a 24 pt title over 12 pt lines), the inline editor's caret on a small row was as tall as the region's first size and reached into the row below: egui draws the caret at least one row of the text field's font tall, and that font had the paragraph's first size. The field's font is now half the region's smallest size, so each row's own height decides. Kittest regression; checked in the app on a real document. Estimates unchanged. - **2026-10-16 (M3, #2781):** The Pages panel follows the current page, as Acrobat's page thumbnails do: going to a page whose thumbnail is out of sight (scrolling, Page Down, Go to page, a bookmark) scrolls the panel just enough to show it. A thumbnail already in view, or clicked, leaves the panel where it is, and a page change while the user scrolls the panel or drags thumbnails does not move it. Estimates unchanged. - **2026-10-16 (M12, #1645):** CLI info, text and render use the engine's dynamic XFA layout rather than placeholder pages. Synthetic two-page and encrypted-form regressions cover counts, text, pixels, copy permissions and unchanged input bytes. Estimates unchanged. diff --git a/crates/automation/README.md b/crates/automation/README.md index 4e27702a32..de0d0697cc 100644 --- a/crates/automation/README.md +++ b/crates/automation/README.md @@ -194,6 +194,13 @@ unapplied with a warning. folders. The CLI also exposes it as `pdfcraft-cli batch`. See the [batch guide](../../docs/cli-batch.md) for examples, output protection, errors and limits. +## Page deletion limits + +`page_delete` prunes named destinations iteratively. Its shared traversal budget is 100,000 +nodes and `/Names` and `/Kids` array entries, counting repeated child edges. Exceeding it +returns a named-destination work-limit error; the engine keeps the PDF, unsaved edits and +undo/redo history unchanged. Deep trees within the budget do not consume recursive stack space. + ## Viewer navigation history A running PdfCraft instance launched with `--control FILE` exposes its per-tab history through diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index f3bcfd21a4..f00b963b59 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -779,6 +779,90 @@ fn edit_undo_redo_and_save_round_trip() { assert_eq!(after.windows(5).filter(|w| w == b"%%EOF").count(), 2); } +/// A shallow-object destination graph: a deep chain, or a wide repeated-child array. +fn destination_graph_fixture(depth: usize, wide: bool) -> Vec { + use pdfcraft_cos::{Dict, Document, Object, PdfString, SaveOptions}; + let mut doc = Document::open(std::sync::Arc::new(fixture(3))).unwrap(); + let pages = pdfcraft_organize::pages(&doc).unwrap(); + let mut leaf = Dict::new(); + leaf.set( + b"Names".to_vec(), + Object::Array(vec![ + Object::String(PdfString::text("gone")), + Object::Array(vec![Object::Ref(pages[0].obj), Object::name("Fit")]), + Object::String(PdfString::text("kept")), + Object::Array(vec![Object::Ref(pages[2].obj), Object::name("Fit")]), + ]), + ); + let mut tree = doc.add(leaf); + if wide { + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(tree); 100_000])); + tree = doc.add(node); + } else { + for _ in 0..depth { + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(tree)])); + tree = doc.add(node); + } + } + let mut names = Dict::new(); + names.set(b"Dests".to_vec(), Object::Ref(tree)); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"Names".to_vec(), Object::Dict(names))).unwrap(); + pdfcraft_cos::write_incremental(&doc, &SaveOptions::default()).unwrap() +} + +#[test] +fn page_delete_handles_deep_destination_trees_with_history_and_reopen() { + let dir = workdir("deep-destinations"); + let original = destination_graph_fixture(16_384, false); + std::fs::write(dir.join("deep.pdf"), &original).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path":"deep.pdf"}))["doc"].as_u64().unwrap(); + let render = a.call("page_render", &json!({"doc":doc,"page":2,"dpi":72})).unwrap(); + ok(&mut a, "page_delete", json!({"doc":doc,"pages":[1]})); + assert_eq!(page_text(&mut a, doc), ["Page 2", "Page 3"]); + assert_eq!(a.call("page_render", &json!({"doc":doc,"page":1,"dpi":72})).unwrap(), render); + ok(&mut a, "edit_undo", json!({"doc":doc})); + assert_eq!(page_text(&mut a, doc), ["Page 1", "Page 2", "Page 3"]); + ok(&mut a, "edit_redo", json!({"doc":doc})); + for full in [false, true] { + let path = format!("saved-{full}.pdf"); + ok(&mut a, "doc_save", json!({"doc":doc,"path":path,"full":full})); + let reopened = ok(&mut a, "doc_open", json!({"path":path}))["doc"].as_u64().unwrap(); + assert_eq!(page_text(&mut a, reopened), ["Page 2", "Page 3"]); + assert_eq!(a.call("page_render", &json!({"doc":reopened,"page":1,"dpi":72})).unwrap(), render); + } + assert_eq!(std::fs::read(dir.join("deep.pdf")).unwrap(), original); +} + +#[test] +fn page_delete_destination_budget_preserves_unsaved_work_and_history() { + let dir = workdir("wide-destinations"); + let original = destination_graph_fixture(0, true); + std::fs::write(dir.join("wide.pdf"), &original).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path":"wide.pdf"}))["doc"].as_u64().unwrap(); + ok(&mut a, "doc_set_info", json!({"doc":doc,"key":"Title","value":"Unsaved work"})); + let info = ok(&mut a, "doc_info", json!({"doc":doc})); + let before = a.session().get(pdfcraft_engine::DocId(doc)).unwrap(); + let bytes = before.bytes.clone(); + let generation = before.edit_generation(); + let undo = before.can_undo().map(str::to_owned); + assert!( + matches!(err(&mut a, "page_delete", json!({"doc":doc,"pages":[1]})), ToolError::Failed(message) if message.contains("named destination tree") && message.contains("100000-entry work limit")) + ); + let after = a.session().get(pdfcraft_engine::DocId(doc)).unwrap(); + assert!(std::sync::Arc::ptr_eq(&after.bytes, &bytes)); + assert_eq!(after.edit_generation(), generation); + assert_eq!(after.can_undo(), undo.as_deref()); + assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc})), info); + assert_eq!(page_text(&mut a, doc), ["Page 1", "Page 2", "Page 3"]); + ok(&mut a, "edit_undo", json!({"doc":doc})); + assert_eq!(a.session().get(pdfcraft_engine::DocId(doc)).unwrap().can_undo(), None); + assert_eq!(std::fs::read(dir.join("wide.pdf")).unwrap(), original); +} + /// Issue #1329: an in-place save says it was incremental only when it kept the file it was /// saving over: protecting or sanitizing rewrites the file, and the result says so. #[test] diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 99c228c60f..ff1c3d3e35 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -365,43 +365,60 @@ fn prune_legacy(doc: &Document, dests: &mut Dict, gone: &[ObjRef], dead: &mut Ve /// Prune a name tree held in `slot` (a reference, or a dictionary written inline). A reference is /// updated in place and yields `None`; an inline dictionary that changed is handed back. fn prune_tree_slot(doc: &mut Document, slot: &Object, gone: &[ObjRef], dead: &mut Vec>) -> Result, OrganizeError> { - let mut seen = Vec::new(); - match slot { + let mut stack = Vec::new(); + let mut left = MAX_DESTINATION_TREE_WORK; + let inline = match slot { Object::Ref(r) => { - prune_tree_ref(doc, *r, gone, dead, &mut seen)?; - Ok(None) + stack.push(*r); + None } Object::Dict(d) => { let mut node = d.clone(); - Ok(prune_tree_node(doc, &mut node, gone, dead, &mut seen)?.then_some(node)) + prune_tree_node(doc, &mut node, gone, dead, &mut stack, &mut left)?.then_some(node) + } + _ => None, + }; + // Name trees consist of shallow indirect nodes. Their graph depth must not become + // call-stack depth; the shared budget also bounds wide trees and repeated edges. + let mut seen = std::collections::HashSet::new(); + while let Some(r) = stack.pop() { + if !seen.insert(r) { + continue; + } + let Some(mut node) = doc.get(r).as_dict().cloned() else { continue }; + if prune_tree_node(doc, &mut node, gone, dead, &mut stack, &mut left)? { + doc.update_dict(r, |d| *d = node)?; } - _ => Ok(None), } + Ok(inline) } -fn prune_tree_ref(doc: &mut Document, r: ObjRef, gone: &[ObjRef], dead: &mut Vec>, seen: &mut Vec) -> Result<(), OrganizeError> { - if seen.contains(&r) { - return Ok(()); - } - seen.push(r); - let Some(mut node) = doc.get(r).as_dict().cloned() else { return Ok(()) }; - if prune_tree_node(doc, &mut node, gone, dead, seen)? { - doc.update_dict(r, |d| *d = node)?; - } +/// Count nodes and the entries in their /Names and /Kids arrays, including repeated edges. +const MAX_DESTINATION_TREE_WORK: usize = 100_000; + +fn destination_tree_work(left: &mut usize, count: usize) -> Result<(), OrganizeError> { + *left = left + .checked_sub(count) + .ok_or_else(|| OrganizeError::Invalid(format!("the named destination tree exceeds the {MAX_DESTINATION_TREE_WORK}-entry work limit")))?; Ok(()) } -/// Drop the /Names pairs of one name-tree node that go to a `gone` page, then do the same for its -/// /Kids. True when `node` itself changed (kids held by reference are written back here). +/// Drop the /Names pairs going to a `gone` page, and queue /Kids for the iterative walk. +/// True when `node` itself changed; child nodes are written back by the caller. fn prune_tree_node( - doc: &mut Document, + doc: &Document, node: &mut Dict, gone: &[ObjRef], dead: &mut Vec>, - seen: &mut Vec, + stack: &mut Vec, + left: &mut usize, ) -> Result { + destination_tree_work(left, 1)?; let mut changed = false; - if let Some(pairs) = node.get(b"Names").map(|n| doc.resolve(n)).and_then(|n| n.as_array().cloned()) { + if let Some(names) = node.get(b"Names").map(|n| doc.resolve(n)) + && let Some(pairs) = names.as_array() + { + destination_tree_work(left, pairs.len())?; let mut kept = Vec::with_capacity(pairs.len()); for pair in pairs.chunks(2) { match pair { @@ -418,10 +435,14 @@ fn prune_tree_node( changed = true; } } - let kids = node.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); - for kid in kids { - if let Some(r) = kid.as_ref() { - prune_tree_ref(doc, r, gone, dead, seen)?; + if let Some(children) = node.get(b"Kids").map(|k| doc.resolve(k)) + && let Some(kids) = children.as_array() + { + destination_tree_work(left, kids.len())?; + for kid in kids.iter().rev() { + if let Some(r) = kid.as_ref() { + stack.push(r); + } } } Ok(changed) diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 9bd3eb396a..f7e023e5d6 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -595,6 +595,102 @@ fn deleting_a_page_drops_the_named_destinations_to_it() { // ── Combine / extract / split ────────────────────────────────────────────────────────────────── +#[test] +fn deleting_a_page_prunes_deep_named_destination_trees() { + let mut doc = doc_a(); + let original_pages = pages(&doc).unwrap(); + let mut leaf = Dict::new(); + leaf.set( + b"Names".to_vec(), + Object::Array(vec![ + Object::String(PdfString::text("gone")), + Object::Array(vec![Object::Ref(original_pages[0].obj), Object::name("Fit")]), + Object::String(PdfString::text("kept")), + Object::Array(vec![Object::Ref(original_pages[2].obj), Object::name("Fit")]), + ]), + ); + let leaf = doc.add(leaf); + let mut tree = leaf; + for _ in 0..16_384 { + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(tree)])); + tree = doc.add(node); + } + let mut names = Dict::new(); + names.set(b"Dests".to_vec(), Object::Ref(tree)); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"Names".to_vec(), Object::Dict(names))).unwrap(); + + // Every node is a shallow indirect object: the page operation must not turn the + // document's graph depth into call-stack depth. + delete_pages(&mut doc, &[0]).unwrap(); + for bytes in [write_incremental(&doc, &SaveOptions::default()).unwrap(), write_full(&doc, &SaveOptions::default()).unwrap()] { + let reopened = open(bytes); + assert_eq!(labels(&reopened), ["A2", "A3"]); + let catalog = reopened.get(reopened.root().unwrap()); + let names = reopened.resolve(catalog.as_dict().unwrap().get(b"Names").unwrap()); + let mut node = reopened.resolve(names.as_dict().unwrap().get(b"Dests").unwrap()); + for _ in 0..16_384 { + let child = node.as_dict().unwrap().get(b"Kids").unwrap().as_array().unwrap()[0].clone(); + node = reopened.resolve(&child); + } + let pairs = node.as_dict().unwrap().get(b"Names").and_then(Object::as_array).cloned().unwrap(); + assert_eq!(pairs.len(), 2); + assert_eq!(pairs[0].as_string().unwrap().to_text(), "kept"); + assert_eq!(pairs[1].as_array().unwrap()[0].as_ref(), Some(pages(&reopened).unwrap()[1].obj)); + } +} + +#[test] +fn destination_pruning_visits_shared_and_cyclic_nodes_once() { + for inline_names in [false, true] { + for inline_tree in [false, true] { + let mut doc = doc_a(); + let gone = pages(&doc).unwrap()[0].obj; + let mut leaf = Dict::new(); + leaf.set( + b"Names".to_vec(), + Object::Array(vec![Object::String(PdfString::text("gone")), Object::Array(vec![Object::Ref(gone), Object::name("Fit")])]), + ); + leaf.set(b"Unknown".to_vec(), Object::Int(42)); + let leaf = doc.add(leaf); + let cycle = doc.add(Object::Null); + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(cycle), Object::Ref(leaf), Object::Ref(leaf)])); + doc.set(cycle, Object::Dict(node.clone())); + let mut names = Dict::new(); + names.set(b"Dests".to_vec(), if inline_tree { Object::Dict(node) } else { Object::Ref(cycle) }); + let names = if inline_names { Object::Dict(names) } else { Object::Ref(doc.add(names)) }; + doc.update_dict(doc.root().unwrap(), |d| d.set(b"Names".to_vec(), names)).unwrap(); + + assert_eq!(drop_named_destinations(&mut doc, &[gone]).unwrap(), [b"gone".to_vec()]); + let leaf = doc.get(leaf); + assert_eq!(leaf.as_dict().unwrap().get(b"Names").and_then(Object::as_array).unwrap().len(), 0); + assert_eq!(leaf.as_dict().unwrap().int(b"Unknown"), Some(42)); + } + } +} + +#[test] +fn destination_pruning_counts_repeated_edges_in_its_work_limit() { + for extra in [0, 1] { + let mut doc = doc_a(); + let leaf = doc.add(Dict::new()); + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(leaf); MAX_DESTINATION_TREE_WORK - 2 + extra])); + let mut names = Dict::new(); + names.set(b"Dests".to_vec(), Object::Dict(node)); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"Names".to_vec(), Object::Dict(names))).unwrap(); + let result = delete_pages(&mut doc, &[0]); + if extra == 0 { + result.unwrap(); + } else { + assert!( + matches!(result, Err(OrganizeError::Invalid(message)) if message.contains("named destination tree") && message.contains("100000-entry work limit")) + ); + } + } +} + /// Build a classic-xref PDF from object bodies (object i+1 = bodies[i]). fn build(bodies: &[String], trailer: &str) -> Vec { let mut out = b"%PDF-1.7\n".to_vec(); diff --git a/docs/roadmap.md b/docs/roadmap.md index 9b5100f37e..e6238bd6e0 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # PdfCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M14/#2383 GPU-limit candidate; M4 #3309: bounded iterative destination pruning; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-18 · **Change:** minor (M4, #1919: repeated page indices accumulate rotations; M4: indirect page rotations refreshed upstream; M13: CLI check visits each directory once through aliases and cycles; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -121,7 +121,7 @@ total matches the parity estimate. | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. A custom text match that ends on a reconstructed separator stops before the next glyph (#1315). Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); Lab Bradford adaptation uses LMS white-point ratios, covered by independent neutral/chromatic and direct/Indexed render oracles; inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Literal page-label prefix whitespace is preserved; nine public-inspection cases cover prefix spaces, encodings and empty-prefix controls | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). | -| M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Extract and Split preserve shared Type 3 glyph resource graphs, retaining conservative pruning limits. Undo back to the opened or last saved file is clean; Revert opens that saved file after an undo; a later save appends to it so the saved revision stays in the file (#1310, #1311, #1312). Ctrl-drag (Option on macOS) on a grid thumbnail copies the pages into the drop gap, with a "+" badge and the copies selected (#2997). Missing: page transitions, recovery on the web | +| M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Extract and Split preserve shared Type 3 glyph resource graphs, retaining conservative pruning limits. Undo back to the opened or last saved file is clean; Revert opens that saved file after an undo; a later save appends to it so the saved revision stays in the file (#1310, #1311, #1312). Ctrl-drag (Option on macOS) on a grid thumbnail copies the pages into the drop gap, with a "+" badge and the copies selected (#2997). #3309: page deletion walks named destinations iteratively with a shared work budget; deep/cyclic trees and atomic engine refusal have synthetic coverage. Missing: page transitions, recovery on the web | | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Extract and Split preserve shared Type 3 glyph resource graphs, retaining conservative pruning limits. Undo back to the opened or last saved file is clean; Revert opens that saved file after an undo; a later save appends to it so the saved revision stays in the file (#1310, #1311, #1312). Rotation queries and edits resolve indirect values on pages and ancestors. Refreshed against upstream `99941daa` (2026-10-15). Repeated page indices accumulate rotations from the latest local value (#1919); saved/reopened regressions retain inherited angles and siblings. Missing: page transitions, recovery on the web | | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Extract and Split preserve shared Type 3 glyph resource graphs, retaining conservative pruning limits. Undo back to the opened or last saved file is clean; Revert opens that saved file after an undo; a later save appends to it so the saved revision stays in the file (#1310, #1311, #1312). Rotation queries and edits resolve indirect values on pages and ancestors. Missing: page transitions, recovery on the web | | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Extract and Split preserve shared Type 3 glyph resource graphs, retaining conservative pruning limits. Missing: page transitions, recovery on the web M4: Insert-from-file preserves source bookmarks; integration tests retained with current main. | @@ -194,10 +194,12 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st ## Revision history + **2026-10-15 · minor:** M14/#2383 candidate: GPU device requests clamp unsupported limits while retaining the adapter texture extent. Synthetic V3D/downlevel regressions and Windows GPU-required tests pass; Raspberry Pi hardware remains unverified. Estimates unchanged. | Date | Change | Summary | |---|---|---| +| 2026-10-18 | minor | M4, #3309: named-destination pruning is iterative and bounded; deep-tree saves, cycles and atomic engine refusal are tested. Estimates unchanged. | | 2026-10-15 | minor | M7: Edit text caret height follows its row, with a kittest regression; estimates unchanged | | 2026-10-18 | minor | M4, PR #1919 review: repeated page indices apply each rotation to the latest page-local value, preserving indirect/inherited angles. Synthetic adjacent/interleaved duplicates retain siblings and page contents through incremental and full saves. Estimates unchanged. | | 2026-10-18 | minor | M3, #3149: page replacement cancels render demand collected for the previous geometry; all viewer raster upload paths reject empty or oversized textures before cache eviction. Synthetic regressions reproduce the reported 1880×2350 upload at a 2048 limit, retain cached images and unsaved edits, and resume bounded backdrops and tiles. Estimates unchanged. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 547c5561c7..d6bd8d72de 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2668,8 +2668,8 @@ milestone = "M4" status = "shipped" commands = ["page.delete"] tools = ["page_delete"] -evidence = ["crates/organize/src/tests.rs::delete_keeps_inherited_attributes_on_survivors", "crates/organize/src/tests.rs::cannot_delete_every_page_or_missing_pages", "crates/ui-egui/tests/editing.rs::delete_is_refused_when_it_would_remove_every_page"] -notes = "" +evidence = ["crates/organize/src/tests.rs::delete_keeps_inherited_attributes_on_survivors", "crates/organize/src/tests.rs::cannot_delete_every_page_or_missing_pages", "crates/ui-egui/tests/editing.rs::delete_is_refused_when_it_would_remove_every_page", "crates/automation/tests/automation.rs::page_delete_handles_deep_destination_trees_with_history_and_reopen", "crates/automation/tests/automation.rs::page_delete_destination_budget_preserves_unsaved_work_and_history"] +notes = "Named-destination pruning is iterative; a shared 100,000-entry work budget refuses excessive trees atomically at the engine boundary. No new destination-tree repair is attempted." [[feature]] id = "organize.move-pages" From 2448074defa9a4f3a7f379527737a5128b4318f3 Mon Sep 17 00:00:00 2001 From: PerchAnton <101743951+PerchAnton@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:25:45 +0300 Subject: [PATCH 08/23] M4: assert redo survives destination-budget refusal --- crates/automation/tests/automation.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index f00b963b59..0da59aceab 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -844,11 +844,15 @@ fn page_delete_destination_budget_preserves_unsaved_work_and_history() { let mut a = auto(&dir); let doc = ok(&mut a, "doc_open", json!({"path":"wide.pdf"}))["doc"].as_u64().unwrap(); ok(&mut a, "doc_set_info", json!({"doc":doc,"key":"Title","value":"Unsaved work"})); + ok(&mut a, "doc_set_info", json!({"doc":doc,"key":"Author","value":"Redo probe"})); + ok(&mut a, "edit_undo", json!({"doc":doc})); let info = ok(&mut a, "doc_info", json!({"doc":doc})); let before = a.session().get(pdfcraft_engine::DocId(doc)).unwrap(); let bytes = before.bytes.clone(); let generation = before.edit_generation(); let undo = before.can_undo().map(str::to_owned); + let redo = before.can_redo().map(str::to_owned); + assert!(redo.is_some(), "the refusal must preserve an existing redo step"); assert!( matches!(err(&mut a, "page_delete", json!({"doc":doc,"pages":[1]})), ToolError::Failed(message) if message.contains("named destination tree") && message.contains("100000-entry work limit")) ); @@ -856,8 +860,12 @@ fn page_delete_destination_budget_preserves_unsaved_work_and_history() { assert!(std::sync::Arc::ptr_eq(&after.bytes, &bytes)); assert_eq!(after.edit_generation(), generation); assert_eq!(after.can_undo(), undo.as_deref()); + assert_eq!(after.can_redo(), redo.as_deref()); assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc})), info); assert_eq!(page_text(&mut a, doc), ["Page 1", "Page 2", "Page 3"]); + ok(&mut a, "edit_redo", json!({"doc":doc})); + assert_eq!(ok(&mut a, "doc_info", json!({"doc":doc}))["author"], "Redo probe"); + ok(&mut a, "edit_undo", json!({"doc":doc})); ok(&mut a, "edit_undo", json!({"doc":doc})); assert_eq!(a.session().get(pdfcraft_engine::DocId(doc)).unwrap().can_undo(), None); assert_eq!(std::fs::read(dir.join("wide.pdf")).unwrap(), original); From 060381c745719403a16c85e91107eecf673f4097 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Mon, 19 Oct 2026 06:47:56 +0800 Subject: [PATCH 09/23] fix(a11y): join and splice page content streams with cross-stream rules Join multiple streams in a page's /Contents array with a space (b' ') rather than a newline (b'\n') in autotag::content_bytes, alt::page_content, and content::page_content so a % comment at the end of one stream continues through the start of the next stream per ISO 32000-2 section 7.8.2. Update alt::artifact_marked_content to join page content streams with pdfcraft_content::Pieces::join, parse operators across the joined stream, and rewrite only matching BDC spans via Pieces::splice so operators split across stream boundaries and cross-stream comments are preserved. Fixes #3217 --- crates/a11y/src/alt.rs | 40 ++++--- crates/a11y/src/autotag.rs | 2 +- crates/a11y/src/content.rs | 2 +- crates/a11y/tests/cross_stream.rs | 167 ++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 18 deletions(-) create mode 100644 crates/a11y/tests/cross_stream.rs diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index 92a5ae8c20..486ce9af14 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -165,7 +165,7 @@ pub(crate) fn page_content(doc: &Document, page: &Dict) -> Vec { .flat_map(|x| match &*doc.resolve(x) { Object::Stream(s) => { let mut v = s.decoded().unwrap_or_default(); - v.push(b'\n'); + v.push(b' '); v } _ => Vec::new(), @@ -321,25 +321,33 @@ fn marked(doc: &Document, props: &Dict, op: &pdfcraft_content::Op, ids: &HashSet fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids: &HashSet) -> Result { let Some(entries) = content_entries(doc, &page.dict) else { return Ok(false) }; let props = page_properties(doc, &page.dict); + let mut streams: Vec)>> = Vec::with_capacity(entries.len()); + for entry in &entries { + match &*doc.resolve(entry) { + Object::Stream(s) => streams.push(Some((s.clone(), s.decoded().unwrap_or_default()))), + _ => streams.push(None), + } + } + let decoded: Vec<&[u8]> = streams.iter().flatten().map(|(_, bytes)| bytes.as_slice()).collect(); + let pieces = pdfcraft_content::Pieces::join(&decoded); + let edits: Vec<_> = + pieces.parse().ops.into_iter().filter(|op| marked(doc, &props, op, ids)).map(|op| (op.span, b"/Artifact BMC".to_vec())).collect(); + if edits.is_empty() { + return Ok(false); + } + let mut spliced = pieces.splice(edits).into_iter(); let mut next = Vec::with_capacity(entries.len()); let mut changed = false; - for entry in entries { - let Some(stream) = (match &*doc.resolve(&entry) { - Object::Stream(s) => Some(s.clone()), - _ => None, - }) else { + for (entry, stream) in entries.into_iter().zip(streams) { + let Some((stream, old)) = stream else { next.push(entry); continue; }; - let mut ops = pdfcraft_content::parse(&stream.decoded().unwrap_or_default()).ops; - let mut ours = false; - for op in &mut ops { - if marked(doc, &props, op, ids) { - *op = pdfcraft_content::Op::new("BMC", vec![Object::name("Artifact")]); - ours = true; - } - } - if !ours { + let Some(new) = spliced.next() else { + next.push(entry); + continue; + }; + if new == old { next.push(entry); continue; } @@ -348,7 +356,7 @@ fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids: dict.remove(b"Length"); dict.remove(b"Filter"); dict.remove(b"DecodeParms"); - let stream = Object::Stream(Stream::flate(dict, &pdfcraft_content::serialize_ops(&ops))); + let stream = Object::Stream(Stream::flate(dict, &new)); let stored = match entry { Object::Ref(r) => { doc.set(r, stream); diff --git a/crates/a11y/src/autotag.rs b/crates/a11y/src/autotag.rs index 28a613f366..4462025cf5 100644 --- a/crates/a11y/src/autotag.rs +++ b/crates/a11y/src/autotag.rs @@ -70,7 +70,7 @@ fn content_bytes(doc: &Document, page: &pdfcraft_model::Page, index: usize) -> R .map_err(|e| unsupported(index, format!("content cannot be decoded strictly: {e}")))? }; all.extend_from_slice(&decoded); - all.push(b'\n'); + all.push(b' '); } Ok(all) } diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index e8a7db377f..f72c40416e 100644 --- a/crates/a11y/src/content.rs +++ b/crates/a11y/src/content.rs @@ -47,7 +47,7 @@ fn page_content(doc: &Document, page: &Dict) -> Vec { .flat_map(|x| match &*doc.resolve(x) { Object::Stream(s) => { let mut v = s.decoded().unwrap_or_default(); - v.push(b'\n'); + v.push(b' '); v } _ => Vec::new(), diff --git a/crates/a11y/tests/cross_stream.rs b/crates/a11y/tests/cross_stream.rs new file mode 100644 index 0000000000..3094914366 --- /dev/null +++ b/crates/a11y/tests/cross_stream.rs @@ -0,0 +1,167 @@ +use pdfcraft_a11y::{Options, Rule, Status, autotag_text, check, figures, mark_decorative, mark_tag_artifact, tag_content, tags}; +use pdfcraft_content::{Op, Pieces}; +use pdfcraft_cos::{Document, ObjRef, Object}; + +const FONT: &str = "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>"; +const HIDDEN_AND_AFTER: &str = "BT /F1 12 Tf 72 520 Td (Hidden) Tj ET\nBT /F1 12 Tf 72 500 Td (After) Tj ET"; + +fn stream(data: &str) -> String { + format!("<< /Length {} >>\nstream\n{data}\nendstream", data.len()) +} + +fn build_pdf(objects: &[(u32, String)]) -> Document { + let mut raw = Vec::from(&b"%PDF-1.7\n"[..]); + let max_num = objects.iter().map(|(n, _)| *n).max().unwrap_or(0); + let mut offsets = std::collections::HashMap::new(); + for (num, body) in objects { + offsets.insert(*num, raw.len()); + raw.extend_from_slice(format!("{num} 0 obj\n{body}\nendobj\n").as_bytes()); + } + let xref = raw.len(); + let size = max_num + 1; + raw.extend_from_slice(format!("xref\n0 {size}\n0000000000 65535 f \n").as_bytes()); + for num in 1..size { + if let Some(off) = offsets.get(&num) { + raw.extend_from_slice(format!("{off:010} 00000 n \n").as_bytes()); + } else { + raw.extend_from_slice(b"0000000000 65535 f \n"); + } + } + raw.extend_from_slice(format!("trailer\n<< /Size {size} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes()); + Document::open(std::sync::Arc::new(raw)).expect("valid synthetic PDF") +} + +fn untagged_two_streams(first: &str, second: &str) -> Document { + build_pdf(&[ + (1, "<< /Type /Catalog /Pages 2 0 R >>".into()), + (2, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into()), + (3, "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents [5 0 R 6 0 R] /Resources << /Font << /F1 4 0 R >> >> >>".into()), + (4, FONT.into()), + (5, stream(first)), + (6, stream(second)), + ]) +} + +fn tagged_figure_two_streams(after_figure: &str, second: &str) -> Document { + let figure = "/Figure << /MCID 0 >> BDC q 50 0 0 50 72 600 cm /Im1 Do Q EMC\n"; + let first = format!("{figure}{after_figure}"); + build_pdf(&[ + ( + 1, + "<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 6 0 R /MarkInfo << /Marked true >> >>" + .into(), + ), + (2, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into()), + ( + 3, + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /StructParents 0 /Contents [10 0 R 11 0 R] /Resources << /Font << /F1 4 0 R >> /XObject << /Im1 5 0 R >> >> >>".into(), + ), + (4, FONT.into()), + ( + 5, + "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\x00\x7f\x40\x20\nendstream".into(), + ), + (6, "<< /Type /StructTreeRoot /K 7 0 R /ParentTree 9 0 R >>".into()), + (7, "<< /Type /StructElem /S /Document /P 6 0 R /K [8 0 R] >>".into()), + ( + 8, + "<< /Type /StructElem /S /Figure /P 7 0 R /Pg 3 0 R /K 0 /Alt (A grey square) >>" + .into(), + ), + (9, "<< /Nums [0 [8 0 R]] >>".into()), + (10, stream(&first)), + (11, stream(second)), + ]) +} + +fn joined_ops(doc: &Document) -> (Vec, usize) { + let page = &pdfcraft_model::pages(doc)[0]; + let contents = page.dict.get(b"Contents").expect("page contents"); + let pieces: Vec> = match &*doc.resolve(contents) { + Object::Stream(s) => vec![s.decoded().expect("decoded stream")], + Object::Array(items) => items + .iter() + .filter_map(|item| match &*doc.resolve(item) { + Object::Stream(s) => Some(s.decoded().expect("decoded stream")), + _ => None, + }) + .collect(), + _ => Vec::new(), + }; + let joined = Pieces::join(&pieces); + let parsed = joined.parse(); + (parsed.ops, parsed.skipped) +} + +fn shown_strings(ops: &[Op]) -> Vec { + ops.iter().filter(|op| op.is("Tj")).filter_map(|op| op.operands.first()?.as_string().map(|s| s.to_text())).collect() +} + +#[test] +fn autotag_keeps_text_commented_out_across_stream_boundary_hidden() { + let mut doc = untagged_two_streams("BT /F1 12 Tf 72 540 Td (Visible) Tj ET\n% a note at the end of the first stream", HIDDEN_AND_AFTER); + let result = autotag_text(&mut doc).expect("autotag succeeds"); + assert_eq!(result.pages, 1); + assert_eq!(result.paragraphs, 2, "commented-out text at start of second stream must not be tagged"); + + let (ops, skipped) = joined_ops(&doc); + assert_eq!(skipped, 0); + assert_eq!(shown_strings(&ops), ["Visible", "After"]); + + let p_tags: Vec<_> = tags(&doc).into_iter().filter(|t| t.kind == "P").collect(); + assert_eq!(p_tags.len(), 2); + let texts: Vec<_> = p_tags.iter().flat_map(|t| tag_content(&doc, t.object).expect("tag content")).map(|item| item.text).collect(); + assert_eq!(texts, ["Visible", "After"]); + + // Control: when the comment ends with a newline inside the first stream, Hidden is visible. + let mut control = untagged_two_streams("BT /F1 12 Tf 72 540 Td (Visible) Tj ET\n% a note at the end of the first stream\n", HIDDEN_AND_AFTER); + let control_res = autotag_text(&mut control).expect("control autotag succeeds"); + assert_eq!(control_res.paragraphs, 3); + let (control_ops, _) = joined_ops(&control); + assert_eq!(shown_strings(&control_ops), ["Visible", "Hidden", "After"]); +} + +#[test] +fn mark_decorative_preserves_shape_split_across_content_streams() { + let mut doc = tagged_figure_two_streams("0 0 1 rg 300 600 100 50", "re f\nBT /F1 12 Tf 72 500 Td (After) Tj ET"); + mark_decorative(&mut doc, ObjRef::new(8, 0)).expect("mark decorative succeeds"); + assert!(figures(&doc).is_empty()); + + let (ops, skipped) = joined_ops(&doc); + assert_eq!(skipped, 0, "split rectangle operands must not be dropped as skipped tokens"); + let re_op = ops.iter().find(|op| op.is("re")).expect("rectangle operator must survive"); + assert_eq!(re_op.nums::<4>(), Some([300.0, 600.0, 100.0, 50.0])); + assert_eq!(shown_strings(&ops), ["After"]); + + // Control: rectangle whole in the first stream. + let mut control = tagged_figure_two_streams("0 0 1 rg 300 600 100 50 re f", "BT /F1 12 Tf 72 500 Td (After) Tj ET"); + mark_decorative(&mut control, ObjRef::new(8, 0)).expect("control mark decorative succeeds"); + let (control_ops, control_skipped) = joined_ops(&control); + assert_eq!(control_skipped, 0); + let control_re = control_ops.iter().find(|op| op.is("re")).expect("control rectangle"); + assert_eq!(control_re.nums::<4>(), Some([300.0, 600.0, 100.0, 50.0])); + assert_eq!(shown_strings(&control_ops), ["After"]); +} + +#[test] +fn mark_decorative_and_mark_artifact_keep_cross_stream_comment_hidden() { + let mut doc = tagged_figure_two_streams("% a note at the end of the first stream", HIDDEN_AND_AFTER); + mark_decorative(&mut doc, ObjRef::new(8, 0)).expect("mark decorative succeeds"); + let (ops, skipped) = joined_ops(&doc); + assert_eq!(skipped, 0); + assert_eq!(shown_strings(&ops), ["After"], "commented-out Hidden text must not become visible after mark_decorative"); + + let mut doc_artifact = tagged_figure_two_streams("% a note at the end of the first stream", HIDDEN_AND_AFTER); + mark_tag_artifact(&mut doc_artifact, ObjRef::new(8, 0)).expect("mark_tag_artifact succeeds"); + let (artifact_ops, artifact_skipped) = joined_ops(&doc_artifact); + assert_eq!(artifact_skipped, 0); + assert_eq!(shown_strings(&artifact_ops), ["After"], "commented-out Hidden text must not become visible after mark_tag_artifact"); + + // Checker also honours the cross-stream comment when scanning for untagged content. + let mut comment_only = tagged_figure_two_streams("% a note at the end of the first stream", "BT /F1 12 Tf 72 520 Td (Hidden) Tj ET\n"); + let report = check(&comment_only, &Options { rules: [Rule::TaggedContent].into(), pages: None }); + assert_eq!(report.result(Rule::TaggedContent).expect("rule result").status, Status::Passed); + mark_decorative(&mut comment_only, ObjRef::new(8, 0)).expect("mark decorative"); + let report_after = check(&comment_only, &Options { rules: [Rule::TaggedContent].into(), pages: None }); + assert_eq!(report_after.result(Rule::TaggedContent).expect("rule result").status, Status::Passed); +} From e3d095f632a0aff64030b8eb410231f54b5566e9 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Mon, 19 Oct 2026 11:45:29 +0800 Subject: [PATCH 10/23] M12: isolate decorative edits of shared content streams --- ROADMAP.md | 2 + crates/a11y/src/alt.rs | 11 ++-- crates/a11y/tests/cross_stream.rs | 89 ++++++++++++++++++++++++++++++- docs/roadmap.md | 2 +- 4 files changed, 94 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index bad75fc3be..a72dc5f827 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -171,6 +171,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-19 (M12, #3217):** Decorative marking replaces changed content-stream occurrences with fresh objects, preserving other uses of shared streams and leaving untouched entries byte-identical. Regressions cover repeated references, another page sharing a stream, unique references, and full/incremental save-and-reopen. Estimates unchanged. + - **2026-10-18 (M6, #2892):** Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. Estimates unchanged. - **2026-10-18 (M7, #3212):** Headers, Bates numbers, watermarks and added text stay visible after unfinished marked content, with original content preserved. diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index 486ce9af14..e4755ab385 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -357,14 +357,9 @@ fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids: dict.remove(b"Filter"); dict.remove(b"DecodeParms"); let stream = Object::Stream(Stream::flate(dict, &new)); - let stored = match entry { - Object::Ref(r) => { - doc.set(r, stream); - Object::Ref(r) - } - _ => Object::Ref(doc.add(stream)), - }; - next.push(stored); + // Splicing depends on this occurrence's neighbours. The original stream may also + // appear elsewhere in this array or on another page, where it must stay unchanged. + next.push(Object::Ref(doc.add(stream))); } if changed { let contents = match next.as_slice() { diff --git a/crates/a11y/tests/cross_stream.rs b/crates/a11y/tests/cross_stream.rs index 3094914366..ad613a2939 100644 --- a/crates/a11y/tests/cross_stream.rs +++ b/crates/a11y/tests/cross_stream.rs @@ -1,6 +1,6 @@ use pdfcraft_a11y::{Options, Rule, Status, autotag_text, check, figures, mark_decorative, mark_tag_artifact, tag_content, tags}; use pdfcraft_content::{Op, Pieces}; -use pdfcraft_cos::{Document, ObjRef, Object}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, SaveOptions, Stream, write_full, write_incremental}; const FONT: &str = "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>"; const HIDDEN_AND_AFTER: &str = "BT /F1 12 Tf 72 520 Td (Hidden) Tj ET\nBT /F1 12 Tf 72 500 Td (After) Tj ET"; @@ -97,6 +97,93 @@ fn shown_strings(ops: &[Op]) -> Vec { ops.iter().filter(|op| op.is("Tj")).filter_map(|op| op.operands.first()?.as_string().map(|s| s.to_text())).collect() } +#[test] +fn mark_decorative_leaves_the_other_stream_untouched() { + let mut doc = tagged_figure_two_streams("0 0 1 rg 300 600 100 50 re f", "BT /F1 12 Tf 72 500 Td (After) Tj ET"); + let before = doc.get(ObjRef::new(11, 0)).clone(); + mark_decorative(&mut doc, ObjRef::new(8, 0)).expect("mark decorative succeeds"); + assert!(figures(&doc).is_empty()); + assert_eq!(*doc.get(ObjRef::new(11, 0)), *before, "the second stream holds no figure and keeps its object"); + let page = doc.get(ObjRef::new(3, 0)); + let contents = page.as_dict().unwrap().get(b"Contents").unwrap().as_array().unwrap(); + assert_eq!(contents[1].as_ref(), Some(ObjRef::new(11, 0)), "the untouched stream must not be replaced by a re-encoded copy"); +} + +/// A and B supply different operands to R's BDC. Reusing R must not let the first +/// occurrence's splice remove the unselected second occurrence's BDC. +fn split_marked_content(shared: bool) -> Document { + let mut doc = tagged_figure_two_streams("", ""); + doc.set(ObjRef::new(10, 0), Object::Stream(Stream::flate(Dict::new(), b"/Figure << /MCID 0 >>"))); + let body = Object::Stream(Stream::flate(Dict::new(), b"BDC 0 0 10 10 re f EMC")); + doc.set(ObjRef::new(11, 0), body.clone()); + let b = doc.add(Object::Stream(Stream::flate(Dict::new(), b"/Span << /MCID 1 >>"))); + let second = if shared { ObjRef::new(11, 0) } else { doc.add(body) }; + doc.update_dict(ObjRef::new(3, 0), |p| { + p.set( + b"Contents".to_vec(), + Object::Array(vec![Object::Ref(ObjRef::new(10, 0)), Object::Ref(ObjRef::new(11, 0)), Object::Ref(b), Object::Ref(second)]), + ); + }) + .unwrap(); + Document::open(std::sync::Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap() +} + +fn assert_selected_occurrence_only(shared: bool) { + let mut doc = split_marked_content(shared); + let (before, skipped) = joined_ops(&doc); + assert_eq!(skipped, 0); + assert_eq!(before.iter().filter(|op| op.is("BDC")).count(), 2); + let figure = figures(&doc)[0].obj; + mark_decorative(&mut doc, figure).unwrap(); + for bytes in [write_full(&doc, &SaveOptions::default()).unwrap(), write_incremental(&doc, &SaveOptions::default()).unwrap()] { + let reopened = Document::open(std::sync::Arc::new(bytes)).unwrap(); + let (ops, skipped) = joined_ops(&reopened); + assert_eq!(skipped, 0); + assert_eq!(ops.iter().filter(|op| op.is("BMC") && op.name(0) == Some(b"Artifact")).count(), 1); + let unselected: Vec<_> = ops.iter().filter(|op| op.is("BDC")).collect(); + assert_eq!(unselected.len(), 1, "unselected Span must retain its BDC after save/reopen"); + assert_eq!(unselected[0].name(0), Some(b"Span".as_slice())); + assert_eq!(unselected[0].operands[1].as_dict().unwrap().get(b"MCID").and_then(Object::as_int), Some(1)); + assert_eq!(ops.iter().filter(|op| op.is("re")).count(), 2); + assert_eq!(ops.iter().filter(|op| op.is("EMC")).count(), 2); + assert!(figures(&reopened).is_empty()); + } +} + +#[test] +fn mark_decorative_isolates_shared_stream_occurrences_after_save() { + assert_selected_occurrence_only(true); +} + +#[test] +fn mark_decorative_preserves_unique_stream_occurrences_after_save() { + assert_selected_occurrence_only(false); +} + +#[test] +fn mark_decorative_leaves_another_page_using_the_stream_untouched() { + let mut doc = tagged_figure_two_streams("", "BT /F1 12 Tf 72 500 Td (After) Tj ET"); + let page = doc.get(ObjRef::new(3, 0)).as_dict().unwrap().clone(); + let other_page = doc.add(Object::Dict(page)); + doc.update_dict(ObjRef::new(2, 0), |p| { + p.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(3, 0)), Object::Ref(other_page)])); + p.set(b"Count".to_vec(), Object::Int(2)); + }) + .unwrap(); + let before = doc.get(ObjRef::new(10, 0)).clone(); + mark_decorative(&mut doc, ObjRef::new(8, 0)).unwrap(); + for bytes in [write_full(&doc, &SaveOptions::default()).unwrap(), write_incremental(&doc, &SaveOptions::default()).unwrap()] { + let reopened = Document::open(std::sync::Arc::new(bytes)).unwrap(); + let pages = pdfcraft_model::pages(&reopened); + assert_eq!(pages.len(), 2); + let contents = pages[1].dict.get(b"Contents").unwrap().as_array().unwrap(); + assert_eq!(*reopened.resolve(&contents[0]), *before, "the other page's shared figure stream must stay intact"); + let (ops, skipped) = joined_ops(&reopened); + assert_eq!(skipped, 0); + assert!(ops.iter().any(|op| op.is("BMC") && op.name(0) == Some(b"Artifact"))); + } +} + #[test] fn autotag_keeps_text_commented_out_across_stream_boundary_hidden() { let mut doc = untagged_two_streams("BT /F1 12 Tf 72 540 Td (Visible) Tj ET\n% a note at the end of the first stream", HIDDEN_AND_AFTER); diff --git a/docs/roadmap.md b/docs/roadmap.md index e1fdce6505..8ff987fca0 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -146,7 +146,7 @@ total matches the parity estimate. | 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. Missing: OCR languages beyond Latin, editable-text OCR output, full Office layout fidelity/import | | 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). 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.
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). Missing: writing proposed tags and parent tree, field autotag, visual Tags/Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA | +| 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.
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 changed stream occurrences and preserves other uses of shared streams through full/incremental saves (#1324, #3217). An XFA radio item whose value contains a space is written back unchanged (#1319). Missing: writing proposed tags and parent tree, field autotag, visual Tags/Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA | | 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. | | 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.
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; text-only Apply writes marked content, paragraph tags and a parent tree, then opens the Tags panel for review. Missing: mixed-content and field autotag, heading/list/table/figure classification, Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA
Done since: CLI info, text and render use dynamic XFA layout pages, with synthetic count/text/pixel and copy-permission regressions (#1645). | | 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. | From d16c94a7033925231516b97b70298b44f4d5cb49 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Mon, 19 Oct 2026 12:12:12 +0800 Subject: [PATCH 11/23] M12: follow rewritten figure stream in engine regression --- crates/engine/src/tests.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f125c41dac..dfacdc7fd9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5828,11 +5828,14 @@ fn marking_a_figure_decorative_keeps_other_page_streams() { let page = &pdfcraft_model::pages(&doc)[0]; let contents = page.dict.get(b"Contents").and_then(|c| c.as_array()).unwrap(); assert_eq!(contents.len(), 3, "the other streams stay in the page's content list"); + let figure_ref = contents[0].as_ref().unwrap(); + assert_eq!(contents[1].as_ref(), Some(pdfcraft_cos::ObjRef::new(5, 0))); + assert_eq!(contents[2].as_ref(), Some(pdfcraft_cos::ObjRef::new(6, 0))); let stream = |doc: &pdfcraft_cos::Document, n: u32| match &*doc.get(pdfcraft_cos::ObjRef::new(n, 0)) { pdfcraft_cos::Object::Stream(s) => s.clone(), other => panic!("object {n} is {other:?}"), }; - let figure = stream(&doc, 4); + let figure = stream(&doc, figure_ref.num); assert_eq!(figure.dict.get(b"VendorKey").and_then(pdfcraft_cos::Object::as_int), Some(7)); let drawn = String::from_utf8(figure.decoded().unwrap()).unwrap(); assert!(drawn.contains("/Artifact BMC") && !drawn.contains("/MCID 0"), "{drawn}"); @@ -5855,7 +5858,7 @@ fn marking_a_figure_decorative_keeps_other_page_streams() { .collect(); assert!(!text.contains("HeaderSample")); assert_eq!(pdfcraft_edit::scan_markers(&doc).added[0].content, before.added[0].content); - assert_eq!(stream(&doc, 4).dict.get(b"VendorKey").and_then(pdfcraft_cos::Object::as_int), Some(7)); + assert_eq!(stream(&doc, figure_ref.num).dict.get(b"VendorKey").and_then(pdfcraft_cos::Object::as_int), Some(7)); } /// Print as image (Acrobat's Advanced Print Setup): the print-ready PDF carries each page as a From 33268824639728b363782ae7d255cb2e27807db7 Mon Sep 17 00:00:00 2001 From: PerchAnton <101743951+PerchAnton@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:54:13 +0300 Subject: [PATCH 12/23] M4: use fixed-size slices in named destination regressions --- crates/automation/tests/automation.rs | 2 +- crates/organize/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index 118aebf448..0e75451a88 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -824,7 +824,7 @@ fn assert_destination_state(a: &Automation, doc: u64, depth: usize, count: usize let pairs = node.as_dict().unwrap().get(b"Names").and_then(Object::as_array).unwrap(); assert_eq!(pairs.len(), if deleted { count / 2 * 2 } else { count * 2 }); let expected = (0..count).filter(|i| !deleted || i % 2 == 1); - for (pair, i) in pairs.chunks_exact(2).zip(expected) { + for (pair, i) in pairs.as_chunks::<2>().0.iter().zip(expected) { assert_eq!(pair[0].as_string().unwrap().to_text(), format!("dest-{i:05}")); let target = if i % 2 == 0 { pages[0].obj } else { pages.last().unwrap().obj }; assert_eq!(pair[1].as_array().unwrap()[0].as_ref(), Some(target), "destination {i}"); diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index c40347d173..d2720b4f25 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -308,7 +308,7 @@ fn deleting_a_page_keeps_sixty_thousand_named_destinations_usable() { let pairs = leaf.get(b"Names").unwrap().as_array().unwrap(); assert_eq!(pairs.len(), 60_000, "30,000 surviving name/value pairs"); let page = pages(&reopened).unwrap()[1].obj; - for (pair, i) in pairs.chunks_exact(2).zip((1..60_000).step_by(2)) { + for (pair, i) in pairs.as_chunks::<2>().0.iter().zip((1..60_000).step_by(2)) { assert_eq!(pair[0].as_string().unwrap().to_text(), format!("dest-{i:05}")); assert_eq!(pair[1].as_array().unwrap()[0].as_ref(), Some(page)); } From 0ba01a0557121a5dd3031fbd4e5c57f61b933444 Mon Sep 17 00:00:00 2001 From: djmushin <31074036+djmushin@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:36:38 -0400 Subject: [PATCH 13/23] M6: optimize AcroForm JavaScript snapshots and diffs --- crates/engine/src/js.rs | 96 ++++++++++-- crates/engine/src/tests.rs | 182 +++++++++++++++++++++++ crates/js/src/lib.rs | 17 ++- crates/js/src/tests.rs | 34 +++++ docs/form-performance.md | 278 +++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 2 +- 6 files changed, 594 insertions(+), 15 deletions(-) create mode 100644 docs/form-performance.md diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index b797bc8d83..6d344573a3 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -70,11 +70,15 @@ fn display(doc: &pdfcraft_cos::Document, f: &Field) -> i32 { /// A form field as scripts see it. pub fn field_state(doc: &pdfcraft_cos::Document, f: &Field) -> pdfcraft_js::FieldState { + field_state_with_display(f, display(doc, f)) +} + +fn field_state_with_display(f: &Field, display: i32) -> pdfcraft_js::FieldState { let mut s = pdfcraft_js::FieldState::new(f.name.clone(), kind(f.kind), f.value.clone()); s.default = f.default.clone(); s.readonly = f.read_only(); s.required = f.has(pdfcraft_forms::flags::REQUIRED); - s.display = display(doc, f); + s.display = display; s.char_limit = f.max_len; s.options = match f.kind { FieldKind::CheckBox | FieldKind::Radio => f.widgets.iter().filter_map(|w| w.on_state.clone()).map(|o| (o.clone(), o)).collect(), @@ -85,10 +89,19 @@ pub fn field_state(doc: &pdfcraft_cos::Document, f: &Field) -> pdfcraft_js::Fiel /// The changes a script made, against the fields as they were. fn changes(before: &[pdfcraft_js::FieldState], after: &[pdfcraft_js::FieldState]) -> Vec { + if after.is_empty() { + return Vec::new(); + } + // Outcomes contain only fields touched by the script. Index the original snapshot once, + // keeping the first match as the old linear lookup did for malformed duplicate names. + let mut by_name = std::collections::HashMap::with_capacity(before.len()); + for field in before { + by_name.entry(field.name.as_str()).or_insert(field); + } after .iter() .filter_map(|a| { - let b = before.iter().find(|b| b.name == a.name)?; + let b = by_name.get(a.name.as_str())?; let c = FieldChange { name: a.name.clone(), value: (a.value != b.value).then(|| a.value.clone()), @@ -106,14 +119,25 @@ pub struct JsRunner { pub doc: pdfcraft_js::DocInfo, pub doc_scripts: Vec, pub output: JsOutput, - /// The fields in the document as the forms code last read them (for `display`). + /// Immutable document snapshot from the start of the edit (for widget `display`). cos: pdfcraft_cos::Document, + // `cos` is the same immutable pre-edit snapshot for the runner's lifetime. Cache only + // its scalar display readings, not COS objects: large forms exceed the object cache and + // otherwise reparse every widget for every calculation. Values/flags still come from + // the live `fields` snapshot on each call. A new edit gets a new runner and cache. + displays: std::collections::HashMap, } impl JsRunner { pub fn new(cos: &pdfcraft_cos::Document, file_name: &str) -> JsRunner { let doc = pdfcraft_js::DocInfo { file_name: file_name.to_string(), num_pages: pdfcraft_model::pages(cos).len(), page: 0, info: info(cos) }; - JsRunner { doc, doc_scripts: pdfcraft_forms::document_scripts(cos), output: JsOutput::default(), cos: cos.clone() } + JsRunner { + doc, + doc_scripts: pdfcraft_forms::document_scripts(cos), + output: JsOutput::default(), + cos: cos.clone(), + displays: std::collections::HashMap::new(), + } } } @@ -130,19 +154,28 @@ fn info(cos: &pdfcraft_cos::Document) -> Vec<(String, String)> { impl Scripts for JsRunner { fn run(&mut self, event: FieldEvent, script: &str, target: &Field, value: &str, fields: &[Field]) -> ScriptResult { - let states: Vec<_> = fields.iter().map(|f| field_state(&self.cos, f)).collect(); + let states: Vec<_> = fields + .iter() + .map(|f| { + let display = match f.widgets.first() { + Some(w) => *self.displays.entry(w.obj).or_insert_with(|| display(&self.cos, f)), + None => pdfcraft_js::DISPLAY_VISIBLE, + }; + field_state_with_display(f, display) + }) + .collect(); + let states = std::sync::Arc::new(states); let ev = pdfcraft_js::Event::field(event.name(), &target.name, value); - let o = pdfcraft_js::run(script, &ev, &self.doc, &states, &self.doc_scripts, Limits::default()); + let o = pdfcraft_js::run_shared(script, &ev, &self.doc, std::sync::Arc::clone(&states), &self.doc_scripts, Limits::default()); self.output.absorb(&o); if o.error.is_some() { // As in Acrobat, a failing script leaves the value alone (the error goes to the console). return ScriptResult { rc: true, value: value.to_string(), changes: Vec::new(), message: None }; } - let after: Vec<_> = states.iter().map(|s| o.changed.iter().find(|c| c.name == s.name).unwrap_or(s).clone()).collect(); ScriptResult { rc: o.rc, value: o.value.clone(), - changes: changes(&states, &after), + changes: changes(&states, &o.changed), message: (!o.rc).then(|| o.alerts.last().cloned()).flatten(), } } @@ -213,8 +246,7 @@ impl Session { None => pdfcraft_js::Event::doc("Console"), }; let mut o = pdfcraft_js::run(script, &event, &runner.doc, &states, &runner.doc_scripts, Limits::default()); - let after: Vec<_> = states.iter().map(|s| o.changed.iter().find(|c| c.name == s.name).unwrap_or(s).clone()).collect(); - let changes = changes(&states, &after); + let changes = changes(&states, &o.changed); let resets: Vec> = o .requests .iter() @@ -368,3 +400,47 @@ impl Session { Ok(names) } } + +#[cfg(test)] +mod review_tests { + use super::*; + + #[test] + fn duplicate_field_names_diff_against_first_original() { + let first = pdfcraft_js::FieldState::new("duplicate", pdfcraft_js::FieldType::Text, vec!["first".into()]); + let mut second = first.clone(); + second.value = vec!["second".into()]; + second.display = pdfcraft_js::DISPLAY_HIDDEN; + let before = vec![first.clone(), second.clone()]; + assert!(changes(&before, std::slice::from_ref(&first)).is_empty()); + let actual = changes(&before, std::slice::from_ref(&second)); + assert_eq!( + actual, + vec![FieldChange { + name: "duplicate".into(), + value: Some(second.value), + display: Some(pdfcraft_js::DISPLAY_HIDDEN), + ..Default::default() + }] + ); + let outcome = pdfcraft_js::run_shared( + "event.value = getField('duplicate').value; getField('duplicate').value = 'updated';", + &pdfcraft_js::Event::field("Calculate", "duplicate", ""), + &pdfcraft_js::DocInfo::default(), + std::sync::Arc::new(before.clone()), + &[], + Limits::default(), + ); + assert_eq!(outcome.error, None); + assert_eq!(outcome.value, "first"); + assert_eq!(outcome.changed.len(), 1); + assert_eq!( + changes(&before, &outcome.changed), + vec![FieldChange { name: "duplicate".into(), value: Some(vec!["updated".into()]), ..Default::default() }] + ); + assert!(changes(&before, &[]).is_empty()); + let mut missing = first; + missing.name = "missing".into(); + assert!(changes(&before, &[missing]).is_empty()); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f125c41dac..67af975966 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6106,3 +6106,185 @@ fn undo_never_makes_an_unsaved_document_clean() { s.undo(new).unwrap(); assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); } +/// Reproducible workload with 3,129 fields and 190 ordered custom calculations. +/// No external PDF assets are needed; only in-memory contributor-original objects. +fn synthetic_acroform(field_count: usize, calculation_count: usize) -> Vec { + use pdfcraft_cos::{Dict, Object, PdfString}; + let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(6))).unwrap(); + let mut refs = Vec::new(); + let mut order = Vec::new(); + for i in 0..field_count { + let name = match i { + 0 => "ranks".to_string(), + 1 => "trained".to_string(), + 2 => "bonus".to_string(), + _ => format!("field{i}"), + }; + let mut d = Dict::new(); + d.set(b"Type".to_vec(), Object::name("Annot")); + d.set(b"Subtype".to_vec(), Object::name("Widget")); + d.set(b"FT".to_vec(), Object::name(if i == 1 { "Btn" } else { "Tx" })); + d.set(b"T".to_vec(), PdfString::text(&name)); + d.set(b"V".to_vec(), if i == 1 { Object::name("Off") } else { Object::String(PdfString::text("0")) }); + d.set(b"Rect".to_vec(), Object::Array([20, 20, 100, 40].map(Object::Int).to_vec())); + d.set(b"P".to_vec(), Object::Ref(pdfcraft_cos::ObjRef::new(4, 0))); + if (2..calculation_count + 2).contains(&i) { + let js = if i == 2 { + "var r = Number(getField('ranks').value); event.value = r + (r > 0 && getField('trained').isBoxChecked(0) ? 3 : 0);".to_string() + } else { + format!("event.value = Number(getField('{}').value) + 1;", if i == 3 { "bonus".to_string() } else { format!("field{}", i - 1) }) + }; + let mut a = Dict::new(); + a.set(b"S".to_vec(), Object::name("JavaScript")); + a.set(b"JS".to_vec(), PdfString::text(&js)); + let mut aa = Dict::new(); + aa.set(b"C".to_vec(), Object::Dict(a)); + d.set(b"AA".to_vec(), Object::Dict(aa)); + } + let r = doc.add(Object::Dict(d)); + refs.push(Object::Ref(r)); + if (2..calculation_count + 2).contains(&i) { + order.push(Object::Ref(r)); + } + } + doc.update_dict(pdfcraft_cos::ObjRef::new(4, 0), |d| d.set(b"Annots".to_vec(), Object::Array(refs.clone()))).unwrap(); + let mut af = Dict::new(); + af.set(b"Fields".to_vec(), Object::Array(refs)); + af.set(b"CO".to_vec(), Object::Array(order)); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), Object::Dict(af))).unwrap(); + pdfcraft_cos::write_incremental(&doc, &Default::default()).unwrap() +} + +#[test] +#[ignore = "manual performance measurement: 3,129 fields, 190 scripts"] +fn acroform_interaction_benchmark() { + let bytes = Arc::new(synthetic_acroform(3129, 190)); + // Optional local export for comparing CLI build profiles. Never overwrite an input. + if let Some(path) = std::env::var_os("PDFCRAFT_BENCH_EXPORT") { + use std::io::Write; + let mut file = std::fs::OpenOptions::new().write(true).create_new(true).open(path).unwrap(); + file.write_all(&bytes).unwrap(); + } + let mut s = Session::new(); + let id = s.open("synthetic-large-form.pdf", None, bytes, None).unwrap(); + let value = |s: &Session, name: &str| s.get(id).unwrap().form.iter().find(|f| f.name == name).unwrap().value.clone(); + for (name, v) in [ + ("ranks", FieldValue::Text("1".into())), + ("trained", FieldValue::Check(true)), + ("ranks", FieldValue::Text("2".into())), + ("trained", FieldValue::Check(false)), + ("ranks", FieldValue::Text("0".into())), + ] { + let started = std::time::Instant::now(); + s.apply(id, Edit::SetFieldValue { name: name.into(), value: v }).unwrap(); + eprintln!("BENCH {name}: {:.3} ms", started.elapsed().as_secs_f64() * 1000.0); + assert!(s.take_js_output(id).errors.is_empty()); + } + assert_eq!(value(&s, "bonus"), ["0"]); + assert_eq!(value(&s, "field191"), ["189"]); + assert_eq!(value(&s, "field3128"), ["0"]); +} + +#[test] +fn checkbox_and_rank_changes_recalculate_ordered_dependents() { + let mut s = Session::new(); + let id = s.open("skills.pdf", None, Arc::new(synthetic_acroform(8, 4)), None).unwrap(); + for (name, input, bonus) in [ + ("trained", FieldValue::Check(true), "0"), + ("ranks", FieldValue::Text("1".into()), "4"), + ("trained", FieldValue::Check(false), "1"), + ("ranks", FieldValue::Text("2".into()), "2"), + ("trained", FieldValue::Check(true), "5"), + ("ranks", FieldValue::Text("0".into()), "0"), + ] { + s.apply(id, Edit::SetFieldValue { name: name.into(), value: input }).unwrap(); + let fields = &s.get(id).unwrap().form; + let value = |name: &str| fields.iter().find(|f| f.name == name).unwrap().value.clone(); + assert_eq!(value("bonus"), [bonus]); + assert_eq!(value("field5"), [(bonus.parse::().unwrap() + 3).to_string()]); + assert_eq!(value("field7"), ["0"]); + assert!(s.take_js_output(id).errors.is_empty()); + } + s.undo(id).unwrap(); + assert_eq!(s.get(id).unwrap().form.iter().find(|f| f.name == "bonus").unwrap().value, ["5"]); + let saved = s.save_bytes(id).unwrap(); + let reopened = s.open("saved.pdf", None, saved, None).unwrap(); + assert_eq!(s.get(reopened).unwrap().form.iter().find(|f| f.name == "bonus").unwrap().value, ["5"]); + let appearance = |s: &Session| { + let doc = s.get(id).unwrap(); + let widget = doc.form.iter().find(|f| f.name == "bonus").unwrap().widgets.first().unwrap().obj; + doc.editor.as_ref().unwrap().cos.get(widget).as_dict().unwrap().get(b"AP").cloned() + }; + let before = appearance(&s); + assert!(before.is_some()); + s.apply(id, Edit::SetFieldValue { name: "field7".into(), value: FieldValue::Text("9".into()) }).unwrap(); + assert_eq!(appearance(&s), before, "unchanged calculated values keep their appearance streams"); +} + +#[test] +fn dynamic_calculation_reads_and_side_effects_reach_later_fields() { + use pdfcraft_cos::{Dict, Object, PdfString}; + let mut cos = pdfcraft_cos::Document::open(Arc::new(synthetic_acroform(8, 4))).unwrap(); + let fields = pdfcraft_forms::fields(&cos); + let bonus = fields.iter().find(|f| f.name == "bonus").unwrap(); + let selector = fields.iter().find(|f| f.name == "field7").unwrap(); + cos.update_dict(selector.obj, |d| d.set(b"V".to_vec(), PdfString::text("ranks"))).unwrap(); + let mut a = Dict::new(); + a.set(b"S".to_vec(), Object::name("JavaScript")); + a.set(b"JS".to_vec(), PdfString::text( + "var key = getField('field7').value; var v = Number(getField(key).value); getField('field6').value = v * 2; getField('field6').readonly = true; event.value = v + 1;" + )); + let mut aa = Dict::new(); + aa.set(b"C".to_vec(), Object::Dict(a)); + cos.update_dict(bonus.obj, |d| d.set(b"AA".to_vec(), Object::Dict(aa))).unwrap(); + let bytes = pdfcraft_cos::write_incremental(&cos, &Default::default()).unwrap(); + let mut s = Session::new(); + let id = s.open("dynamic.pdf", None, Arc::new(bytes), None).unwrap(); + s.apply(id, Edit::SetFieldValue { name: "ranks".into(), value: FieldValue::Text("4".into()) }).unwrap(); + let fields = &s.get(id).unwrap().form; + let field = |name: &str| fields.iter().find(|f| f.name == name).unwrap(); + assert_eq!(field("bonus").value, ["5"]); + assert_eq!(field("field5").value, ["8"], "later calculations see the ordered result"); + assert_eq!(field("field6").value, ["8"], "side-effect value applied"); + assert!(field("field6").read_only(), "side-effect property applied"); + assert_eq!(field("field7").value, ["ranks"]); + assert!(s.take_js_output(id).errors.is_empty()); +} + +#[test] +fn scripted_display_changes_survive_warm_cache_and_new_edit() { + use pdfcraft_forms::{FieldEvent, Scripts}; + let mut cos = pdfcraft_cos::Document::open(Arc::new(synthetic_acroform(2, 0))).unwrap(); + for (display, flags) in [(1, 6), (3, 36), (2, 0), (0, 4)] { + let fields = pdfcraft_forms::fields(&cos); + let target = fields.iter().find(|f| f.name == "ranks").unwrap(); + let other = fields.iter().find(|f| f.name == "trained").unwrap(); + let old_display = crate::js::field_state(&cos, other).display; + let mut runner = crate::js::JsRunner::new(&cos, "synthetic.pdf"); + // Populate the display cache before a script writes the same field. + let warm = runner.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &fields); + assert_eq!(warm.value, old_display.to_string()); + let result = runner.run( + FieldEvent::Calculate, + &format!("getField('trained').display = {display}; event.value = getField('trained').display;"), + target, + "", + &fields, + ); + assert_eq!(result.value, display.to_string()); + assert_eq!(result.changes.len(), 1); + assert_eq!(result.changes.first().unwrap().display, Some(display)); + pdfcraft_forms::apply_script_changes(&mut cos, &result.changes, &mut pdfcraft_forms::NoScripts).unwrap(); + assert_eq!(cos.get(other.widgets.first().unwrap().obj).as_dict().unwrap().int(b"F").unwrap() & 38, flags); + // Within an edit the original implementation reads an immutable pre-edit COS snapshot. + let same_edit = runner.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &fields); + assert_eq!(same_edit.value, old_display.to_string()); + // The next edit must rebuild its cache from the newly written annotation flags. + let mut next = crate::js::JsRunner::new(&cos, "synthetic.pdf"); + let read = next.run(FieldEvent::Calculate, "event.value = getField('trained').display;", target, "", &pdfcraft_forms::fields(&cos)); + assert_eq!(read.value, display.to_string()); + assert!(read.changes.is_empty()); + assert!(runner.output.errors.is_empty()); + assert!(next.output.errors.is_empty()); + } +} diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index d8d9fa8bac..785345c672 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -25,6 +25,7 @@ use std::cell::RefCell; use std::collections::BTreeSet; +use std::sync::Arc; use boa_engine::object::builtins::{JsArray, JsFunction}; use boa_engine::object::{FunctionObjectBuilder, ObjectInitializer}; @@ -194,7 +195,7 @@ impl Default for Limits { } struct Host { - fields: Vec, + fields: Arc>, changed: BTreeSet, alerts: Vec, console: Vec, @@ -286,7 +287,8 @@ fn read(ctx: &Context, name: &JsString) -> JsResult { fn write(ctx: &Context, name: &JsString, change: impl FnOnce(&mut FieldState)) -> JsResult<()> { let i = field_index(ctx, name)?; let mut h = host(ctx)?.borrow_mut(); - change(&mut h.fields[i]); + let field = Arc::make_mut(&mut h.fields).get_mut(i).ok_or_else(|| error("the field is no longer available"))?; + change(field); h.changed.insert(i); Ok(()) } @@ -1098,6 +1100,13 @@ fn refuse(source: &str) -> Option { /// Run `script` for `event`, after the document-level scripts (`doc_scripts`, which define the /// functions field scripts call), with the form's `fields`. pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], doc_scripts: &[String], limits: Limits) -> Outcome { + run_shared(script, event, doc, Arc::new(fields.to_vec()), doc_scripts, limits) +} + +/// Like [`run`], sharing an immutable initial field snapshot. Scripts still get a fresh +/// context; the first field write copies the snapshot, so neither the caller nor another +/// script can observe uncommitted writes. Read-only calculations avoid copying the whole form. +pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { let failed = |why: String| Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), error: Some(why), ..Default::default() }; if let Some(why) = std::iter::once(script).chain(doc_scripts.iter().map(String::as_str)).find_map(refuse) { return failed(why); @@ -1119,12 +1128,12 @@ pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], do run_here(script, event, doc, fields, doc_scripts, limits) } -fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], doc_scripts: &[String], limits: Limits) -> Outcome { +fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { let mut ctx = Context::default(); ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); ctx.insert_data(RefCell::new(Host { - fields: fields.to_vec(), + fields, changed: BTreeSet::new(), alerts: Vec::new(), console: Vec::new(), diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 61d3a71e51..1d2856b8bd 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -1132,3 +1132,37 @@ mod session { assert!(o.abandoned || o.error.as_deref().is_some_and(|e| e.contains("too long")), "{o:?}"); } } + +#[test] +fn shared_field_snapshots_isolate_writes_errors_and_script_globals() { + let initial = Arc::new(fields()); + let before = initial.as_ref().clone(); + let event = Event::field("Calculate", "total", ""); + let write = run_shared( + "var scriptLocal = 1; getField('qty').value = 9; getField('agree').checkThisBox(0, true); event.value = getField('qty').value;", + &event, + &doc(), + Arc::clone(&initial), + &[], + Limits::default(), + ); + assert_eq!(write.error, None); + assert_eq!(write.value, "9"); + assert_eq!(write.changed.iter().find(|f| f.name == "qty").unwrap().value, ["9"]); + assert_eq!(write.changed.iter().find(|f| f.name == "agree").unwrap().value, ["Yes"]); + assert_eq!(*initial, before, "script writes cannot mutate the caller's snapshot"); + let failed = run_shared("getField('qty').value = 99; throw new Error('rejected');", &event, &doc(), Arc::clone(&initial), &[], Limits::default()); + assert!(failed.error.is_some()); + assert_eq!(*initial, before, "an exception cannot leak partial writes into the initial snapshot"); + let read = run_shared( + "event.value = typeof scriptLocal + ':' + getField('qty').value + ':' + getField('agree').isBoxChecked(0);", + &event, + &doc(), + initial, + &[], + Limits::default(), + ); + assert_eq!(read.error, None); + assert_eq!(read.value, "undefined:4:false"); + assert!(read.changed.is_empty()); +} diff --git a/docs/form-performance.md b/docs/form-performance.md new file mode 100644 index 0000000000..c00adf0913 --- /dev/null +++ b/docs/form-performance.md @@ -0,0 +1,278 @@ +# AcroForm JavaScript performance + +Status: implemented; proposed as an independent performance PR. Last reviewed and updated: +2026-10-19. Measurements below are historical synthetic evidence collected before this split, +not fresh measurements on upstream main. No private document is needed for the benchmark. + +## Architecture + +- `crates/ui-egui/src/forms_ui.rs` queues `Edit::SetFieldValue` for a text commit or checkbox + toggle. `crates/automation/src/forms.rs::form_fill` uses the same edit API. +- `crates/engine/src/lib.rs::Session::apply` runs edits on a copy, preserving rollback and undo. + `run_edit` calls `pdfcraft_forms::set_value_with` with an engine `JsRunner`. +- `crates/forms/src/lib.rs` validates and writes the edited value, then runs one calculation + pass. `/CO` precedes calculated fields omitted from `/CO`. Each script sees prior writes. + There is no dependency graph or dependency-based filtering: dynamic references and script + side effects are evaluated on every pass. +- `crates/engine/src/js.rs::JsRunner::run` snapshots the fields, invokes `pdfcraft_js::run_shared`, + then converts changed JavaScript field states into form updates. +- `crates/js/src/lib.rs::run_shared` starts a scoped worker per script. `run_here` builds a fresh Boa + context, installs the API, evaluates document scripts and evaluates the action. No script or + bytecode cache is present in this AcroForm path. XFA uses a separate session implementation. +- `crates/forms/src/scripting.rs::apply_changes` applies script side effects before the next + calculation. It rereads the field snapshot when side effects occur. Calculation results + equal to the existing value already skip value writes and appearance generation. +- Checkboxes write the field `/V` and widget `/AS`; they enter the same calculation pass as + text edits. JavaScript exposes the export value or `Off`, and `isBoxChecked`. +- `Session::refresh_scoped` writes an incremental working revision, rereads fields and rebuilds + the render pool. `crates/ui-egui/src/editing.rs::apply_edit` compares old/new fields and + invalidates affected pages (including dependent fields on other pages). It does not blindly + invalidate every page for an ordinary AcroForm fill. Rendering uses page/tile requests; + widget appearance streams are separate from cached page rasters. + +## Findings and changes + +The dominant measured cost was the bridge, not executing calculation logic: + +1. `JsRunner` expanded a sparse `Outcome::changed` into a complete second snapshot. `changes` + then searched the first snapshot linearly for **each** field, even when JavaScript had not + changed any field directly. That repeated quadratic comparison ran after every calculation. + It now compares the sparse outcome only, with an indexed original snapshot when needed. + Empty outcomes return immediately; unchanged writes still produce no form updates. The + console/button path uses the same conversion. +2. Once that was removed, field-state construction dominated. Reading every widget's display + flags on every script traversed the same immutable pre-edit COS snapshot. This workload + exceeds COS's 1,024-object editing-cache cap, repeatedly reparsing widget dictionaries. + The runner now memoizes only those scalar display readings by widget reference. Each edit + creates a new runner; current values, read-only/required flags and options still come from + the current field snapshot. No global cache or object-cache limit changes were needed. + +All calculation actions still execute in the same order. No dependency filtering, script +removal, context reuse, bytecode caching or recursive scheduling changes were introduced. +Equal calculated values already skipped appearance regeneration; this behavior remains. +The Release investigation also found a full field-state copy inside the JavaScript host on +**every** script invocation. `pdfcraft_js::run_shared` now accepts an immutable shared snapshot; +`Arc::make_mut` detaches it on the first field write. The engine retains the initial snapshot +for comparing effects. Read-only calculations no longer copy every field again. Each script +still gets a fresh context and thread; exceptions, globals and uncommitted writes remain +isolated. Scripts that write fields retain the previous copy isolation. The existing borrowed +`run` API remains available. No new dependencies were added. + +## Measurements + +Historically measured under Rust 1.99.1 on Linux x86_64 with the default optimized development +profile, two-job builds. Serial benchmark runs had no concurrent compilation. These are +**synthetic engine measurements, not measurements of the original Pathfinder PDF or GUI input +latency**. Five edits: ranks 1, checkbox on, ranks 2, checkbox off, ranks 0. Every edit runs all +190 custom calculations among 3,129 fields on a six-page document. Widgets are colocated +on the first page; the fixture is an engine calculation stress test, not a layout replica. + +| Synchronous edit | Before (ms) | After (ms) | +|---|---:|---:| +| ranks 1 | 4,425.274 | 424.465 | +| checkbox on | 4,378.642 | 458.716 | +| ranks 2 | 4,291.131 | 521.721 | +| checkbox off | 4,391.942 | 492.851 | +| ranks 0 | 4,322.570 | 453.392 | +| Median | **4,378.642** | **458.716** | + +Median speedup: **9.55×**, approximately **89.5% less synchronous editing time**. The synthetic +scripts are deliberately small; this does not predict the exact result for other documents. +Raw logs and the baseline test executable are retained locally in `target/form-performance/`. + +Historical serial runs collected with optional instrumentation (mean per edit; inclusive +nested spans). That instrumentation is not included in this PR: + +| Phase | Before (ms) | After (ms) | +|---|---:|---:| +| JavaScript, including thread/context setup | 225.934 | 201.584 | +| Changed-field comparison | 2,814.805 | 0.024 | +| Entire script dispatch, all 190 scripts | 4,320.643 | 352.733 | +| Calculation pass, including changed appearances | 4,399.084 | 390.958 | +| Direct field write | 0.201 | 0.196 | +| Appearance generation, including Format | 61.225 | 22.981 | +| Form event, including direct write and calculations | 4,411.767 | 402.981 | +| Engine refresh/serialization | 23.499 | 21.826 | +| Whole engine edit | 4,442.144 | 431.539 | + +An intermediate run after sparse comparison alone measured 1,193.419 ms/edit in field-state +construction. After display memoization it measured 101.821 ms/edit. The intermediate run's +whole engine edit averaged 1,592.101 ms. Appearance time also benefits from the changed cache +working set; there was no change to which calculated appearances are generated. + +Rendering and compositor time are not included in these edit numbers. The original sheet and +GUI acceptance run remain required to quantify them. + +## Release comparison and remaining cost + +Historical comparison used the repository's Release profile (thin LTO, one code-generation +unit) and an optional standalone profiler, which is not included in this PR. +A fresh in-memory fixture was exported with create-new semantics to +`target/form-performance/synthetic-large.pdf` (SHA-256 +`300431eeae117458d96e52877e12a9e63ea8dcbb8ad4500da717ba00ebb2f1e1`). It remained unchanged. +The example alternated `ranks` between 1 and 2 for six samples per process, always executing +all 190 scripts. Page rendering was timed separately. + +Initial matched comparison, before the additional shared-snapshot change: + +| Build | Samples | Median engine edit (ms) | +|---|---:|---:| +| Development | 6 | 460.697 | +| Release | 6 | 408.358 | + +Release alone reduced the median by about 11%. A detailed Release run measured 106.947 ms/edit +constructing field states, 174.696 ms in JavaScript setup/execution, and 19.808 ms refreshing +engine view data. Calculation-order setup was only 0.319 ms. That supported investigating the +host's extra copy, rather than changing calculation scheduling or disabling calculations. + +The pre-change Release profiler was retained, then compared with the shared-snapshot Release +profiler in alternating runs (old, new, old, new). No compilation ran during measurement. + +| Release version | Samples | Median edit (ms) | Range (ms) | +|---|---:|---:|---:| +| Before shared snapshots | 12 | 381.380 | 353.468–447.833 | +| With shared snapshots | 12 | 267.063 | 258.545–313.777 | + +The additional optimization reduced median engine time by **30.0%** (1.43× speedup). +This is a separate comparison from the earlier five-edit benchmark; do not combine their +speedup factors. The page-render/setup cost in the initial matched comparison was about +6–9 ms for this deliberately overlapping synthetic layout, not for the original sheet. + +After shared snapshots, a historical Release run with optional instrumentation averaged: + +| Phase | Mean per edit (ms) | +|---|---:| +| Field-state construction | 75.754 | +| JavaScript setup/execution | 83.198 | +| Complete script dispatch | 200.480 | +| Calculation pass | 237.780 | +| Calculated-value writes, including appearances | 25.089 | +| Appearances | 23.712 | +| All complete field enumerations (three per edit) | 27.387 | +| Calculation-order setup | 0.305 | +| Engine refresh | 19.128 | +| Whole engine edit | 272.807 | + +These inclusive timings show that scheduling is now negligible on the fixture. The main +remaining costs are constructing/disposing field snapshots and creating/executing isolated +JavaScript contexts. Further snapshot caching or context reuse would need additional semantic +work and real-document evidence; no dependency-based skipping or global/context reuse was +introduced. The current shared-snapshot change was retained because its paired measurement +showed a substantial improvement and isolation regressions passed. + +**Remaining limit:** these numbers do not quantify real-document or compositor latency. +A representative document and delayed field are still needed for a reproducible engine/render/UI +comparison. Prior manual acceptance on the integrated release reported responsive field editing; +that is not a timing measurement or independent validation of this split. + +## Reproduce + +Use the same compiler, build profile and machine for before/after runs. The default development +profile optimizes workspace crates at level 1 and dependencies at level 2. Build with two jobs +and a separate target directory, then run the benchmark without concurrent compilation: + +```sh +CARGO_TARGET_DIR=target/acroform-benchmark cargo test -p pdfcraft-engine -j 2 --lib acroform_interaction_benchmark --no-run +CARGO_TARGET_DIR=target/acroform-benchmark cargo test -p pdfcraft-engine -j 2 --lib acroform_interaction_benchmark -- --ignored --nocapture +``` + +The benchmark creates a six-page, 3,129-field/190-calculation workload from contributor-original +PDF objects in memory and times five synchronous edits with `Instant`. It also checks the final +calculated chain and an independent field. Widgets overlap on page one; this is an engine stress +test, not a realistic page layout. There is no machine-dependent timing threshold in CI. + +To export a synthetic fixture for a standalone comparison, set `PDFCRAFT_BENCH_EXPORT` to a new +filename when running the ignored benchmark. Its parent must exist; export refuses to overwrite +any existing file. The ordinary benchmark writes no PDF. + +## Validation + +Regression coverage includes shared-snapshot write/error/global isolation, duplicate field names, +scripted display changes with a warm cache and a fresh edit, ordered checkbox/rank calculations, +dynamic field lookup, property/value side effects, undo/save/reopen, and unchanged appearances. +The existing JavaScript and engine suites cover the unchanged public scripting behavior. + +Before the split, the ignored benchmark passed both before and after optimization. Earlier +scoped native/WASM checks, Clippy, layering, asset and parity checks, UI form/JavaScript tests, +and automation `javascript_through_tools` checks also passed on the mixed review history. +Those historical checks do not certify these independent branches or hosted CI. + +## Independent branch validation + +Validated on upstream `a12fe8f4116984ebbc870eb514346ea922233316` using a fresh, isolated +Cargo target directory, offline locked dependencies and two build jobs: + +- All five focused regressions passed separately: duplicate names, warm/new-edit display + caches, ordered checkbox/rank dependents, dynamic lookup/side effects and shared-snapshot + write/error/global isolation. +- `cargo test -p pdfcraft-js -p pdfcraft-engine -p pdfcraft-forms -j 2`: engine 285 unit + and 9 integration tests, JavaScript 47 tests, and forms 80 tests passed. Two unit timing + probes and two private-file acceptance tests were ignored in the regular suite. +- The ignored 3,129-field/190-calculation benchmark was then run explicitly and passed; + its final calculated chain and independent field remained correct. This is a diagnostic + timing run, not a new before/after comparison on this upstream base. +- Scoped `cargo clippy --all-targets -- -D warnings` and WASM checks of those three + crates passed, as did formatting, whitespace, layering, asset and parity checks. +- `javascript_through_tools` passed. A CLI built in the isolated target directory then + reproduced bonuses 1 → 4 → 5 → 2 → 0; save/reopen and `page_render` passed, and the + synthetic page image was inspected. + +These are local checks, not hosted CI. No GUI executable was rebuilt and no private PDF +was used for this independent validation. + +## Rebase validation + +Revalidated on upstream `932bf9c6bfdd63e2a21bb831696a3447b9def9cf` (2026-10-18). +The performance implementation and all five focused regressions are unchanged from the +previously reviewed fix. Upstream's newer tests and CSV documentation are retained. + +- Engine: 286 unit and 9 integration tests passed; JavaScript: 48 passed; forms: 82 passed. + Two unit timing probes and two private-file acceptance tests remain ignored in the + regular suite. +- All five focused regressions and the explicit 3,129-field/190-calculation benchmark passed. +- Scoped all-targets Clippy with `-D warnings`, WASM, formatting, whitespace, layering, + asset and parity checks passed. +- `javascript_through_tools` and a freshly built CLI's fill/save/reopen/render smoke check + passed. The bonus chain remained 1 → 4 → 5 → 2 → 0, and the synthetic render was inspected. + +Validation used a separate Cargo target directory. No GUI build or private PDF was needed; +these are local checks, not hosted CI or a new before/after performance comparison. + +## PR #3318 refresh + +Rebased onto freshly fetched upstream main +`e397efa85451be00f8d56df7649807fe31e019c7` on 2026-10-19. Both roadmap conflicts +retain upstream's menu, button-label and content-editing notes alongside the performance +notes. Every performance source, regression-test and parity hunk is exactly the original +performance patch applied to current upstream; upstream's additional tests are retained. +Main advanced after initial validation on `43550c44`; the non-overlapping redaction update +was incorporated and all checks below were repeated on the final base. + +- All five focused regressions passed. +- Engine: 287 unit and 15 integration tests passed. JavaScript: 49 tests passed. Forms: + 84 tests passed. Doc tests passed. The regular suite still ignored two timing probes + and two private-file tests; the synthetic benchmark was then run explicitly and passed. +- The 3,129-field/190-calculation benchmark preserved the expected calculated values. Its + five diagnostic edits took 317.347–328.488 ms; this is not a new before/after comparison. +- Scoped all-targets Clippy with warnings denied, WASM, formatting, whitespace, layering, + assets and parity passed. +- Automation JavaScript and CLI checks passed: bonuses 1 → 4 → 5 → 2 → 0, save/reopen, + and page rendering. The synthetic render was inspected. + +Builds used an isolated Cargo target outside the worktrees. No GUI executable was built, +no private PDF was used, and the compatibility branch was not changed. These are local +checks, not hosted CI or a new performance speedup claim. + +## Revision history + +- 2026-10-19: refresh PR #3318 onto upstream `e397efa8`, retain upstream roadmap additions + and all performance changes, and repeat focused, scoped, benchmark and headless validation. + +- 2026-10-18: rebase onto upstream `932bf9c6`; retain all performance source edits and regression + tests; preserve upstream CSV documentation and repeat scoped native/WASM and headless checks. +- 2026-10-18: separate performance changes for review on current upstream main; add duplicate-name + and scripted-display coverage; keep historical optional instrumentation results as evidence only. +- 2026-10-18: retain shared snapshots after a paired 30% synthetic Release improvement and + verify snapshot isolation. +- 2026-10-17: remove quadratic outcome conversion and repeated display reads; record the + synthetic before/after benchmark. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..4415d7ac5d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4596,7 +4596,7 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props", "form_fill"] -evidence = ["crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat"] +evidence = ["crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat", "crates/engine/src/tests.rs::checkbox_and_rank_changes_recalculate_ordered_dependents", "crates/engine/src/tests.rs::dynamic_calculation_reads_and_side_effects_reach_later_fields"] notes = "/CO order, then other calculated fields; recalculated after every fill and Clear form" [[feature]] From 1cd36bfcb2b31447ab74749177014861219defa7 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:25:47 +0000 Subject: [PATCH 14/23] Decode JavaScript hex and control escapes in AF script strings Found while testing crates/forms/src/af.rs:166 (Parser::string) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/forms/src/af.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/forms/src/af.rs b/crates/forms/src/af.rs index c5b81719ac..5c1ebeea20 100644 --- a/crates/forms/src/af.rs +++ b/crates/forms/src/af.rs @@ -196,6 +196,18 @@ impl Parser<'_> { b'n' => out.push(b'\n'), b'r' => out.push(b'\r'), b't' => out.push(b'\t'), + b'b' => out.push(0x08), + b'f' => out.push(0x0C), + b'v' => out.push(0x0B), + b'0' => out.push(0), + b'x' => { + let digits = self.s.get(self.i..self.i.checked_add(2)?)?; + let code = hex4(&[b'0', b'0', digits[0], digits[1]])?; + self.i += 2; + let ch = char::from_u32(u32::from(code)).unwrap_or('\u{FFFD}'); + let mut encoded = [0u8; 4]; + out.extend_from_slice(ch.encode_utf8(&mut encoded).as_bytes()); + } b'u' => { let code = hex4(self.s.get(self.i..self.i.checked_add(4)?)?)?; self.i += 4; @@ -1111,6 +1123,15 @@ pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option< mod tests { use super::*; + #[test] + fn javascript_escapes_are_decoded() { + assert_eq!( + parse_format("AFNumber_Format(2, 0, 0, 0, \"\\x24\", true);"), + Some(Format::Number { decimals: 2, sep: 0, neg: 0, currency: "$".into(), prepend: true }) + ); + assert_eq!(parse_format("AFDate_FormatEx(\"\\b\\f\\v\\0\\q\");"), Some(Format::Date("\u{8}\u{c}\u{b}\0q".into()))); + } + #[test] fn scripts_are_recognised_and_written_back() { assert_eq!( From 25e273a33d08a9e075dde089009a58dce9320e6b Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:47:09 +0200 Subject: [PATCH 15/23] Pair /ActualText correctly past 256 levels of marked content Fixes #3573 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/edit/src/tests.rs | 21 +++++++++++++++++++++ crates/edit/src/text.rs | 4 +++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 879ebc6c66..56755d1b94 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -5218,3 +5218,24 @@ fn a_clipping_copy_on_the_same_baseline_is_refused() { assert!(matches!(&block, Err(EditError::Invalid(m)) if m.contains("clipping path")), "{block:?}"); assert_eq!(text::text_lines(&doc, 0).unwrap().len(), 2, "refused means untouched"); } + +/// #3573: past the 256-level marked-content cap every `EMC` must still close its own sequence, +/// so `/ActualText` nested that deep is neither dropped nor applied to too little text. +#[test] +fn actual_text_pairs_past_the_marked_content_cap() { + let span = "/Span <> BDC BT /F1 24 Tf 72 700 Td (abc) Tj ET EMC"; + let outer = "/Span <> BDC BT /F1 24 Tf 72 700 Td /Y BMC (abc) Tj EMC (def) Tj ET EMC"; + for (content, expected) in [ + (span.to_string(), "REPLACED"), + (format!("{}{span}{}", "/X BMC ".repeat(256), " EMC".repeat(256)), "REPLACED"), + (format!("{}{span}{}", "/X BMC ".repeat(1000), " EMC".repeat(1000)), "REPLACED"), + (outer.to_string(), "OUTER"), + (format!("{}{outer}{}", "/X BMC ".repeat(255), " EMC".repeat(255)), "OUTER"), + ] { + let doc = text_page(&content); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.iter().map(|l| l.text.as_str()).collect::>(), [expected]); + let reading: Vec = text::reading_blocks(&doc, 0).unwrap().into_iter().map(|b| b.text).collect(); + assert_eq!(reading, [expected]); + } +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 2672592178..8ddef552dc 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -829,7 +829,9 @@ fn interpret( let mut actual: Vec> = Vec::new(); for (i, op) in ops.iter().enumerate() { match op.op.as_slice() { - b"BMC" | b"BDC" if marked.len() < 256 => { + // No depth cap: every `EMC` pops, so a capped push would pair it with the enclosing + // sequence (#3573). Both stacks stay bounded by the number of operators. + b"BMC" | b"BDC" => { // Marked text paragraphs/headings are authoritative boundaries. Ignore // artifacts and inner spans, which inherit their enclosing region. let region = matches!(op.name(0), Some(b"P" | b"H" | b"H1" | b"H2" | b"H3" | b"H4" | b"H5" | b"H6" | b"LI" | b"TD" | b"TH")); From bfc748738424d2a9c730a8ddae0bd5535af798de Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 20:08:41 +0500 Subject: [PATCH 16/23] Tagged PDF: resolve /MarkInfo /Marked when it is an indirect reference Any value in a PDF may be written as an indirect reference, the tagged flag included: `/MarkInfo << /Marked 9 0 R >>` with object 9 holding `true` is valid, and veraPDF's corpus has pass files that write it that way. Both readers of the flag tested the reference itself, so such a document was reported as untagged: doc_info "tagged": false Accessibility Checker "The document isn't marked as tagged (MarkInfo Marked)" Both now resolve the value first. On a pair of otherwise identical tagged pages, one with `/Marked true` and one with `/Marked 8 0 R`: before direct: tagged true indirect: tagged false after direct: tagged true indirect: tagged true and the checker reports the finding for neither. A document with no flag at all still fails the rule, which the test asserts alongside, so this does not turn the rule into one that always passes. Fixes #3671. --- crates/a11y/src/content.rs | 7 +++++-- crates/a11y/src/tests.rs | 20 ++++++++++++++++++++ crates/render/src/inspect.rs | 3 +++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index e8a7db377f..d4673f0fe1 100644 --- a/crates/a11y/src/content.rs +++ b/crates/a11y/src/content.rs @@ -167,12 +167,15 @@ pub(crate) fn document_rule(doc: &Document, rule: Rule, pages: &[Page], scan: &S } } Rule::TaggedPdf => { - let marked = cat.get(b"MarkInfo").map(|m| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").cloned())); + // `/Marked` may be an indirect reference like any other value, so it is resolved + // before the test below, which looks for the boolean itself (#3671). + let marked = + cat.get(b"MarkInfo").map(|m| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| doc.resolve(v)))); let mut out = Vec::new(); if !cat.contains(b"StructTreeRoot") { out.push(Finding::plain(None, "The document has no tags (structure tree)")); } - if !matches!(marked, Some(Object::Bool(true))) { + if !matches!(marked.as_deref(), Some(Object::Bool(true))) { out.push(Finding::plain(None, "The document isn't marked as tagged (MarkInfo Marked)")); } out diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index fcd681efc9..cabbf1bcf8 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -192,6 +192,26 @@ fn an_untagged_document_fails_the_document_rules() { assert_eq!((only.count(Status::Failed), only.count(Status::Skipped)), (1, 31)); } +/// `/MarkInfo /Marked` may be written as an indirect reference, like any value. The rule read the +/// reference itself and so reported a tagged document as untagged (#3671). +#[test] +fn marked_as_an_indirect_reference_still_counts_as_tagged() { + let mut bodies = good(); + // The catalog is object 1; point /Marked at a new object holding `true`. + let last = bodies.len() + 1; + bodies[0] = bodies[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {last} 0 R >>")); + bodies.push("true".into()); + let doc = pdf(&bodies, ""); + let report = check(&doc, &Options { rules: [Rule::TaggedPdf].into(), pages: None }); + assert_eq!(status(&report, Rule::TaggedPdf), Status::Passed, "{:?}", failed(&report)); + + // The direct form is unchanged, and a document with no flag at all still fails. + assert_eq!(status(&check(&pdf(&good(), ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }), Rule::TaggedPdf), Status::Passed); + let mut none = good(); + none[0] = none[0].replace("/MarkInfo << /Marked true >>", ""); + assert_eq!(status(&check(&pdf(&none, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }), Rule::TaggedPdf), Status::Failed); +} + /// A tagged document that passes every automatic rule. fn good() -> Vec { vec![ diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 0979f3d38a..f7752c4a53 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -643,6 +643,9 @@ impl<'a> Inspector<'a> { .ok() .and_then(|o| self.dict(o)) .and_then(|m| m.get(b"Marked").ok()) + // Any value may be written as an indirect reference, `/Marked 9 0 R` included, so the + // flag is resolved before it is read as a boolean (#3671). + .map(|o| self.resolve(o)) .and_then(|o| o.as_bool().ok()) .unwrap_or(false); match self.page_labels(catalog, &mut info.pages) { From 0f5e74101c3799be31ac6d126a62b50205f2cc29 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 21:52:36 +0500 Subject: [PATCH 17/23] Follow a chain of references to the tagged flag, as the inspector does Document::resolve follows one reference, and an indirect object's value may itself be a reference (ISO 32000-2 7.3.10): `/Marked 8 0 R` with object 8 holding `9 0 R` holding `true` is a tagged document. The rule was left holding a reference and reported it untagged, while the render inspector -- which follows chains through lopdf's dereference -- read the same file as tagged. The point of this branch is that the two readers agree, so the chain is followed here too. Bounded at 32 references and cycle-safe: a damaged file can point a reference at itself, and the test covers that case as well as the two-hop chain. Without the fix the chain case reports left: Failed, right: Passed Reported by CodeRabbit on #3686. --- crates/a11y/src/content.rs | 29 ++++++++++++++++++++++++++--- crates/a11y/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index d4673f0fe1..0354d3f084 100644 --- a/crates/a11y/src/content.rs +++ b/crates/a11y/src/content.rs @@ -143,6 +143,28 @@ fn catalog(doc: &Document) -> Dict { doc.root().and_then(|r| doc.get(r).as_dict().cloned()).unwrap_or_default() } +/// The most references one value may be written through. A chain is unusual but legal, and the +/// render inspector follows them (lopdf's `dereference` has a limit of its own), so the two +/// readers of a flag must agree about one. +const MAX_REFERENCE_CHAIN: usize = 32; + +/// The value a chain of indirect references ends at. [`Document::resolve`] follows one reference, +/// and an indirect object's value may itself be a reference (ISO 32000-2 §7.3.10): `/Marked +/// 8 0 R` with object 8 holding `9 0 R` holding `true` is a tagged document, which this rule read +/// as untagged while `doc_info` read it as tagged. Bounded and cycle-safe, since a damaged file +/// can point a reference at itself; a chain longer than that, or one that loops, is no value. +fn resolved(doc: &Document, o: &Object) -> std::sync::Arc { + let mut seen: HashSet = HashSet::new(); + let mut value = doc.resolve(o); + while let Object::Ref(r) = &*value { + if seen.len() >= MAX_REFERENCE_CHAIN || !seen.insert(*r) { + return std::sync::Arc::new(Object::Null); + } + value = doc.get(*r); + } + value +} + fn text_of(doc: &Document, d: &Dict, key: &[u8]) -> Option { d.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.to_text())).map(|s| s.trim().to_owned()).filter(|s| !s.is_empty()) } @@ -167,10 +189,11 @@ pub(crate) fn document_rule(doc: &Document, rule: Rule, pages: &[Page], scan: &S } } Rule::TaggedPdf => { - // `/Marked` may be an indirect reference like any other value, so it is resolved - // before the test below, which looks for the boolean itself (#3671). + // `/Marked` may be an indirect reference like any other value -- or a chain of + // them -- so it is followed to the value before the test below, which looks for the + // boolean itself (#3671). let marked = - cat.get(b"MarkInfo").map(|m| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| doc.resolve(v)))); + cat.get(b"MarkInfo").map(|m| resolved(doc, m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| resolved(doc, v)))); let mut out = Vec::new(); if !cat.contains(b"StructTreeRoot") { out.push(Finding::plain(None, "The document has no tags (structure tree)")); diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index cabbf1bcf8..2298b7fe93 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -205,6 +205,24 @@ fn marked_as_an_indirect_reference_still_counts_as_tagged() { let report = check(&doc, &Options { rules: [Rule::TaggedPdf].into(), pages: None }); assert_eq!(status(&report, Rule::TaggedPdf), Status::Passed, "{:?}", failed(&report)); + // A reference to a reference is followed too: the render inspector follows chains, so the + // two readers of the flag would otherwise disagree about one file. + let mut chain = good(); + let first = chain.len() + 1; + chain[0] = chain[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {first} 0 R >>")); + chain.push(format!("{} 0 R", first + 1)); + chain.push("true".into()); + let report = check(&pdf(&chain, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }); + assert_eq!(status(&report, Rule::TaggedPdf), Status::Passed, "{:?}", failed(&report)); + + // A reference that points at itself is no value, not a hang. + let mut loops = good(); + let at = loops.len() + 1; + loops[0] = loops[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {at} 0 R >>")); + loops.push(format!("{at} 0 R")); + let report = check(&pdf(&loops, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }); + assert_eq!(status(&report, Rule::TaggedPdf), Status::Failed); + // The direct form is unchanged, and a document with no flag at all still fails. assert_eq!(status(&check(&pdf(&good(), ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }), Rule::TaggedPdf), Status::Passed); let mut none = good(); From 3246587e855e7ba3e533c5b5394d5d5c1f4004ca Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:38:34 +0000 Subject: [PATCH 18/23] Return empty WordNum result when cents rounding carries to 1e12 Found while testing crates/js/src/formcalc.rs:2024-2050 (word_num) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/js/src/formcalc.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/js/src/formcalc.rs b/crates/js/src/formcalc.rs index dc32d457bf..e399923907 100644 --- a/crates/js/src/formcalc.rs +++ b/crates/js/src/formcalc.rs @@ -2027,6 +2027,10 @@ fn word_num(n: f64, cents: bool) -> String { } let cents_total = (n * 100.0).round() as u64; let whole = if cents { cents_total / 100 } else { n.trunc() as u64 }; + if whole >= 1_000_000_000_000 { + // Rounding to cents carried past the supported range. + return String::new(); + } let mut parts = Vec::new(); let groups = [(1_000_000_000u64, "Billion"), (1_000_000, "Million"), (1_000, "Thousand")]; let mut rest = whole; @@ -2596,6 +2600,7 @@ mod tests { assert_eq!(parse_date("2/3/01", "M/D/YY"), Some((2001, 2, 3))); assert_eq!(format_date(2001, 2, 3, "MMMM D, YYYY"), "February 3, 2001"); assert_eq!(word_num(1234.5, true), "One Thousand Two Hundred Thirty-Four Dollars And 50 Cents"); + assert_eq!(word_num(999_999_999_999.999, true), ""); assert_eq!(num_text(3.0), "3"); assert_eq!(num_text(2.5), "2.5"); assert_eq!(num_text(0.1 + 0.2), "0.3"); From c7505930b896a30e0493db1c3ed2ac585e8e5000 Mon Sep 17 00:00:00 2001 From: Aman Mehtar Date: Tue, 20 Oct 2026 23:29:13 +0530 Subject: [PATCH 19/23] M12: accept Identity CJK Unicode mappings in accessibility checks (#3673) Treat predefined CJK CMaps and Adobe CID collections as alternative mapping routes. Resolve indirect metadata and reject unknown names and foreign registries without changing ToUnicode handling. Add synthetic checker and read-only automation regressions with red/green evidence. Co-Authored-By: Claude Code --- ROADMAP.md | 4 + crates/a11y/README.md | 4 + crates/a11y/src/content.rs | 61 +++++++++++-- crates/a11y/src/tests.rs | 120 ++++++++++++++++++++++++++ crates/automation/tests/automation.rs | 57 ++++++++++++ docs/gaps.md | 8 ++ docs/roadmap.md | 7 +- docs/target-app-parity.md | 3 + parity/acrobat-features.toml | 4 +- 9 files changed, 256 insertions(+), 12 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..cbdb11995b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,8 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M12 #3673: Character encoding accepts Identity-H/V with Adobe CJK collections; estimates unchanged) + > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) @@ -172,6 +174,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-20 (M12, #3673):** Character encoding accepts Identity-H/V fonts with Adobe CJK collections and recognizes predefined non-Identity CMaps independently. Unknown CMaps and foreign registries remain refused; synthetic checker and read-only automation regressions passed after failing before the fix. Per-glyph mapping validation and embedded-font extraction remain separate. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/a11y/README.md b/crates/a11y/README.md index 0a653f1316..96cdad13cc 100644 --- a/crates/a11y/README.md +++ b/crates/a11y/README.md @@ -16,6 +16,10 @@ let html = report_html(&report, "file.pdf", "2026-10-02", "en", &in_english); / - Each result is Passed, Failed (with findings and pages), Needs manual check, or Skipped. - **Content checks** walk the page content streams (and form XObjects) tracking marked content: painting outside MCID-marked or `/Artifact` content is untagged. +- **Character encoding** accepts a predefined non-Identity CJK CMap or an Adobe CJK + character collection (`GB1`, `CNS1`, `Japan1`, `Korea1`, `KR`), including Identity-H/V. + Unknown CMaps and foreign registries do not establish that route. This is a font-level + declaration check, not validation of every ToUnicode entry or used glyph. - **Structure checks** read the structure tree through the role map; table regularity counts `ColSpan`/`RowSpan` attributes. - The report is a self-contained HTML page. Fixes that edit the document (language, title, tab diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index e8a7db377f..5811061ec4 100644 --- a/crates/a11y/src/content.rs +++ b/crates/a11y/src/content.rs @@ -360,8 +360,47 @@ fn full_name(doc: &Document, field: ObjRef) -> String { parts.join(".") } -/// Whether text in this font can be mapped to Unicode (ToUnicode, a standard encoding, or a -/// predefined CJK character collection). +/// Predefined CJK CMaps (ISO 32000-2 §9.7.5.2, Table 116), excluding Identity-H/V. +/// Test complete names: neither a custom CMap nor a nonexistent vertical variant is predefined. +fn predefined_cjk_cmap(name: &[u8]) -> bool { + if matches!(name, b"H" | b"V" | b"83pv-RKSJ-H" | b"90pv-RKSJ-H" | b"KSCpc-EUC-H" | b"UniAKR-UTF8-H" | b"UniAKR-UTF16-H" | b"UniAKR-UTF32-H") { + return true; + } + let Some(base) = name.strip_suffix(b"-H").or_else(|| name.strip_suffix(b"-V")) else { return false }; + matches!( + base, + b"GB-EUC" + | b"GBpc-EUC" + | b"GBK-EUC" + | b"GBKp-EUC" + | b"GBK2K" + | b"UniGB-UCS2" + | b"UniGB-UTF16" + | b"B5pc" + | b"HKscs-B5" + | b"ETen-B5" + | b"ETenms-B5" + | b"CNS-EUC" + | b"UniCNS-UCS2" + | b"UniCNS-UTF16" + | b"90ms-RKSJ" + | b"90msp-RKSJ" + | b"Add-RKSJ" + | b"EUC" + | b"Ext-RKSJ" + | b"UniJIS-UCS2" + | b"UniJIS-UCS2-HW" + | b"UniJIS-UTF16" + | b"KSC-EUC" + | b"KSCms-UHC" + | b"KSCms-UHC-HW" + | b"UniKS-UCS2" + | b"UniKS-UTF16" + ) +} + +/// Whether text in this font declares a Unicode mapping route (ToUnicode, a standard encoding, +/// a predefined CJK CMap, or an Adobe CJK character collection). This does not validate every glyph. fn maps_to_unicode(doc: &Document, f: &Dict) -> bool { if f.contains(b"ToUnicode") { return true; @@ -369,16 +408,22 @@ fn maps_to_unicode(doc: &Document, f: &Dict) -> bool { let enc = f.get(b"Encoding").map(|e| doc.resolve(e)); match f.name(b"Subtype") { Some(b"Type0") => { - let identity = matches!(enc.as_deref(), Some(Object::Name(n)) if n.starts_with(b"Identity")); - let ordering = f + // ISO 32000-2 §9.10.2: either a predefined non-Identity CMap or the descendant's + // Adobe CJK collection supplies the route. Identity-H/V do not rule out the latter. + if matches!(enc.as_deref(), Some(Object::Name(n)) if predefined_cjk_cmap(n)) { + return true; + } + let system = f .get(b"DescendantFonts") .map(|d| doc.resolve(d)) .and_then(|d| d.as_array().and_then(|a| a.first().cloned())) .and_then(|d| doc.resolve(&d).as_dict().cloned()) - .and_then(|d| d.get(b"CIDSystemInfo").map(|c| doc.resolve(c)).and_then(|c| c.as_dict().cloned())) - .and_then(|c| c.get(b"Ordering").map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.bytes.clone()))); - let known = matches!(ordering.as_deref(), Some(b"Japan1" | b"GB1" | b"CNS1" | b"Korea1" | b"KR")); - !identity && known + .and_then(|d| d.get(b"CIDSystemInfo").map(|c| doc.resolve(c)).and_then(|c| c.as_dict().cloned())); + let Some(system) = system else { return false }; + let registry = system.get(b"Registry").map(|r| doc.resolve(r)); + let ordering = system.get(b"Ordering").map(|o| doc.resolve(o)); + matches!(registry.as_deref(), Some(Object::String(s)) if s.bytes == b"Adobe") + && matches!(ordering.as_deref(), Some(Object::String(s)) if matches!(s.bytes.as_slice(), b"Japan1" | b"GB1" | b"CNS1" | b"Korea1" | b"KR")) } Some(b"Type3") => enc.is_some(), _ => match enc.as_deref() { diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index fcd681efc9..0889a7e356 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -536,6 +536,126 @@ fn scans_identity_fonts_and_long_documents() { assert_eq!(some.result(Rule::CharacterEncoding).unwrap().findings[0].page, Some(3)); } +fn cid_encoding_pdf(encoding: &str, system: &str, to_unicode: bool) -> Document { + let unicode = if to_unicode { "/ToUnicode 7 0 R" } else { "" }; + pdf( + &[ + "<< /Type /Catalog /Pages 2 0 R /MarkInfo << /Marked true >> >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >>".into(), + stream("/P << /MCID 0 >> BDC BT /F1 12 Tf <002200230024> Tj ET EMC"), + format!("<< /Type /Font /Subtype /Type0 /BaseFont /SyntheticCID /Encoding {encoding} /DescendantFonts [6 0 R] {unicode} >>"), + format!("<< /Type /Font /Subtype /CIDFontType0 /BaseFont /SyntheticCID /CIDSystemInfo {system} >>"), + stream( + "/CIDInit /ProcSet findresource begin 12 dict begin begincmap /CMapName /SyntheticUnicode def /CMapType 2 def 1 begincodespacerange <0000> endcodespacerange 3 beginbfchar <0022> <0041> <0023> <0042> <0024> <0043> endbfchar endcmap CMapName currentdict /CMap defineresource pop end end", + ), + ], + "", + ) +} + +#[test] +fn character_encoding_accepts_identity_cjk_collections() { + for encoding in ["/Identity-H", "/Identity-V"] { + for ordering in ["Japan1", "GB1", "CNS1", "Korea1", "KR"] { + let system = format!("<< /Registry (Adobe) /Ordering ({ordering}) /Supplement 0 >>"); + let doc = cid_encoding_pdf(encoding, &system, false); + let report = checked(&doc, &[Rule::CharacterEncoding]); + let result = report.result(Rule::CharacterEncoding).unwrap(); + assert_eq!(result.status, Status::Passed, "{encoding}, Adobe-{ordering}: {:?}", result.findings); + assert!(result.findings.is_empty()); + } + } +} + +#[test] +fn character_encoding_accepts_predefined_cmaps_without_a_known_collection() { + // The predefined CMap and the descendant's character collection are alternative routes. + let system = "<< /Registry (Private) /Ordering (Custom) /Supplement 0 >>"; + for name in [ + "GB-EUC-H", + "GB-EUC-V", + "GBK2K-H", + "GBK2K-V", + "UniGB-UTF16-H", + "UniGB-UTF16-V", + "B5pc-H", + "B5pc-V", + "UniCNS-UCS2-H", + "UniCNS-UCS2-V", + "83pv-RKSJ-H", + "90pv-RKSJ-H", + "90ms-RKSJ-H", + "90ms-RKSJ-V", + "H", + "V", + "UniJIS-UCS2-HW-H", + "UniJIS-UCS2-HW-V", + "KSCpc-EUC-H", + "KSC-EUC-V", + "KSCms-UHC-HW-H", + "KSCms-UHC-HW-V", + "UniKS-UTF16-H", + "UniKS-UTF16-V", + "UniAKR-UTF8-H", + "UniAKR-UTF16-H", + "UniAKR-UTF32-H", + ] { + let doc = cid_encoding_pdf(&format!("/{name}"), system, false); + let report = checked(&doc, &[Rule::CharacterEncoding]); + assert_eq!(status(&report, Rule::CharacterEncoding), Status::Passed, "{name}"); + } +} + +#[test] +fn character_encoding_rejects_unknown_cmaps_and_foreign_collections() { + for encoding in + ["/Identity-H", "/Identity-V", "/Custom-H", "/UniCustom-UTF16-H", "/UniJIS-UCS2-HW-extra-H", "/83pv-RKSJ-V", "/KSCpc-EUC-V", "null"] + { + for system in [ + "<< /Registry (Adobe) /Ordering (Identity) /Supplement 0 >>", + "<< /Registry (Adobe) /Ordering (Custom) /Supplement 0 >>", + "<< /Registry (Private) /Ordering (Japan1) /Supplement 0 >>", + "<< /Registry (adobe) /Ordering (Japan1) /Supplement 0 >>", + "<< /Ordering (Japan1) /Supplement 0 >>", + "<< /Registry /Adobe /Ordering /Japan1 /Supplement 0 >>", + "null", + ] { + let doc = cid_encoding_pdf(encoding, system, false); + let report = checked(&doc, &[Rule::CharacterEncoding]); + let result = report.result(Rule::CharacterEncoding).unwrap(); + assert_eq!(result.status, Status::Failed, "{encoding}, {system}"); + assert_eq!(result.findings.len(), 1); + assert_eq!(result.findings[0].page, Some(0)); + assert!(result.findings[0].message.contains("SyntheticCID")); + } + } + let doc = cid_encoding_pdf("/Identity-H", "<< /Registry (Private) /Ordering (Custom) /Supplement 0 >>", true); + assert_eq!(status(&checked(&doc, &[Rule::CharacterEncoding]), Rule::CharacterEncoding), Status::Passed, "ToUnicode is still sufficient"); +} + +#[test] +fn character_encoding_resolves_indirect_cid_metadata() { + use pdfcraft_cos::{Dict, ObjRef, PdfString}; + let mut doc = cid_encoding_pdf("/Identity-H", "null", false); + let registry = doc.add(Object::String(PdfString::literal(b"Adobe".to_vec()))); + let ordering = doc.add(Object::String(PdfString::literal(b"Japan1".to_vec()))); + let mut system = Dict::new(); + system.set(b"Registry".to_vec(), Object::Ref(registry)); + system.set(b"Ordering".to_vec(), Object::Ref(ordering)); + let system = doc.add(Object::Dict(system)); + doc.update_dict(ObjRef::new(6, 0), |d| d.set(b"CIDSystemInfo".to_vec(), Object::Ref(system))).unwrap(); + let descendants = doc.add(Object::Array(vec![Object::Ref(ObjRef::new(6, 0))])); + let encoding = doc.add(Object::Name(b"Identity-V".to_vec())); + doc.update_dict(ObjRef::new(5, 0), |d| { + d.set(b"DescendantFonts".to_vec(), Object::Ref(descendants)); + d.set(b"Encoding".to_vec(), Object::Ref(encoding)); + }) + .unwrap(); + let report = checked(&doc, &[Rule::CharacterEncoding]); + assert_eq!(status(&report, Rule::CharacterEncoding), Status::Passed); +} + #[test] fn rules_have_ids_and_the_report_lists_them() { assert_eq!(Rule::ALL.len(), 32); diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index cbf40f1962..d49428789e 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -2294,6 +2294,63 @@ fn exporting_images_and_text_through_tools() { assert!(a.call("doc_export_text", &json!({ "doc": doc, "path": "/etc/x.txt" })).is_err(), "confined to the root"); } +#[test] +fn accessibility_character_encoding_accepts_identity_cjk_through_tools() { + use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, SaveOptions, Stream, write_full}; + use std::sync::Arc; + + let dir = workdir("a11y-identity-cjk"); + let mut a = auto(&dir); + for encoding in ["Identity-H", "Identity-V"] { + let mut source = Document::open(Arc::new(fixture(1))).unwrap(); + let mut system = Dict::new(); + system.set(b"Registry".to_vec(), Object::String(PdfString::literal(b"Adobe".to_vec()))); + system.set(b"Ordering".to_vec(), Object::String(PdfString::literal(b"Japan1".to_vec()))); + system.set(b"Supplement".to_vec(), Object::Int(0)); + let system = source.add(Object::Dict(system)); + let mut descriptor = Dict::new(); + descriptor.set(b"Type".to_vec(), Object::Name(b"FontDescriptor".to_vec())); + descriptor.set(b"FontName".to_vec(), Object::Name(b"SyntheticCID".to_vec())); + descriptor.set(b"Flags".to_vec(), Object::Int(6)); + descriptor.set(b"FontBBox".to_vec(), Object::Array([0, -200, 1000, 800].into_iter().map(Object::Int).collect())); + descriptor.set(b"ItalicAngle".to_vec(), Object::Int(0)); + descriptor.set(b"Ascent".to_vec(), Object::Int(800)); + descriptor.set(b"Descent".to_vec(), Object::Int(-200)); + descriptor.set(b"CapHeight".to_vec(), Object::Int(700)); + descriptor.set(b"StemV".to_vec(), Object::Int(80)); + let descriptor = source.add(Object::Dict(descriptor)); + let mut cid = Dict::new(); + cid.set(b"Type".to_vec(), Object::Name(b"Font".to_vec())); + cid.set(b"Subtype".to_vec(), Object::Name(b"CIDFontType0".to_vec())); + cid.set(b"BaseFont".to_vec(), Object::Name(b"SyntheticCID".to_vec())); + cid.set(b"CIDSystemInfo".to_vec(), Object::Ref(system)); + cid.set(b"FontDescriptor".to_vec(), Object::Ref(descriptor)); + cid.set(b"DW".to_vec(), Object::Int(1000)); + let cid = source.add(Object::Dict(cid)); + source + .update_dict(ObjRef::new(3, 0), |font| { + font.set(b"Subtype".to_vec(), Object::Name(b"Type0".to_vec())); + font.set(b"BaseFont".to_vec(), Object::Name(b"SyntheticCID".to_vec())); + font.set(b"Encoding".to_vec(), Object::Name(encoding.as_bytes().to_vec())); + font.set(b"DescendantFonts".to_vec(), Object::Array(vec![Object::Ref(cid)])); + }) + .unwrap(); + source.set(ObjRef::new(5, 0), Object::Stream(Stream::flate(Dict::new(), b"BT /F1 24 Tf 20 150 Td <002200230024> Tj ET"))); + let name = format!("{encoding}.pdf"); + let bytes = write_full(&source, &SaveOptions::default()).unwrap(); + std::fs::write(dir.join(&name), &bytes).unwrap(); + let doc = ok(&mut a, "doc_open", json!({ "path": name }))["doc"].as_u64().unwrap(); + assert_eq!(page_text(&mut a, doc), ["ABC"], "the collection maps the CIDs to Unicode without ToUnicode"); + let report = ok(&mut a, "accessibility_check", json!({ "doc": doc, "rules": ["character-encoding"] })); + let result = report["results"].as_array().unwrap().iter().find(|r| r["rule"] == "character-encoding").unwrap(); + assert_eq!(result["status"], "passed", "{encoding}: {result}"); + assert_eq!(result["findings"], json!([])); + assert_eq!(ok(&mut a, "doc_info", json!({ "doc": doc }))["document"]["dirty"], false); + assert_eq!(std::fs::read(dir.join(name)).unwrap(), bytes, "checking is read-only"); + } + std::fs::remove_dir_all(dir).unwrap(); +} + #[test] fn accessibility_check_report_and_fixes_through_tools() { let dir = workdir("a11y"); diff --git a/docs/gaps.md b/docs/gaps.md index 2682d090ac..b201a17fee 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,5 +1,7 @@ # Where PdfCraft falls short of Acrobat Pro +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (gap 6 #3673: Character encoding CJK mapping routes corrected; no readiness re-estimate) + > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (gap 12: reading order is scored, not just word F1) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-14 · **Change:** minor (M1: raw-deflate fallback preserves decoding-limit errors; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-13 · **Change:** minor (gap 14: Vietnamese catalog shipped; Hindi/Indonesian/Korean and Arabic RTL remain) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -144,6 +146,11 @@ update the numbers. - **Fix verified (2026-10-14):** raw-deflate fallback returns a decoding-limit error instead of accepting an empty partial stream when a misleading zlib header triggers a retry. Synthetic filter and COS stream regressions cover refusal at the limit and decoding within it. +- **Fix verified (2026-10-20, #3673):** Character encoding accepts Identity-H/V fonts + whose Adobe CJK collection supplies the Unicode mapping, independently of predefined + non-Identity CMaps. Unknown CMaps and foreign registries remain refused. Synthetic checker + and read-only automation regressions cover the change; per-glyph validation and the + separate embedded-font extraction gap remain open. No readiness or backlog re-count. - **Impact:** every one is a user who lost trust. Crash reports outrank feature work (`AGENTS.md` §4). - **Estimate:** 45–80 h (≈ 0.2–0.4 h per issue). **Doc:** this file. @@ -382,6 +389,7 @@ update the numbers. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | Gap 6 #3673: Character encoding accepts Identity-H/V with Adobe CJK collections, or a predefined non-Identity CMap; negative and indirect-reference regressions and a read-only automation test passed after failing before the fix. Per-glyph validation and embedded-font extraction remain separate. Scores and estimates unchanged. | | 2026-10-15 | minor | Gap 12: the text oracle scores reading order as well as word F1, which is a bag of words and cannot see order; the first measurement separated 18 improvements from 6 regressions in a change F1 called unchanged on 26 of 29 files. Estimates unchanged. | | 2026-10-14 | minor | M1: raw-deflate fallback preserves decoding-limit errors in strict and tolerant decoding; synthetic filter and COS stream regressions. Estimates unchanged. | | 2026-10-15 | patch | Gap 24: record localization and installers as partial (unusable Chinese, Telugu and Arabic; unsigned Linux packages) | diff --git a/docs/roadmap.md b/docs/roadmap.md index 5d73511e9c..87cbba9ca3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,5 +1,7 @@ # PdfCraft roadmap: milestones and what's next +> **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-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -147,9 +149,9 @@ total matches the parity estimate. | 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. #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 | | 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). 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.
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). Missing: writing proposed tags and parent tree, field autotag, visual Tags/Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA | +| 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). Missing: writing proposed tags and parent tree, field autotag, visual Tags/Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA | | 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. | -| 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.
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; text-only Apply writes marked content, paragraph tags and a parent tree, then opens the Tags panel for review. Missing: mixed-content and field autotag, heading/list/table/figure classification, Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA
Done since: CLI info, text and render use dynamic XFA layout pages, with synthetic count/text/pixel and copy-permission regressions (#1645). | +| 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; text-only Apply writes marked content, paragraph tags and a parent tree, then opens the Tags panel for review. Missing: mixed-content and field autotag, heading/list/table/figure classification, Order/Content panels, Reading Order tool, compare, geospatial/3D measurement, search index, XFA
Done since: CLI info, text and render use dynamic XFA layout pages, with synthetic count/text/pixel and copy-permission regressions (#1645). | | 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. | | M14 | 1.0 polish: performance, localization, installers | 120–250 | 15% | 80–150 | Done: PhotoCraft's translation system (`i18n/`: TSV catalogs, `tl!`, command-id and plural entries, system-language detection, strict catalog tests); every dialog, panel and notice goes through `tl!`. Catalogs of ≈ 2,000–2,140 entries for Japanese, Simplified and Traditional Chinese, Russian, Bulgarian, German, Spanish, French, Telugu, Hungarian, Ukrainian, Brazilian Portuguese and Arabic (logical-order catalog put into display order; layout not yet mirrored); Czech covers the menus. Italian has 2,528 entries: every key of the other catalogs that the code looks up. Signed and notarized macOS builds, signed Windows installers (x64, x86, ARM64), AppImage, deb, rpm, Flatpak and FreeBSD packages. #307 (bounded large-PDF memory): shared resource indexes and parser metadata, lazy inspection, bounded caches; a 9,156-page file inspects in ≈ 1.8 s at ≈ 500 MiB instead of exceeding 10 GiB. Edits let go of the previous working file before writing the next (a 1 GiB file peaks at two copies while editing, not three; gap 13). Shortcut display uses localized modifier/key names across menus, toolbar, palette, Help and tooltips (#496). Lithuanian adds a 2,578-entry catalog and date names, reviewed by a native-speaking contributor. Validated candidate (macOS raster probe; Windows runtime unverified): the Windows policy changes only embedded JetBrains Mono; horizontal alpha-edge energy per ink improves 2.78%, 3.87% and 1.59% at 100%, 125% and 150%, and is down 0.79% at 200%. Inter is pixel-identical; layout hashes, glyph coverage and the 8 MiB atlas are unchanged. Timing and RSS ranges overlap, with no improvement claimed. Estimates unchanged. Candidate #2383 clamps GPU requests to supported limits while retaining the adapter texture extent; synthetic V3D/downlevel and Windows GPU-required tests pass, but Raspberry Pi startup is unverified. Estimates unchanged. Missing: a right-to-left layout, a Chinese UI face in releases, Hindi/Indonesian/Korean/Vietnamese, file-backed input and global memory budgets, performance budgets, keyboard-only operation Linux/Unix UI fallback fonts also use Fontconfig directories, including Nix store paths, with bounded discovery and the existing candidate allowlist; relative includes and digit-prefixed config filtering have isolated regressions. | | M14 | 1.0 polish: performance, localization, installers | 120–250 | 15% | 80–150 | Done: PhotoCraft's translation system (`i18n/`: TSV catalogs, `tl!`, command-id and plural entries, system-language detection, strict catalog tests); every dialog, panel and notice goes through `tl!`. Catalogs of ≈ 2,000–2,140 entries for Japanese, Simplified and Traditional Chinese, Russian, Bulgarian, German, Spanish, French, Telugu, Hungarian, Ukrainian, Brazilian Portuguese and Arabic (logical-order catalog put into display order; layout not yet mirrored); Czech covers the menus. Italian has 2,528 entries: every key of the other catalogs that the code looks up. Signed and notarized macOS builds, signed Windows installers (x64, x86, ARM64), AppImage, deb, rpm, Flatpak and FreeBSD packages. #307 (bounded large-PDF memory): shared resource indexes and parser metadata, lazy inspection, bounded caches; a 9,156-page file inspects in ≈ 1.8 s at ≈ 500 MiB instead of exceeding 10 GiB. Edits let go of the previous working file before writing the next (a 1 GiB file peaks at two copies while editing, not three; gap 13). Shortcut display uses localized modifier/key names across menus, toolbar, palette, Help and tooltips (#496). Lithuanian adds a 2,578-entry catalog and date names, reviewed by a native-speaking contributor. Validated candidate (macOS raster probe; Windows runtime unverified): the Windows policy changes only embedded JetBrains Mono; horizontal alpha-edge energy per ink improves 2.78%, 3.87% and 1.59% at 100%, 125% and 150%, and is down 0.79% at 200%. Inter is pixel-identical; layout hashes, glyph coverage and the 8 MiB atlas are unchanged. Timing and RSS ranges overlap, with no improvement claimed. Estimates unchanged. Missing: a right-to-left layout, a Chinese UI face in releases, Hindi/Indonesian/Korean/Vietnamese, file-backed input and global memory budgets, performance budgets, keyboard-only operation XDG desktops use the configuration base for preferences with non-overwriting migration and original-data preservation (#1395). | @@ -200,6 +202,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | M12 #3673: Character encoding accepts Identity-H/V with Adobe CJK collections, or a predefined non-Identity CMap; unknown names and foreign registries do not establish a mapping route. Synthetic checker and read-only automation regressions passed after failing before the fix. Per-glyph validation and embedded-font extraction remain separate; estimates unchanged. | | 2026-10-18 | minor | M5, PR #1971: nested source `/Page` and `/Pages` dictionaries export as null without traversing page content or the page tree. Synthetic saved/reopened three-page regression retains unrelated metadata and aliases. FDF import semantics, copied-graph limits and estimates unchanged. | | 2026-10-15 | minor | M5: budget retained FDF entries before cloning; discovery keeps lazy array owners and annotation positions, with alias/cache-eviction stress fixtures. Limits cover the copied graph; source parsing/page discovery retain existing COS/parser limits, with no total source-memory bound. FDF import semantics and estimates unchanged. | | 2026-10-15 | minor | M7: Edit text caret height follows its row, with a kittest regression; estimates unchanged | diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 9119063e20..39dc6be519 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,5 +1,7 @@ # PdfCraft parity with Adobe Acrobat Pro +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M12 #3673: Character encoding CJK mapping routes corrected; scores and estimates unchanged) + > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (localization ≈59% → ≈68%; weighted readiness 49.25 → 49.7 ≈ 50%) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) The authoritative assessment of how close PdfCraft is to Acrobat Pro. [ROADMAP.md](../ROADMAP.md) @@ -292,6 +294,7 @@ licensed OCR or AI model. | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | M12 #3673: the Character encoding rule accepts Identity-H/V fonts with Adobe CJK collections, or a predefined non-Identity CMap, while rejecting unknown names and foreign registries. Synthetic checker and read-only automation regressions passed after failing before the fix; per-glyph mapping validation and embedded-font extraction remain separate. Breadth, readiness and hours unchanged. | | 2026-10-12 | minor | Exclusive image crops resize with their pixels across nested states, intervening operators and content streams; shared clips remain fixed. Synthetic engine/tool regressions and private image save/reopen checks; estimates unchanged. | | 2026-10-11 | minor | Web printing shipped (gap 4): Hardware, Platforms and L Print text updated. Text only: scores, readiness and hours not re-judged | | 2026-10-11 | minor | Contributor fork: document-font selection and subset completion, CFF, style ranges, contextual/bidi save/reopen and all-page Chinese keyboard acceptance; curve/hole hit testing (shared clips stay fixed, as on main). Scope remains bounded; percentages unchanged | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..15e1d7c57c 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -8600,8 +8600,8 @@ milestone = "M12" status = "shipped" commands = ["a11y.check"] tools = ["accessibility_check"] -evidence = ["crates/a11y/src/tests.rs::scans_identity_fonts_and_long_documents"] -notes = "rule character-encoding: ToUnicode, a standard encoding, or a predefined CJK collection; each font once" +evidence = ["crates/a11y/src/tests.rs::scans_identity_fonts_and_long_documents", "crates/a11y/src/tests.rs::character_encoding_accepts_identity_cjk_collections", "crates/a11y/src/tests.rs::character_encoding_accepts_predefined_cmaps_without_a_known_collection", "crates/a11y/src/tests.rs::character_encoding_rejects_unknown_cmaps_and_foreign_collections", "crates/a11y/src/tests.rs::character_encoding_resolves_indirect_cid_metadata", "crates/automation/tests/automation.rs::accessibility_character_encoding_accepts_identity_cjk_through_tools"] +notes = "rule character-encoding: ToUnicode, a standard encoding, a predefined non-Identity CJK CMap, or Adobe GB1/CNS1/Japan1/Korea1/KR; Identity-H/V accepted for those collections (#3673), indirect metadata resolved and unknown CMaps/foreign registries refused; each font once. Font-level declarations only: ToUnicode contents and every used glyph are not validated; embedded-font extraction gaps remain separate." [[feature]] id = "a11y.rule-tagged-multimedia" From cbcdca77f0923acf7ea8187fb6b7b5b313154dfd Mon Sep 17 00:00:00 2001 From: Unhumanje <54317404+petarlaf@users.noreply.github.com> Date: Wed, 21 Oct 2026 01:24:42 +0200 Subject: [PATCH 20/23] fix(organize): imports find the objects they added when new numbers fill gaps import_pages_with and insert_file_pages took the objects numbered above the destination's highest number for the ones the import added. When no number is left above the highest (an exhausted /Size, or object 4294967295), Document::add fills free numbers below it: the copies skipped resource sharing, and a refused import left its objects in the destination. With object 4294967295 the `n + 1` overflowed, a panic in debug builds and in release a wrap to 0 that counted every object as added. Record the object numbers before the import and take the ones present afterwards but not before. --- crates/organize/src/import.rs | 22 ++++--- crates/organize/src/tests.rs | 111 +++++++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 23 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 588539866a..80f90fb2a4 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -17,7 +17,7 @@ //! - layers (optional content groups) used by the pages are registered in the destination's //! `/OCProperties`, keeping their default on/off state. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; @@ -240,23 +240,31 @@ pub fn import_pages_with( at: usize, conflicts: FieldConflicts, ) -> Result { - let first_new = dst.object_numbers().last().map_or(1, |n| n + 1); + let before: HashSet = dst.object_numbers().into_iter().collect(); let imported = match import_pages_mapped(dst, src, src_pages, at, conflicts) { Ok(imported) => imported, Err(e) => { // A refusal while copying (a source too deep to follow) has only added objects: // take them back, so the destination is as it was. - for n in dst.object_numbers().into_iter().filter(|n| *n >= first_new) { - dst.free(ObjRef::new(n, dst.generation(n))); + for r in added_objects(dst, &before) { + dst.free(r); } return Err(e); } }; - let created: Vec = dst.object_numbers().into_iter().filter(|n| *n >= first_new).map(|n| ObjRef::new(n, dst.generation(n))).collect(); + let created = added_objects(dst, &before); crate::dedupe::dedupe_resources(dst, &created, true); Ok(ImportOutcome { pages: imported.pages, renamed_fields: imported.renamed_fields }) } +/// The objects in `dst` that are not among `before`, the numbers it held before an import: the +/// ones the import added. They are not always numbered above the others: when no number is left +/// above the highest (the document holds object 4294967295, or its `/Size` is exhausted), +/// [`Document::add`] fills free numbers below it. +fn added_objects(dst: &Document, before: &HashSet) -> Vec { + dst.object_numbers().into_iter().filter(|n| !before.contains(n)).map(|n| ObjRef::new(n, dst.generation(n))).collect() +} + /// Insert Pages from File: [`import_pages_with`], and the inserted file's bookmarks come along /// under one new bookmark titled `title` (the file's name) at its first inserted page, as Combine /// Files does for each file. That bookmark goes among the top-level bookmarks where the pages went: @@ -270,9 +278,9 @@ pub fn insert_file_pages( at: usize, conflicts: FieldConflicts, ) -> Result { - let first_new = dst.object_numbers().last().map_or(1, |n| n + 1); + let before: HashSet = dst.object_numbers().into_iter().collect(); let imported = import_pages_mapped(dst, src, src_pages, at, conflicts)?; - let created: Vec = dst.object_numbers().into_iter().filter(|n| *n >= first_new).map(|n| ObjRef::new(n, dst.generation(n))).collect(); + let created = added_objects(dst, &before); crate::dedupe::dedupe_resources(dst, &created, true); let outline = source_outline(src, imported.page_map); if let Some(first) = imported.pages.first() diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 9bd3eb396a..81123060a2 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -2750,27 +2750,31 @@ fn combine_field_conflicts_previews_each_file_in_order() { assert_eq!(preview[2], [("Text1".to_string(), "Text1-2".to_string())], "the cascade continues past the earlier rename"); } +/// A one-page source whose page refers (`/Foo`) to object 6, followed by `tail` as objects 6, 7, … +fn chained(tail: &[String]) -> Document { + let mut b: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Foo 6 0 R >>".into(), + body("C1"), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(), + ]; + b.extend_from_slice(tail); + open(build(&b, "/Root 1 0 R")) +} + +/// [`chained`] with `n` objects, each referring to the next. +fn chain(n: usize) -> Document { + let mut tail: Vec = (0..n).map(|i| format!("<< /Next {} 0 R >>", 7 + i)).collect(); + tail.push("<< /End true >>".into()); + chained(&tail) +} + /// #1800: thousands of shallow objects, each referring to the next, used to recurse once per /// object and exhaust the stack. The copy is refused instead, and the destination is as it was; /// a chain within the budget and a short cycle copy as before. #[test] fn a_chain_of_objects_too_deep_to_copy_is_refused_and_leaves_the_destination_alone() { - fn chained(tail: &[String]) -> Document { - let mut b: Vec = vec![ - "<< /Type /Catalog /Pages 2 0 R >>".into(), - "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), - "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Foo 6 0 R >>".into(), - body("C1"), - "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(), - ]; - b.extend_from_slice(tail); - open(build(&b, "/Root 1 0 R")) - } - fn chain(n: usize) -> Document { - let mut tail: Vec = (0..n).map(|i| format!("<< /Next {} 0 R >>", 7 + i)).collect(); - tail.push("<< /End true >>".into()); - chained(&tail) - } // On a thread with less stack than any platform's main thread, so the budget is proven. let run = std::thread::Builder::new() .stack_size(512 << 10) @@ -2790,6 +2794,81 @@ fn a_chain_of_objects_too_deep_to_copy_is_refused_and_leaves_the_destination_alo assert!(run.is_ok(), "the copy must not overflow the stack"); } +/// The fixture after an update that adds object 20 and declares `/Size 4294967295`. No number is +/// left above that, so [`Document::add`] fills free numbers from 12 on, the gap below object 20 first. +fn fixture_with_an_exhausted_size() -> Document { + let mut bytes = fixture(); + let text = String::from_utf8_lossy(&bytes).into_owned(); + let prev: usize = text.rsplit("startxref").next().and_then(|t| t.split_whitespace().next()).and_then(|n| n.parse().ok()).expect("startxref"); + let added = bytes.len(); + bytes.extend_from_slice(b"20 0 obj\n<< /Unused true >>\nendobj\n"); + let xref = bytes.len(); + bytes.extend_from_slice( + format!( + "xref\n20 1\n{added:010} 00000 n \ntrailer\n<< /Size 4294967295 /Root 1 0 R /Info 11 0 R /Prev {prev} >>\nstartxref\n{xref}\n%%EOF\n" + ) + .as_bytes(), + ); + let doc = open(bytes); + assert!(doc.repair_log().iter().any(|l| l.contains("exhausted trailer")), "{:?}", doc.repair_log()); + doc +} + +/// The fixture holding object 4294967295, as a document does once an edit writes to an object of +/// that number (a damaged file's catalog, say). [`Document::add`] then fills free numbers below it. +fn fixture_holding_object_4294967295() -> Document { + let mut doc = open(fixture()); + doc.set(ObjRef::new(u32::MAX, 0), Object::String(PdfString::text("top"))); + doc +} + +/// Documents whose new objects are numbered in the gaps below their highest number. +fn documents_numbering_new_objects_below_their_highest() -> [(&'static str, Document); 2] { + [("an exhausted /Size", fixture_with_an_exhausted_size()), ("object 4294967295", fixture_holding_object_4294967295())] +} + +/// Duplicate, Replace and Insert Pages took the objects numbered above the destination's highest +/// number for the ones the import added. When new objects fill gaps below it, the copies escaped +/// resource sharing (each import stored the fonts again), and with object 4294967295 the number +/// after the highest overflowed: a panic in debug builds, and in release a wrap to 0 that counted +/// every object as added. The import now compares the object numbers before and after. +#[test] +fn an_import_shares_the_resources_it_added_wherever_they_are_numbered() { + let mut plain = open(fixture()); + duplicate_pages(&mut plain, &[0], None).unwrap(); + for (what, mut doc) in documents_numbering_new_objects_below_their_highest() { + let before = doc.object_numbers(); + duplicate_pages(&mut doc, &[0], None).unwrap(); + assert_eq!(labels(&doc), ["Page 1", "Page 1", "Page 2", "Page 3"], "{what}"); + assert_eq!(count_fonts(&doc), count_fonts(&plain), "{what}: the copy shares the font, as it does in the plain fixture"); + insert_file_pages(&mut doc, &open(fixture()), "Inserted", &[2], 0, FieldConflicts::Rename).unwrap(); + assert_eq!(labels(&doc), ["Page 3", "Page 1", "Page 1", "Page 2", "Page 3"], "{what}"); + assert_eq!(count_fonts(&doc), count_fonts(&plain), "{what}: the inserted page shares the font too"); + let after = doc.object_numbers(); + assert!(before.iter().all(|n| after.contains(n)), "{what}: every object the document had is still there"); + assert_eq!(labels(&full_roundtrip(&doc)), ["Page 3", "Page 1", "Page 1", "Page 2", "Page 3"], "{what}"); + } +} + +/// A refused import takes back what it added, which in these documents is numbered below their +/// highest object, and leaves the objects they had. +#[test] +fn a_refused_import_takes_back_what_it_added_wherever_it_is_numbered() { + let run = std::thread::Builder::new() + .stack_size(512 << 10) + .spawn(|| { + for (what, mut dst) in documents_numbering_new_objects_below_their_highest() { + let before = (labels(&dst), dst.object_numbers()); + let err = import_pages(&mut dst, &chain(20_000), &[0], 0).unwrap_err(); + assert!(matches!(&err, OrganizeError::Invalid(m) if m.contains("levels deep")), "{what}: {err:?}"); + assert_eq!((labels(&dst), dst.object_numbers()), before, "{what}: the destination is as it was"); + } + }) + .unwrap() + .join(); + assert!(run.is_ok(), "a refused import must leave the document as it was, without panicking"); +} + #[test] fn rotate_repeated_indices_accumulate_from_the_current_page_value() { for level in 0..3 { From eaed3409f83fa483e58ada700f3a4cf6a2a47e1d Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 21 Oct 2026 15:37:35 -0400 Subject: [PATCH 21/23] M14: the Preferences OK button stays in reach below the scrolling settings (#3557) The settings scroll when the window is short, but OK was the last thing inside the scrolled content, so in a short window it was off screen until you scrolled to it, and the dialog was cut off partway through the theme cards. OK is now a row below the scroll area, outside it. The scroll area gets the height the window leaves (at least 60 pt, so OK stays on screen in the smallest window, 520 pt, at the largest interface size, 300%) and a scrollbar of its own instead of the floating one that lay over the right edge of the theme cards. A scroll area starts from the room its modal offers, and a modal offers more each frame for the first few, so with OK outside the scroll the dialog opened small and stretched by about 45 pt a frame for ten frames, and clicks in the first frames missed. The scroll area is asked for its full height from the start (min_scrolled_height, which egui applies only when the settings overflow it), so it opens at full size and does not stretch in a tall window. - test: OK and the title are on screen at window heights from 173 to 1100; OK closes the dialog and does not move while the settings scroll, and a far-down setting scrolls into view between the title and OK; the dialog does not grow after it opens; a very tall window has no blank gap above OK. Both the original layout and the stretching variants fail them. - parity: misc.preferences-dialog evidence and what is still missing. --- ROADMAP.md | 2 + crates/ui-egui/src/dialogs.rs | 15 +++- crates/ui-egui/src/js_ui.rs | 17 +++- crates/ui-egui/tests/preferences_layout.rs | 98 ++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/tests/preferences_layout.rs diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..281f220502 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -591,3 +591,5 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-13 (M7):** Edit a PDF ▸ Add content: Header and footer, Watermark and Background open an Add / Update / Remove menu (Update and Remove enabled only when the document has that mark), as Acrobat's Watermark control does. The catalogue gains `ToolItem::actions`, so any tool can list more commands (#1192). - **2026-10-18 (M8, #1350):** Published the history-preserving integration; simplified the inherited-form resource guard for Rust 1.96 Clippy. Formatting, 64 redaction tests, scoped Clippy, layering, assets and the scoped wasm check pass locally. CI remains pending; estimates unchanged. + +- **2026-10-21 (M14, #3557):** Preferences: the OK button stays below the scrolling settings instead of being the last thing in them, so it is in reach in a short window. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 981db214b2..82754376ca 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -964,8 +964,19 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { } Dialog::Preferences => { // The settings outgrow a short window: scroll them rather than run off the screen. - let max = (ui.ctx().content_rect().height() - 120.0).max(200.0); - close = egui::ScrollArea::vertical().max_height(max).show(ui, |ui| crate::js_ui::preferences_body(ui, app, &t)).inner; + // The OK row stays below the scroll, in reach however far the settings are scrolled. + let max = (ui.ctx().content_rect().height() - 120.0 - crate::js_ui::PREFERENCES_BUTTON_ROW).max(crate::js_ui::PREFERENCES_MIN_SCROLL); + // A scroll area starts from the room the dialog offers it, and a modal offers more + // each frame for the first few, so with OK outside the scroll the dialog would grow + // over those frames. Asking for the full height as the least it may scroll in + // (which only applies when the settings overflow it) opens the dialog at full size. + // A scrollbar of its own: the floating one lies over the right edge of the theme + // cards and other full-width rows (#3557). + ui.scope(|ui| { + ui.spacing_mut().scroll.floating = false; + egui::ScrollArea::vertical().max_height(max).min_scrolled_height(max).show(ui, |ui| crate::js_ui::preferences_body(ui, app, &t)); + }); + close = crate::js_ui::preferences_buttons(ui); return; } Dialog::RecognizeText => { diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index df461fc6e7..00b129b9bb 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -261,7 +261,18 @@ fn reading_mode_label(mode: &str) -> &'static str { } /// Preferences: interface language and size, appearance, identity and JavaScript. Returns `true` to close. -pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { +/// The height the Preferences dialog keeps below its scrolling settings for the OK row: the +/// button, and the space above it. +pub(crate) const PREFERENCES_BUTTON_ROW: f32 = 52.0; + +/// The least height the scrolling settings are given. The dialog around them takes about 80 more, +/// so this still fits the smallest window (520 pt) at the largest interface size (300%, which +/// leaves 173 pt): a taller floor would push OK off the screen again. +pub(crate) const PREFERENCES_MIN_SCROLL: f32 = 60.0; + +/// The settings, to be drawn in a scroll area; [`preferences_buttons`] goes below it, outside the +/// scroll, so OK is always in reach. +pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { ui.label(egui::RichText::new(tl!("Preferences")).font(theme::semibold(18.0))); ui.horizontal(|ui| { ui.label(tl!("Interface language")); @@ -490,6 +501,10 @@ pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tok .color(t.text_muted), ); }); +} + +/// The Preferences dialog's OK row. Returns `true` to close. +pub(crate) fn preferences_buttons(ui: &mut egui::Ui) -> bool { ui.add_space(10.0); buttons(ui, tl!("OK"), &[]).is_some() } diff --git a/crates/ui-egui/tests/preferences_layout.rs b/crates/ui-egui/tests/preferences_layout.rs new file mode 100644 index 0000000000..3b8d41ec06 --- /dev/null +++ b/crates/ui-egui/tests/preferences_layout.rs @@ -0,0 +1,98 @@ +//! The Preferences dialog keeps OK in reach in a short window (#3557): the settings scroll, the +//! OK row does not. + +use egui::Rect; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use pdfcraft_ui_egui::PdfCraftApp; + +fn harness(height: f32) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1000.0, height)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.set_option("dialog", "preferences").unwrap(); + app + }); + h.run_steps(6); + h +} + +fn on_screen(rect: Rect, height: f32) -> bool { + rect.min.x >= 0.0 && rect.min.y >= 0.0 && rect.max.x <= 1000.0 && rect.max.y <= height +} + +#[test] +fn ok_is_on_screen_without_scrolling_at_any_window_height() { + // From a short laptop window to a tall monitor, the settings never fit the short ones. 173 is + // the smallest window (520 pt) at the largest interface size (300%), 208 is 250%. + for height in [173.0, 208.0, 260.0, 420.0, 480.0, 560.0, 700.0, 900.0, 1100.0] { + let h = harness(height); + let ok = h.get_by_label("OK").rect(); + assert!(on_screen(ok, height), "{height}: OK at {ok:?} is off the {height} pt window"); + let title = h.get_by_label("Preferences").rect(); + assert!(on_screen(title, height), "{height}: the title at {title:?} is off the window"); + // The OK row sits below the scrolled settings, never over them: the settings' last visible + // row ends above it. The title is above everything. + assert!(title.max.y < ok.min.y, "{height}: title {title:?} above OK {ok:?}"); + } +} + +#[test] +fn ok_closes_the_dialog_in_a_short_window_and_the_settings_still_scroll() { + let mut h = harness(480.0); + // A setting far down the list starts out below the visible settings; scrolling brings it in + // between the title and the OK row, and OK did not move. + let ok_before = h.get_by_label("OK").rect(); + let title = h.get_by_label("Preferences").rect(); + let far = h.get_by_label_contains("Reading colours").rect(); + assert!(far.min.y >= ok_before.min.y, "Reading colours starts out of sight: {far:?} vs OK {ok_before:?}"); + h.get_by_label_contains("Reading colours").scroll_to_me(); + h.run_steps(4); + let far = h.get_by_label_contains("Reading colours").rect(); + assert!(far.min.y >= title.max.y && far.max.y <= ok_before.min.y, "Reading colours scrolled in between the title and OK: {far:?}"); + assert_eq!(h.get_by_label("OK").rect(), ok_before, "OK stays put while the settings scroll"); + assert!(h.state().dialog.is_some()); + h.get_by_label("OK").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none(), "OK closes Preferences"); +} + +#[test] +fn the_dialog_opens_at_its_full_size_and_does_not_grow_afterwards() { + // With OK outside the scroll, the scroll area started from what the modal offered, which + // grew some 45 pt a frame for ten frames: the dialog opened small and stretched, and clicks + // in its first frames landed on controls that were still moving. + for height in [260.0, 560.0, 900.0] { + let mut h = harness(height); + let settled = (h.get_by_label("Preferences").rect(), h.get_by_label("OK").rect()); + h.run_steps(8); + let later = (h.get_by_label("Preferences").rect(), h.get_by_label("OK").rect()); + assert_eq!(settled, later, "{height}: the dialog moved after it had been shown for six frames"); + } + // And it is in place within a few frames of opening, for the first click: a frame or two of + // settling is fine, ten frames of stretching is not. + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app + }); + h.run_steps(4); + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::Preferences); + h.run_steps(3); + let early = h.get_by_label("Right").rect(); + h.run_steps(8); + let late = h.get_by_label("Right").rect(); + assert!((early.min.y - late.min.y).abs() <= 2.0 && (early.min.x - late.min.x).abs() <= 2.0, "{early:?} then {late:?}"); +} + +#[test] +fn a_very_tall_window_does_not_leave_a_blank_gap_above_ok() { + // The settings are some 1 400 pt tall; in a taller window the scroll area is not stretched + // past them. + let mut h = harness(2600.0); + h.run_steps(6); + let ok = h.get_by_label("OK").rect(); + let last = h.get_by_label_contains("Enable Acrobat JavaScript").rect(); + assert!(on_screen(ok, 2600.0) && on_screen(last, 2600.0), "{last:?} {ok:?}"); + assert!(ok.min.y - last.max.y < 120.0, "a gap of {} pt between the last setting and OK", ok.min.y - last.max.y); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..5ccebf6e05 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -9792,8 +9792,8 @@ milestone = "M14" status = "partial" commands = ["app.preferences"] tools = ["js_enabled"] -evidence = ["crates/ui-egui/tests/javascript.rs::document_scripts_and_preferences", "crates/ui-egui/tests/ui.rs::preferences_selects_default_workspace_for_next_open", "crates/ui-egui/tests/control.rs::control_default_workspace_and_session_override", "apps/pdfcraft/src/settings.rs::xdg_preferences_use_config_instead_of_data", "apps/pdfcraft/src/settings.rs::migration_preserves_preferences_and_other_data"] -notes = "Preferences dialog with Interface language, Documents and view (persistent default workspace with session override), Identity, Fill & Sign (flatten on save, off by default) and JavaScript; other categories still live in their tools, and it isn't generated from a schema yet. XDG desktops store app.ron under XDG_CONFIG_HOME/pdfcraft or ~/.config/pdfcraft; existing data-folder preferences are copied without replacing configured preferences, with fallback on migration errors. Logs and recovery retain data locations. Portable/non-XDG locations are unchanged." +evidence = ["crates/ui-egui/tests/javascript.rs::document_scripts_and_preferences", "crates/ui-egui/tests/ui.rs::preferences_selects_default_workspace_for_next_open", "crates/ui-egui/tests/control.rs::control_default_workspace_and_session_override", "apps/pdfcraft/src/settings.rs::xdg_preferences_use_config_instead_of_data", "apps/pdfcraft/src/settings.rs::migration_preserves_preferences_and_other_data", "crates/ui-egui/tests/preferences_layout.rs::ok_is_on_screen_without_scrolling_at_any_window_height", "crates/ui-egui/tests/preferences_layout.rs::ok_closes_the_dialog_in_a_short_window_and_the_settings_still_scroll"] +notes = "Preferences dialog with Interface language, Documents and view (persistent default workspace with session override), Identity, Fill & Sign (flatten on save, off by default) and JavaScript; other categories still live in their tools, and it isn't generated from a schema yet. XDG desktops store app.ron under XDG_CONFIG_HOME/pdfcraft or ~/.config/pdfcraft; existing data-folder preferences are copied without replacing configured preferences, with fallback on migration errors. Logs and recovery retain data locations. Portable/non-XDG locations are unchanged. The settings scroll when the window is short (down to a 60 pt scroll area, so OK stays on screen in the smallest window at the largest interface size) and the OK row stays below them, in reach (#3557); the dialog still has no Cancel, and a window shorter than the dialog's title, one row and OK would still clip it." [[feature]] id = "misc.prefs-general" From ad28a5073aa09c79879f9066ffd2653e198edae7 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 21 Oct 2026 16:11:11 -0400 Subject: [PATCH 22/23] js_ui: put the preferences_body doc comment back on preferences_body (#3557) The old doc comment was left above the constants added in this change, where it documents PREFERENCES_BUTTON_ROW wrongly, and its "Returns true to close" no longer holds for preferences_body, which returns (). Move its first sentence onto preferences_body, drop the "Returns" sentence, and leave each constant with only its own comment. --- crates/ui-egui/src/js_ui.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index 00b129b9bb..10e86a1dbf 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -260,7 +260,6 @@ fn reading_mode_label(mode: &str) -> &'static str { } } -/// Preferences: interface language and size, appearance, identity and JavaScript. Returns `true` to close. /// The height the Preferences dialog keeps below its scrolling settings for the OK row: the /// button, and the space above it. pub(crate) const PREFERENCES_BUTTON_ROW: f32 = 52.0; @@ -270,8 +269,8 @@ pub(crate) const PREFERENCES_BUTTON_ROW: f32 = 52.0; /// leaves 173 pt): a taller floor would push OK off the screen again. pub(crate) const PREFERENCES_MIN_SCROLL: f32 = 60.0; -/// The settings, to be drawn in a scroll area; [`preferences_buttons`] goes below it, outside the -/// scroll, so OK is always in reach. +/// Preferences: interface language and size, appearance, identity and JavaScript. Drawn in a scroll +/// area; [`preferences_buttons`] goes below it, outside the scroll, so OK is always in reach. pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) { ui.label(egui::RichText::new(tl!("Preferences")).font(theme::semibold(18.0))); ui.horizontal(|ui| { From f1e5aa1f3092554e5b39e918c07fa4d5f32baf5f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 01:46:04 +0000 Subject: [PATCH 23/23] Integration: progress log newest first --- ROADMAP.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 7846ae4680..eb2c85aae5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -175,10 +175,12 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-21 (M14, #3557):** Preferences: the OK button stays below the scrolling settings instead of being the last thing in them, so it is in reach in a short window. + - **2026-10-20 (M6, calculation order page):** Prepare a form ▸ Calculation order, a page of its own, lists the calculated fields in the order they calculate with their formulas and values, moved with Up/Down arrows (as Acrobat's Set Field Calculation Order), Alt+↑/↓ or by dragging. A card says what is wrong with the order and holds Fix order; a loop has a card of its own that opens one of its formulas. On the page, each calculated field shows its step, and arrows show what the field in focus reads and what reads it. Estimates unchanged. - **2026-10-20 (M6, calculation order):** The order a form's fields calculate in (`/CO`) is read, checked, fixed and set headlessly with `form_calculation_order`: it says which fields read one calculated later (they show stale values), which read each other in a loop, and which the PDF's order leaves out; Fix order moves only what must move, in one undo step, and the form recalculates. Editing a formula no longer moves its field to the end of the order, and a field the order lists twice calculates once. Estimates unchanged. -- **2026-10-19 (M6, renames keep formulas):** Renaming a field renames it in every calculation that reads it, in the same undo step: `AFSimple_Calculate` field lists, simplified field notation (the comment and the JavaScript after it) and `getField` calls naming it in one string literal; the rest of each script is kept as written. A sum used to drop the renamed field, notation read it as 0 and a script kept a stale value. Field Properties says how many formulas changed and `form_set_props` lists them as `formulas_updated`. Names built at run time and other events' scripts are not rewritten. Estimates unchanged. - **2026-10-20 (M12, #3673):** Character encoding accepts Identity-H/V fonts with Adobe CJK collections and recognizes predefined non-Identity CMaps independently. Unknown CMaps and foreign registries remain refused; synthetic checker and read-only automation regressions passed after failing before the fix. Per-glyph mapping validation and embedded-font extraction remain separate. Estimates unchanged. +- **2026-10-19 (M6, renames keep formulas):** Renaming a field renames it in every calculation that reads it, in the same undo step: `AFSimple_Calculate` field lists, simplified field notation (the comment and the JavaScript after it) and `getField` calls naming it in one string literal; the rest of each script is kept as written. A sum used to drop the renamed field, notation read it as 0 and a script kept a stale value. Field Properties says how many formulas changed and `form_set_props` lists them as `formulas_updated`. Names built at run time and other events' scripts are not rewritten. Estimates unchanged. - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. @@ -603,5 +605,3 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-13 (M7):** Edit a PDF ▸ Add content: Header and footer, Watermark and Background open an Add / Update / Remove menu (Update and Remove enabled only when the document has that mark), as Acrobat's Watermark control does. The catalogue gains `ToolItem::actions`, so any tool can list more commands (#1192). - **2026-10-18 (M8, #1350):** Published the history-preserving integration; simplified the inherited-form resource guard for Rust 1.96 Clippy. Formatting, 64 redaction tests, scoped Clippy, layering, assets and the scoped wasm check pass locally. CI remains pending; estimates unchanged. - -- **2026-10-21 (M14, #3557):** Preferences: the OK button stays below the scrolling settings instead of being the last thing in them, so it is in reach in a short window.