From e76ebb9ea3ffc1642cbcf5cf5347b2952e19618e Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:20:36 +0800 Subject: [PATCH 1/6] M6: a document script that throws no longer stops the form's other scripts (#3940) Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/a11y.rs | 4 +++- crates/automation/src/tools.rs | 2 +- crates/engine/src/js.rs | 7 +++++++ crates/engine/src/tests.rs | 28 +++++++++++++++++++++++++++- crates/js/src/lib.rs | 12 ++++++++++-- crates/js/src/tests.rs | 28 ++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/crates/automation/src/a11y.rs b/crates/automation/src/a11y.rs index 21bde353e..6392454db 100644 --- a/crates/automation/src/a11y.rs +++ b/crates/automation/src/a11y.rs @@ -669,7 +669,9 @@ impl Automation { let id = self.doc(a)?.id; let o = self.session.run_javascript(id, a.str("script")?, a.opt_str("field")?).map_err(failed)?; let reqs: Vec = o.requests.iter().map(request_json).collect(); - Ok(json!({ "alerts": o.alerts, "console": o.console, "requests": reqs, "error": o.error, "result": o.result })) + Ok( + json!({ "alerts": o.alerts, "console": o.console, "requests": reqs, "error": o.error, "document_errors": o.doc_errors, "result": o.result }), + ) } pub(crate) fn js_document_scripts(&self, a: &Args) -> Result { diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 4fff59c07..e5c901717 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -1094,7 +1094,7 @@ pub fn tools() -> Vec { t("form_merge_data", "Merge data files into spreadsheet", "Collect the field values of form data files (FDF, XFDF) or filled-in PDF forms into one CSV file at path: a column per field name, a row per file. Returns the row and column counts.") .cmd("form.merge_data") .with(schema(json!({ "paths": { "type": "array", "items": { "type": "string" }, "minItems": 1 }, "path": { "type": "string" } }), &["paths", "path"])), - t("js_run", "Run JavaScript", "Run Acrobat JavaScript in the document, as the JavaScript console does (or as push button `field`'s Mouse Up script when field is given; on a laid-out XFA form, `field` runs that button's XFA click script instead, JavaScript or FormCalc, which can add and remove rows and show or hide subforms). The form object model is available: this/getField, event, app, util, console, display, color, and the document-level scripts. Field changes and resetForm are applied as one undoable step; returns the script's alerts, console output, requests (print, page, url, submit) and error.") + t("js_run", "Run JavaScript", "Run Acrobat JavaScript in the document, as the JavaScript console does (or as push button `field`'s Mouse Up script when field is given; on a laid-out XFA form, `field` runs that button's XFA click script instead, JavaScript or FormCalc, which can add and remove rows and show or hide subforms). The form object model is available: this/getField, event, app, util, console, display, color, and the document-level scripts. Field changes and resetForm are applied as one undoable step; returns the script's alerts, console output, requests (print, page, url, submit), error, and the errors of document-level scripts that failed (the rest still ran).") .cmd("tools.js_console") .with(schema( json!({ "doc": doc(), "script": { "type": "string" }, "field": { "type": "string", "description": "Run as this button's Mouse Up event." } }), diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index a59743dbb..28e593531 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -31,6 +31,13 @@ impl JsOutput { self.console.extend(o.console.iter().cloned()); self.requests.extend(o.requests.iter().cloned()); self.errors.extend(o.error.iter().cloned()); + // The document scripts run before every field script, so a failing one would repeat + // once per calculation; it is reported once. + for e in &o.doc_errors { + if !self.errors.contains(e) { + self.errors.push(e.clone()); + } + } } pub(crate) fn append(&mut self, other: JsOutput) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c8f0eb886..288ebec0d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4015,6 +4015,11 @@ fn recognize_text_reads_a_picture_on_a_page_that_has_text() { /// A form whose scripts are custom JavaScript: total = price × qty (calculate, through a /// document-level function), shown with a custom format; qty is validated; a button script. fn scripted_form() -> Vec { + scripted_form_with("function times\\(a, b\\) { console.println('times'); return a * b; }") +} + +/// [`scripted_form`] with `helpers` (PDF string syntax) as its document-level script. +fn scripted_form_with(helpers: &str) -> Vec { let text = |name: &str, y: u32, aa: &str| { format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T ({name}) /Rect [20 {y} 180 {}] /P 3 0 R /F 4 {aa} >>", y + 20) }; @@ -4028,7 +4033,7 @@ fn scripted_form() -> Vec { "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 65536 /T (go) /Rect [20 150 80 170] /P 3 0 R /F 4 >>".to_string(), "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), "<< >>".to_string(), - "<< /S /JavaScript /JS (function times\\(a, b\\) { console.println('times'); return a * b; }) >>".to_string(), + format!("<< /S /JavaScript /JS ({helpers}) >>"), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new(); @@ -4090,6 +4095,27 @@ fn scripts_in_an_xfa_form_see_the_xfa_plugin_installed() { assert_eq!((o.error, o.result.as_deref()), (None, Some("undefined number 3.5"))); } +/// #3940: a document-level script that defines the helper and then calls a method PdfCraft lacks +/// (an Acrobat form copying a button icon at open) left every calculated field blank. The +/// helper stays defined, the form calculates, and the error is reported once. +#[test] +fn a_document_script_that_throws_still_lets_the_form_calculate() { + let helpers = + "function times\\(a, b\\) { return a * b; } this.getField\\('go'\\).buttonSetIcon\\(this.getField\\('go'\\).buttonGetIcon\\(\\)\\);"; + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("order.pdf", None, Arc::new(scripted_form_with(helpers)), None).unwrap(); + let _ = s.take_js_output(id); + s.apply(id, Edit::SetFieldValue { name: "qty".into(), value: FieldValue::Text("4".into()) }).unwrap(); + let total = s.get(id).unwrap().form.iter().find(|f| f.name == "total").unwrap().value.clone(); + assert_eq!(total, ["10"], "calculated through the helper the failing script defined"); + let out = s.take_js_output(id); + assert_eq!(out.errors.len(), 1, "reported once, not once per script: {out:?}"); + assert!(out.errors[0].starts_with("document script 1: TypeError"), "{out:?}"); + // The console runs too, and says which document script failed. + let o = s.run_javascript(id, "times(2, 3)", None).unwrap(); + assert_eq!((o.error, o.result.as_deref(), o.doc_errors.len()), (None, Some("6"), 1)); +} + #[test] fn edit_history_does_not_replay_field_script_side_effects() { let mut s = Session::new().with_clock(|| 1_700_000_000); diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 13bdc4ba3..4a2c2f666 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -181,6 +181,9 @@ pub struct Outcome { pub requests: Vec, /// The script's error (syntax error, exception, or a limit reached), if it failed. pub error: Option, + /// Errors in the document-level scripts that ran first. Each failing one stops where it + /// failed; the others and the script itself still run, as in Acrobat (#3940). + pub doc_errors: Vec, /// The script's completion value (what the console prints), unless undefined. pub result: Option, } @@ -1281,8 +1284,13 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc Date: Thu, 22 Oct 2026 14:27:13 +0800 Subject: [PATCH 2/6] M6: js_run reports the document scripts that failed (#3940) Co-Authored-By: Claude Opus 5.5 --- crates/automation/tests/it/automation.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 778deaaec..1a5709c18 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -5845,6 +5845,12 @@ fn javascript_through_tools() { assert_eq!(total["value"], "3750"); let r = ok(&mut a, "js_run", json!({ "doc": doc, "script": "nope()" })); assert!(r["error"].as_str().unwrap().contains("nope")); + assert_eq!(r["document_errors"], json!([])); + // A document script that throws (#3940) is reported, and the others and the script still run. + ok(&mut a, "js_set_document_script", json!({ "doc": doc, "name": "zz", "script": "getField('Total').buttonSetIcon(null);" })); + let r = ok(&mut a, "js_run", json!({ "doc": doc, "script": "money(5)" })); + assert_eq!((r["error"].clone(), r["result"].clone()), (json!(null), json!("EUR 5,00"))); + assert!(r["document_errors"][0].as_str().unwrap().starts_with("document script 2: TypeError"), "{r}"); assert_eq!(ok(&mut a, "js_enabled", json!({ "enabled": false }))["enabled"], false); assert!(a.call("js_run", &json!({ "doc": doc, "script": "1" })).is_err()); } From 77db1bd27faf87792358e089b7ff437b09a0e2c3 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:33:26 +0800 Subject: [PATCH 3/6] M6: parity note for document scripts that throw (#3940) Co-Authored-By: Claude Opus 5.5 --- crates/js/src/lib.rs | 4 ++-- parity/acrobat-features.toml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 4a2c2f666..81633d597 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -181,8 +181,8 @@ pub struct Outcome { pub requests: Vec, /// The script's error (syntax error, exception, or a limit reached), if it failed. pub error: Option, - /// Errors in the document-level scripts that ran first. Each failing one stops where it - /// failed; the others and the script itself still run, as in Acrobat (#3940). + /// Errors in the document-level scripts that ran first. Each is a script of its own: one + /// that throws stops where it failed, and the others and the script itself still run (#3940). pub doc_errors: Vec, /// The script's completion value (what the console prints), unless undefined. pub result: Option, diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 973e0cc69..f416435ae 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5141,8 +5141,8 @@ milestone = "M6" status = "shipped" commands = ["tools.document_js"] tools = ["js_document_scripts", "js_set_document_script"] -evidence = ["crates/ui-egui/tests/it/javascript.rs::document_scripts_and_preferences", "crates/automation/tests/it/automation.rs::javascript_through_tools", "crates/engine/src/tests.rs::form_javascript_validates_calculates_and_formats"] -notes = "Names /JavaScript tree: list, add, edit, delete; functions are available to field scripts." +evidence = ["crates/ui-egui/tests/it/javascript.rs::document_scripts_and_preferences", "crates/automation/tests/it/automation.rs::javascript_through_tools", "crates/engine/src/tests.rs::form_javascript_validates_calculates_and_formats", "crates/engine/src/tests.rs::a_document_script_that_throws_still_lets_the_form_calculate"] +notes = "Names /JavaScript tree: list, add, edit, delete; functions are available to field scripts. A script that throws (calling an Acrobat method PdfCraft lacks, say) stops where it failed, but the others and the field scripts still run and the error is reported once (js_run's document_errors, the console) (#3940). Not like Acrobat: the scripts run again before every field script instead of once when the document opens, so what they do at top level repeats." [[feature]] id = "form.document-actions" From a8beb90ccfb4bb51e652dfa2515b663a683660c9 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:58:37 +0800 Subject: [PATCH 4/6] M6: a document script refused before running is skipped and reported too (#3940) CodeRabbit on #3985: a document script over the size or nesting limit made `run` return before any script ran, so the field script failed as before. It is now skipped (never evaluated, as the limits require), reported in its place in doc_errors, and the other scripts and the field script still run. The field script's own limits still refuse it outright. Co-Authored-By: Claude Opus 5.5 --- crates/js/src/lib.rs | 45 ++++++++++++++++++++++++++++++++++++------ crates/js/src/tests.rs | 33 +++++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 81633d597..f6d5a076c 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -1207,16 +1207,31 @@ pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], do /// 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) { + if let Some(why) = refuse(script) { return failed(why); } + // A document script too long or too deeply nested to run safely is skipped and reported, + // as one that throws is; the others and the script still run (#3940). + let mut skipped = Vec::new(); + let kept: Vec<(usize, &str)> = doc_scripts + .iter() + .enumerate() + .filter_map(|(i, d)| match refuse(d) { + Some(why) => { + skipped.push((i, why)); + None + } + None => Some((i, d.as_str())), + }) + .collect(); + let (kept, skipped) = (&kept, &skipped); #[cfg(not(target_arch = "wasm32"))] { std::thread::scope(|scope| { let spawned = std::thread::Builder::new() .name("pdfcraft-js".into()) .stack_size(SCRIPT_STACK) - .spawn_scoped(scope, || run_here(script, event, doc, fields, doc_scripts, limits)); + .spawn_scoped(scope, || run_here(script, event, doc, fields, kept, skipped, limits)); match spawned { Ok(thread) => thread.join().unwrap_or_else(|_| failed("the script stopped with an internal error".into())), Err(e) => failed(format!("the script engine could not start: {e}")), @@ -1224,7 +1239,7 @@ pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { +/// How [`Outcome::doc_errors`] names a failing document script: by its 1-based place. +fn doc_error(index: usize, error: &dyn std::fmt::Display) -> String { + format!("document script {}: {error}", index.saturating_add(1)) +} + +/// `doc_scripts` are the document scripts to run with their places among all of them; +/// `skipped` the ones refused before running, with the reason. +fn run_here( + script: &str, + event: &Event, + doc: &DocInfo, + fields: Arc>, + doc_scripts: &[(usize, &str)], + skipped: &[(usize, String)], + limits: Limits, +) -> Outcome { let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(NoCodeFromStrings)).build() { Ok(ctx) => ctx, Err(e) => { @@ -1280,6 +1310,7 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc = skipped.iter().map(|(i, why)| (*i, doc_error(*i, why))).collect(); let result = (|| -> JsResult<()> { install(&mut ctx)?; let ev = event_object(&mut ctx, event); @@ -1287,9 +1318,9 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc() { let h = h.into_inner(); out.changed = h.changed.iter().map(|i| h.fields[*i].clone()).collect(); diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index aec0631fd..7124af144 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -325,6 +325,33 @@ fn a_failing_document_script_does_not_stop_the_field_script() { assert!(go("event.value = 1;", &Event::field("Calculate", "total", "")).doc_errors.is_empty()); } +/// A document script refused before running (too long, or nested too deeply to run safely) is +/// skipped like one that throws: never run, reported in its place, the rest still run. +#[test] +fn a_document_script_too_big_to_run_is_skipped_and_reported() { + let docs = [ + "this.getField('qty').buttonSetIcon(null);".to_string(), + format!("var big = 1{};", "+1".repeat(MAX_SCRIPT_BYTES / 2)), + "function double(x) { return x * 2; }".to_string(), + format!("var deep = {}1{};", "(".repeat(10_000), ")".repeat(10_000)), + ]; + let o = run( + "event.value = double(getField('qty').value) + typeof big;", + &Event::field("Calculate", "total", ""), + &doc(), + &fields(), + &docs, + Limits::default(), + ); + assert_eq!((o.error.as_deref(), o.value.as_str()), (None, "8undefined")); + let places: Vec<&str> = o.doc_errors.iter().map(|e| e.split(':').next().unwrap_or_default()).collect(); + assert_eq!(places, ["document script 1", "document script 2", "document script 4"], "{:?}", o.doc_errors); + assert!(o.doc_errors[1].contains("too long") && o.doc_errors[2].contains("nested too deeply"), "{:?}", o.doc_errors); + // The field script's own limits still refuse it outright. + let o = run(&format!("1{}", "+1".repeat(MAX_SCRIPT_BYTES)), &Event::field("Calculate", "total", ""), &doc(), &fields(), &docs, Limits::default()); + assert!(o.error.is_some_and(|e| e.contains("too long")) && o.doc_errors.is_empty()); +} + #[test] fn errors_and_runaway_scripts_are_contained() { let o = go("event.value = nosuch.thing;", &Event::field("Calculate", "total", "keep")); @@ -364,9 +391,11 @@ fn deeply_nested_scripts_fail_instead_of_crashing() { let o = go(s, &Event::doc("Open")); assert!(o.error.is_some(), "{}…", &s[..20]); } - // A hostile document-level script is refused too, before the field script runs. + // A hostile document-level script is refused too: it never runs, and is reported, while + // the field script still does (#3940). let o = run("event.value = 'ran';", &Event::field("Calculate", "a", ""), &doc(), &fields(), &[hostile[0].clone()], Limits::default()); - assert!(o.error.is_some()); + assert_eq!((o.error, o.value.as_str()), (None, "ran")); + assert!(o.doc_errors.len() == 1 && o.doc_errors[0].contains("nested too deeply"), "{:?}", o.doc_errors); } #[test] From a7afaacbb9d313209e45ee9a527bdbcc6d0c3e60 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 23 Oct 2026 07:49:43 +0800 Subject: [PATCH 5/6] M6: merged script output reports a document script's error once too (#3940) Review on #3985 (benandez): JsOutput::absorb reported a failing document script once, but append added every error again, so merging two outputs repeated it. pdfcraft_js now says whether an error is a document script's (is_doc_error, next to the doc_error format), and append skips one already reported; each field script error is still its own. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/js.rs | 27 ++++++++++++++++++++++++++- crates/js/src/lib.rs | 14 +++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index 28e593531..301b0592f 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -44,7 +44,13 @@ impl JsOutput { self.alerts.extend(other.alerts); self.console.extend(other.console); self.requests.extend(other.requests); - self.errors.extend(other.errors); + // As in `absorb`: a document script's error, already reported, is not repeated; a field + // script's errors are each their own. + for e in other.errors { + if !(pdfcraft_js::is_doc_error(&e) && self.errors.contains(&e)) { + self.errors.push(e); + } + } } } @@ -455,6 +461,25 @@ mod review_tests { assert!(read <= 4 * n, "{read} characters read for {n} glyphs"); } + /// Review on #3985: merging two outputs reports a failing document script once, as one run + /// does, while each field script error stays. + #[test] + fn merged_output_reports_a_document_script_error_once() { + let run = |errors: &[&str]| JsOutput { errors: errors.iter().map(|e| e.to_string()).collect(), ..JsOutput::default() }; + let mut out = run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"]); + out.append(run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"])); + assert_eq!( + out.errors, + ["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined", "ReferenceError: x is not defined"] + ); + // A field script's message that only looks like one (no script number) is still its own. + for look_alike in ["document script x: thrown by a field", "document script : thrown by a field"] { + out.append(run(&[look_alike])); + out.append(run(&[look_alike])); + assert_eq!(out.errors.iter().filter(|e| *e == look_alike).count(), 2, "{look_alike}"); + } + } + #[test] fn duplicate_field_names_diff_against_first_original() { let first = pdfcraft_js::FieldState::new("duplicate", pdfcraft_js::FieldType::Text, vec!["first".into()]); diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index f6d5a076c..cc835c79e 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -1274,9 +1274,21 @@ impl HostHooks for CheckedCodeFromStrings { } } +/// The prefix of a document script's error ([`doc_error`]). +const DOC_ERROR_PREFIX: &str = "document script "; + /// How [`Outcome::doc_errors`] names a failing document script: by its 1-based place. fn doc_error(index: usize, error: &dyn std::fmt::Display) -> String { - format!("document script {}: {error}", index.saturating_add(1)) + format!("{DOC_ERROR_PREFIX}{}: {error}", index.saturating_add(1)) +} + +/// Whether `error` is one of [`Outcome::doc_errors`]: a document script's failure, which repeats +/// each time the document scripts run again, unlike a field script's own error. +pub fn is_doc_error(error: &str) -> bool { + error + .strip_prefix(DOC_ERROR_PREFIX) + .and_then(|rest| rest.split_once(": ")) + .is_some_and(|(n, _)| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) } /// `doc_scripts` are the document scripts to run with their places among all of them; From 994ce630997b79aa1084f03307165e987b67d109 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:31:13 +0800 Subject: [PATCH 6/6] M6: a document script's error is told apart by where it came from, not its wording (#3940) Review follow-up on #3985: JsOutput deduplicated document-script errors by their text (document script N: ...), so a field script that threw the same words was dropped by append, and a document script's error was dropped by absorb when a field script had already reported that text. JsOutput now records which errors came from document scripts (doc_errors) and deduplicates only those; pdfcraft_js::is_doc_error, which guessed from the wording, is removed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC --- crates/engine/src/js.rs | 54 ++++++++++++++++++++++++------------- crates/js/src/lib.rs | 14 +--------- crates/ui-egui/src/js_ui.rs | 10 +++++-- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index 301b0592f..19b6c8cde 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -19,6 +19,10 @@ pub struct JsOutput { pub requests: Vec, /// Script errors (as Acrobat's console shows them). pub errors: Vec, + /// Which of `errors` are document scripts' (#3940), told apart by where they came from and + /// not by their text, which a field script's error can share: each is reported once, however + /// often the document scripts run again. + pub doc_errors: Vec, } impl JsOutput { @@ -34,7 +38,8 @@ impl JsOutput { // The document scripts run before every field script, so a failing one would repeat // once per calculation; it is reported once. for e in &o.doc_errors { - if !self.errors.contains(e) { + if !self.doc_errors.contains(e) { + self.doc_errors.push(e.clone()); self.errors.push(e.clone()); } } @@ -45,11 +50,17 @@ impl JsOutput { self.console.extend(other.console); self.requests.extend(other.requests); // As in `absorb`: a document script's error, already reported, is not repeated; a field - // script's errors are each their own. + // script's errors are each their own, even one worded like a document script's. + let mut docs = other.doc_errors; for e in other.errors { - if !(pdfcraft_js::is_doc_error(&e) && self.errors.contains(&e)) { - self.errors.push(e); + if let Some(k) = docs.iter().position(|d| *d == e) { + docs.swap_remove(k); + if self.doc_errors.contains(&e) { + continue; + } + self.doc_errors.push(e.clone()); } + self.errors.push(e); } } } @@ -462,22 +473,29 @@ mod review_tests { } /// Review on #3985: merging two outputs reports a failing document script once, as one run - /// does, while each field script error stays. + /// does, while each field script error stays: told apart by where an error came from, so a + /// field script that throws a document script's very words is still reported. #[test] fn merged_output_reports_a_document_script_error_once() { - let run = |errors: &[&str]| JsOutput { errors: errors.iter().map(|e| e.to_string()).collect(), ..JsOutput::default() }; - let mut out = run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"]); - out.append(run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"])); - assert_eq!( - out.errors, - ["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined", "ReferenceError: x is not defined"] - ); - // A field script's message that only looks like one (no script number) is still its own. - for look_alike in ["document script x: thrown by a field", "document script : thrown by a field"] { - out.append(run(&[look_alike])); - out.append(run(&[look_alike])); - assert_eq!(out.errors.iter().filter(|e| *e == look_alike).count(), 2, "{look_alike}"); - } + let doc = "document script 2: TypeError: not a callable function"; + let field = "ReferenceError: x is not defined"; + let run = |error: &str| { + let mut out = JsOutput::default(); + out.absorb(&Outcome { error: Some(error.into()), doc_errors: vec![doc.into()], ..Outcome::default() }); + out + }; + let mut out = run(field); + out.append(run(field)); + assert_eq!(out.errors, [field, doc, field]); + // CodeRabbit on #3985: a field script that throws the document script's text. + out.append(run(doc)); + assert_eq!(out.errors, [field, doc, field, doc]); + let alone = run(doc); + assert_eq!(alone.errors, [doc, doc], "the field's error and the document script's"); + let count = |o: &JsOutput| o.errors.iter().filter(|e| *e == doc).count(); + let mut merged = run(field); + merged.append(alone); + assert_eq!(count(&merged), 2, "the field's copy stays; the document script's is listed once"); } #[test] diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index cc835c79e..f6d5a076c 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -1274,21 +1274,9 @@ impl HostHooks for CheckedCodeFromStrings { } } -/// The prefix of a document script's error ([`doc_error`]). -const DOC_ERROR_PREFIX: &str = "document script "; - /// How [`Outcome::doc_errors`] names a failing document script: by its 1-based place. fn doc_error(index: usize, error: &dyn std::fmt::Display) -> String { - format!("{DOC_ERROR_PREFIX}{}: {error}", index.saturating_add(1)) -} - -/// Whether `error` is one of [`Outcome::doc_errors`]: a document script's failure, which repeats -/// each time the document scripts run again, unlike a field script's own error. -pub fn is_doc_error(error: &str) -> bool { - error - .strip_prefix(DOC_ERROR_PREFIX) - .and_then(|rest| rest.split_once(": ")) - .is_some_and(|(n, _)| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) + format!("document script {}: {error}", index.saturating_add(1)) } /// `doc_scripts` are the document scripts to run with their places among all of them; diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index 7bc3dd044..f5ec01625 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -88,7 +88,13 @@ impl PdfCraftApp { { self.views[i].document_changed(&info); } - let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: o.error.into_iter().collect() }; + let out = JsOutput { + alerts: o.alerts, + console: o.console, + requests: o.requests, + errors: o.error.into_iter().collect(), + doc_errors: Vec::new(), + }; self.handle_js(id, out); } Err(e) => self.notify_fmt("{field}: {e}", &[("field", field), ("e", &e.to_string())]), @@ -128,7 +134,7 @@ impl PdfCraftApp { self.views[i].document_changed(&info); } let (error, result) = (o.error.clone(), o.result.clone()); - let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: Vec::new() }; + let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: Vec::new(), doc_errors: Vec::new() }; self.handle_js(id, out); match error { Some(e) => self.js_console.log.push(format!("Error: {e}")),