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