From ee081d033e48e6dcf386afd818fece4f39297d21 Mon Sep 17 00:00:00 2001 From: FredApps Date: Wed, 11 Nov 2026 19:48:17 +0100 Subject: [PATCH] fix(scripts): keep timers past a refused script; page actions before timers A script refused before it runs (too long, or nested too deeply) left the realm untouched but reported no timers. The viewer keeps only the timers a run reports, so a button or console script over the limit stopped every page timer. The realm now reports the timers it still has on that path. The viewer built the page actions' run and the timers' run before it handled either, so a due timer ran before what the page's open action asked for: on a page that goes on to another, before that page's close action could stop it. The timers now run after the page actions, and only if those left the page, the tab and the dialogs as they were; otherwise they wait a frame. The focused field's editor text is taken before each run, not once for both. Tests: a_script_refused_before_it_runs_reports_the_realms_timers (js), a_script_refused_before_it_runs_keeps_the_pages_timers (engine), timers_wait_for_the_page_a_page_action_goes_to (ui). Each fails without its fix. --- crates/engine/src/live_scripts_tests.rs | 12 +++++++++ crates/js/src/session.rs | 15 ++++++++++- crates/ui-egui/src/workflows/js_ui.rs | 17 +++++++++---- crates/ui-egui/tests/it/view.rs | 34 +++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/live_scripts_tests.rs b/crates/engine/src/live_scripts_tests.rs index bee016b19b..2e92ec8ee7 100644 --- a/crates/engine/src/live_scripts_tests.rs +++ b/crates/engine/src/live_scripts_tests.rs @@ -289,3 +289,15 @@ fn a_cap_of_no_timer_runs_only_moves_the_clock() { assert_eq!((s.script_clock(id), s.next_timer(id)), (Some(100.0), Some(100.0))); assert_eq!(s.run_timers_at_most(id, 100.0, 1).ran, 1); } + +/// A script refused before it runs (too long) leaves the page's timers running. +#[test] +fn a_script_refused_before_it_runs_keeps_the_pages_timers() { + let mut s = Session::new(); + let id = s.open("t.pdf", None, Arc::new(timer_pdf("app.setInterval('1', 10);")), None).unwrap(); + s.show_page(id, 0, 0.0); + assert_eq!(s.next_timer(id), Some(10.0)); + let refused = s.run_javascript(id, &"1;".repeat(256 * 1024), None).expect("runs"); + assert!(refused.error.is_some(), "the script is refused"); + assert_eq!(s.next_timer(id), Some(10.0), "and the interval still runs"); +} diff --git a/crates/js/src/session.rs b/crates/js/src/session.rs index a97b84f370..8235706d3d 100644 --- a/crates/js/src/session.rs +++ b/crates/js/src/session.rs @@ -137,7 +137,8 @@ impl Realm { return self.fire(job, id); } // A script too long or too deeply nested to run is that script's error, as on `main`: the - // value stays and the edit's other scripts still run. The realm is not touched. + // value stays and the edit's other scripts still run. The realm is not touched, so its + // timers are as they were: they are reported, as after any run, or the viewer drops them. if let Some(why) = crate::refuse(&job.script) { return Outcome { rc: true, @@ -145,6 +146,7 @@ impl Realm { value: job.event.value.clone(), change: job.event.change.clone(), error: Some(why), + timers: self.context.as_ref().map(crate::timers::report).unwrap_or_default(), ..Default::default() }; } @@ -679,6 +681,17 @@ mod tests { assert!(gone.timers.is_empty() && gone.changed.is_empty(), "{gone:?}"); } #[test] + /// A script refused before it runs (too long) leaves the realm as it was and reports the + /// timers it still has: the viewer keeps only the timers a run reports. + fn a_script_refused_before_it_runs_reports_the_realms_timers() { + let mut s = ScriptSession::default(); + let set = s.run(job("app.setInterval('1', 40);"), &[]); + assert_eq!(set.timers.len(), 1); + let refused = s.run(job(&"1;".repeat(crate::MAX_SCRIPT_BYTES)), &[]); + assert!(refused.realm_untouched && !refused.execution_refused && refused.error.is_some(), "{:?}", refused.error); + assert_eq!(refused.timers, set.timers); + } + #[test] /// Retained Field identity, custom properties and keypad buffers persist locally while separate documents stay isolated. fn document_realm_keeps_keypad_state_and_isolates_documents() { let mut a = ScriptSession::default(); diff --git a/crates/ui-egui/src/workflows/js_ui.rs b/crates/ui-egui/src/workflows/js_ui.rs index c70fc4cc62..a8d28d7c3a 100644 --- a/crates/ui-egui/src/workflows/js_ui.rs +++ b/crates/ui-egui/src/workflows/js_ui.rs @@ -213,13 +213,20 @@ impl PdfCraftApp { let Some((i, id)) = self.active_ids() else { return }; let now_ms = ctx.input(|input| input.time) * 1000.0; let page = self.views[i].current; - // The field being edited as it was, so a timer changing it shows in its editor while a - // draft the user already typed into is kept (as `run_field_triggers` does). - let before: Option<(String, Vec)> = - self.views[i].forms.focus.as_ref().and_then(|e| { + // The page's actions, then its timers, if what the actions asked for left the viewer as it + // was: a page they went to runs its close and open actions (next frame) before any timer, + // and a dialog they opened holds the timers too. + for timers in [false, true] { + if timers && (self.modal_open() || self.active_ids() != Some((i, id)) || self.views[i].current != page) { + ctx.request_repaint(); + break; + } + // The field being edited as it was, so a script changing it shows in its editor while + // a draft the user already typed into is kept (as `run_field_triggers` does). + let before: Option<(String, Vec)> = self.views[i].forms.focus.as_ref().and_then(|e| { crate::forms_ui::focused_field(e, &self.session.get(id)?.form).map(|f| (crate::forms_ui::draft_text(f), f.value.clone())) }); - for run in [self.session.show_page(id, page, now_ms), self.session.run_timers(id, now_ms)] { + let run = if timers { self.session.run_timers(id, now_ms) } else { self.session.show_page(id, page, now_ms) }; if run.is_empty() { continue; } diff --git a/crates/ui-egui/tests/it/view.rs b/crates/ui-egui/tests/it/view.rs index e9fee18b00..196818e462 100644 --- a/crates/ui-egui/tests/it/view.rs +++ b/crates/ui-egui/tests/it/view.rs @@ -1953,6 +1953,40 @@ fn page_actions_and_timers_wait_under_a_dialog() { assert!(h.state_mut().session.next_timer(id).is_some(), "and runs once it closes"); } +/// Three pages: the first's open action starts an interval counting `n`; the second's open +/// action goes on to the third, and its close action stops the interval. +const PAGE_HOP: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R 4 0 R 5 0 R] /Count 3 /MediaBox [0 0 300 400] >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /AA << /O << /S /JavaScript /JS (var n = 0; var t = app.setInterval\\('n++', 10\\);) >> >> >> endobj +4 0 obj << /Type /Page /Parent 2 0 R /AA << /O << /S /JavaScript /JS (this.pageNum = 2;) >> /C << /S /JavaScript /JS (app.clearInterval\\(t\\);) >> >> >> endobj +5 0 obj << /Type /Page /Parent 2 0 R >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +/// A page whose open action goes to another page runs no timer before its close action: the +/// interval that close action stops doesn't run in between. +#[test] +fn timers_wait_for_the_page_a_page_action_goes_to() { + 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.open_bytes("hop.pdf", None, PAGE_HOP.to_vec()).expect("opens"); + app + }); + h.run_steps(2); + let id = h.state().views[0].id; + assert!(h.state_mut().session.next_timer(id).is_some(), "the first page's open action set the interval"); + let count = |h: &mut Harness<'static, PdfCraftApp>| h.state_mut().session.run_javascript(id, "n", None).expect("runs").result; + let before = count(&mut h); + std::thread::sleep(std::time::Duration::from_millis(30)); + h.state_mut().views[0].go_to_page(1); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 2, "the second page's open action went on"); + assert_eq!(h.state_mut().session.next_timer(id), None, "and its close action stopped the interval"); + assert_eq!(count(&mut h), before, "which didn't run in between"); +} + /// A text field, with a default appearance for its editor. const FIELD_TIMER: &[u8] = b"%PDF-1.7 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> >> endobj