From 5d950e1c1564399892230da23ce6ed551c293543 Mon Sep 17 00:00:00 2001 From: Zachary Kohnen Date: Thu, 10 Mar 2022 08:13:32 +0100 Subject: [PATCH] [egui_web] Prevent event handlers from running if code has panicked (#1306) Closes: #1290 Fix panic reported by @Titaniumtown See https://github.com/emilk/egui/pull/1306#issuecomment-1060775376 --- egui/src/widgets/plot/mod.rs | 10 +- egui_web/CHANGELOG.md | 1 + egui_web/src/backend.rs | 40 +++- egui_web/src/lib.rs | 406 ++++++++++++++++++----------------- egui_web/src/text_agent.rs | 124 ++++++----- 5 files changed, 324 insertions(+), 257 deletions(-) diff --git a/egui/src/widgets/plot/mod.rs b/egui/src/widgets/plot/mod.rs index c8fcf2c7..1668d190 100644 --- a/egui/src/widgets/plot/mod.rs +++ b/egui/src/widgets/plot/mod.rs @@ -1,6 +1,6 @@ //! Simple plotting library. -use std::{cell::RefCell, ops::RangeInclusive, rc::Rc}; +use std::{cell::Cell, ops::RangeInclusive, rc::Rc}; use crate::*; use epaint::ahash::AHashSet; @@ -92,7 +92,7 @@ impl PlotMemory { pub struct LinkedAxisGroup { pub(crate) link_x: bool, pub(crate) link_y: bool, - pub(crate) bounds: Rc>>, + pub(crate) bounds: Rc>>, } impl LinkedAxisGroup { @@ -100,7 +100,7 @@ impl LinkedAxisGroup { Self { link_x, link_y, - bounds: Rc::new(RefCell::new(None)), + bounds: Rc::new(Cell::new(None)), } } @@ -132,11 +132,11 @@ impl LinkedAxisGroup { } fn get(&self) -> Option { - *self.bounds.borrow() + self.bounds.get() } fn set(&self, bounds: PlotBounds) { - *self.bounds.borrow_mut() = Some(bounds); + self.bounds.set(Some(bounds)); } } diff --git a/egui_web/CHANGELOG.md b/egui_web/CHANGELOG.md index 18752976..79754635 100644 --- a/egui_web/CHANGELOG.md +++ b/egui_web/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to the `egui_web` integration will be noted in this file. ## Unreleased +* egui code will no longer be called after panic ([#1306](https://github.com/emilk/egui/pull/1306)) ## 0.17.0 - 2022-02-22 diff --git a/egui_web/src/backend.rs b/egui_web/src/backend.rs index 56acc266..f3033cac 100644 --- a/egui_web/src/backend.rs +++ b/egui_web/src/backend.rs @@ -359,11 +359,37 @@ pub fn start(canvas_id: &str, app: Box) -> Result Result { - let runner_ref = AppRunnerRef(Arc::new(Mutex::new(app_runner))); - install_canvas_events(&runner_ref)?; - install_document_events(&runner_ref)?; - text_agent::install_text_agent(&runner_ref)?; - repaint_every_ms(&runner_ref, 1000)?; // just in case. TODO: make it a parameter - paint_and_schedule(runner_ref.clone())?; - Ok(runner_ref) + let runner_container = AppRunnerContainer { + runner: Arc::new(Mutex::new(app_runner)), + panicked: Arc::new(AtomicBool::new(false)), + }; + + install_canvas_events(&runner_container)?; + install_document_events(&runner_container)?; + text_agent::install_text_agent(&runner_container)?; + repaint_every_ms(&runner_container, 1000)?; // just in case. TODO: make it a parameter + + paint_and_schedule(&runner_container.runner, runner_container.panicked.clone())?; + + // Disable all event handlers on panic + std::panic::set_hook(Box::new({ + let previous_hook = std::panic::take_hook(); + + let panicked = runner_container.panicked; + + move |panic_info| { + tracing::info_span!("egui_panic_handler").in_scope(|| { + tracing::trace!("setting panicked flag"); + + panicked.store(true, SeqCst); + + tracing::info!("egui disabled all event handlers due to panic"); + }); + + // Propagate panic info to the previously registered panic hook + previous_hook(panic_info); + } + })); + + Ok(runner_container.runner) } diff --git a/egui_web/src/lib.rs b/egui_web/src/lib.rs index 04b767fe..1f271ead 100644 --- a/egui_web/src/lib.rs +++ b/egui_web/src/lib.rs @@ -29,14 +29,16 @@ pub mod webgl2; pub use backend::*; -use egui::mutex::Mutex; +use egui::mutex::{Mutex, MutexGuard}; pub use wasm_bindgen; pub use web_sys; use input::*; pub use painter::Painter; +use web_sys::EventTarget; use std::collections::BTreeMap; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use wasm_bindgen::prelude::*; @@ -302,12 +304,56 @@ pub fn percent_decode(s: &str) -> String { // ---------------------------------------------------------------------------- -#[derive(Clone)] -pub struct AppRunnerRef(Arc>); +pub type AppRunnerRef = Arc>; -fn paint_and_schedule(runner_ref: AppRunnerRef) -> Result<(), JsValue> { +pub struct AppRunnerContainer { + runner: AppRunnerRef, + /// Set to `true` if there is a panic. + /// Used to ignore callbacks after a panic. + panicked: Arc, +} + +impl AppRunnerContainer { + /// Convenience function to reduce boilerplate and ensure that all event handlers + /// are dealt with in the same way + pub fn add_event_listener( + &self, + target: &EventTarget, + event_name: &'static str, + mut closure: impl FnMut(E, MutexGuard<'_, AppRunner>) + 'static, + ) -> Result<(), JsValue> { + use wasm_bindgen::JsCast; + + // Create a JS closure based on the FnMut provided + let closure = Closure::wrap({ + // Clone atomics + let runner_ref = self.runner.clone(); + let panicked = self.panicked.clone(); + + Box::new(move |event: web_sys::Event| { + // Only call the wrapped closure if the egui code has not panicked + if !panicked.load(Ordering::SeqCst) { + // Cast the event to the expected event type + let event = event.unchecked_into::(); + + closure(event, runner_ref.lock()); + } + }) as Box + }); + + // Add the event listener to the target + target.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; + + // Bypass closure drop so that event handler can call the closure + closure.forget(); + + Ok(()) + } +} + +fn paint_and_schedule(runner_ref: &AppRunnerRef, panicked: Arc) -> Result<(), JsValue> { fn paint_if_needed(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { - let mut runner_lock = runner_ref.0.lock(); + let mut runner_lock = runner_ref.lock(); if runner_lock.needs_repaint.fetch_and_clear() { let (needs_repaint, clipped_meshes) = runner_lock.logic()?; runner_lock.paint(clipped_meshes)?; @@ -320,34 +366,40 @@ fn paint_and_schedule(runner_ref: AppRunnerRef) -> Result<(), JsValue> { Ok(()) } - fn request_animation_frame(runner_ref: AppRunnerRef) -> Result<(), JsValue> { + fn request_animation_frame( + runner_ref: AppRunnerRef, + panicked: Arc, + ) -> Result<(), JsValue> { use wasm_bindgen::JsCast; let window = web_sys::window().unwrap(); - let closure = Closure::once(move || paint_and_schedule(runner_ref)); + let closure = Closure::once(move || paint_and_schedule(&runner_ref, panicked)); window.request_animation_frame(closure.as_ref().unchecked_ref())?; closure.forget(); // We must forget it, or else the callback is canceled on drop Ok(()) } - paint_if_needed(&runner_ref)?; - request_animation_frame(runner_ref) + // Only paint and schedule if there has been no panic + if !panicked.load(Ordering::SeqCst) { + paint_if_needed(runner_ref)?; + request_animation_frame(runner_ref.clone(), panicked)?; + } + + Ok(()) } -fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { - use wasm_bindgen::JsCast; +fn install_document_events(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { let window = web_sys::window().unwrap(); let document = window.document().unwrap(); - { - // keydown - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::KeyboardEvent| { + runner_container.add_event_listener( + &document, + "keydown", + |event: web_sys::KeyboardEvent, mut runner_lock| { if event.is_composing() || event.key_code() == 229 { // https://www.fxsitecompat.dev/en-CA/docs/2018/keydown-and-keyup-events-are-now-fired-during-ime-composition/ return; } - let mut runner_lock = runner_ref.0.lock(); let modifiers = modifiers_from_event(&event); runner_lock.input.raw.modifiers = modifiers; @@ -400,16 +452,13 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { if prevent_default { event.prevent_default(); } - }) as Box); - document.add_event_listener_with_callback("keydown", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - // keyup - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::KeyboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "keyup", + |event: web_sys::KeyboardEvent, mut runner_lock| { let modifiers = modifiers_from_event(&event); runner_lock.input.raw.modifiers = modifiers; if let Some(key) = translate_key(&event.key()) { @@ -420,19 +469,16 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { }); } runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("keyup", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // paste - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::ClipboardEvent| { + runner_container.add_event_listener( + &document, + "paste", + |event: web_sys::ClipboardEvent, mut runner_lock| { if let Some(data) = event.clipboard_data() { if let Ok(text) = data.get_data("text") { - let mut runner_lock = runner_ref.0.lock(); runner_lock .input .raw @@ -443,85 +489,90 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.prevent_default(); } } - }) as Box); - document.add_event_listener_with_callback("paste", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // cut - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |_: web_sys::ClipboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "cut", + |_: web_sys::ClipboardEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::Cut); runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("cut", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // copy - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |_: web_sys::ClipboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "copy", + |_: web_sys::ClipboardEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::Copy); runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("copy", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; for event_name in &["load", "pagehide", "pageshow", "resize"] { - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - runner_ref.0.lock().needs_repaint.set_true(); - }) as Box); - window.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); + runner_container.add_event_listener( + &document, + event_name, + |_: web_sys::Event, runner_lock| { + runner_lock.needs_repaint.set_true(); + }, + )?; } - { - // hashchange - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - let runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "hashchange", + |_: web_sys::Event, runner_lock| { let mut frame_lock = runner_lock.frame.lock(); // `epi::Frame::info(&self)` clones `epi::IntegrationInfo`, but we need to modify the original here if let Some(web_info) = &mut frame_lock.info.web_info { web_info.location.hash = location_hash(); } - }) as Box); - window.add_event_listener_with_callback("hashchange", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; Ok(()) } /// Repaint at least every `ms` milliseconds. -fn repaint_every_ms(runner_ref: &AppRunnerRef, milliseconds: i32) -> Result<(), JsValue> { +pub fn repaint_every_ms( + runner_container: &AppRunnerContainer, + milliseconds: i32, +) -> Result<(), JsValue> { assert!(milliseconds >= 0); + use wasm_bindgen::JsCast; + let window = web_sys::window().unwrap(); - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - runner_ref.0.lock().needs_repaint.set_true(); + + let closure = Closure::wrap(Box::new({ + let runner = runner_container.runner.clone(); + let panicked = runner_container.panicked.clone(); + + move || { + // Do not lock the runner if the code has panicked + if !panicked.load(Ordering::SeqCst) { + runner.lock().needs_repaint.set_true(); + } + } }) as Box); + window.set_interval_with_callback_and_timeout_and_arguments_0( closure.as_ref().unchecked_ref(), milliseconds, )?; + closure.forget(); Ok(()) } -fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { +fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { use wasm_bindgen::JsCast; - let canvas = canvas_element(runner_ref.0.lock().canvas_id()).unwrap(); + let canvas = canvas_element(runner_container.runner.lock().canvas_id()).unwrap(); { // By default, right-clicks open a context menu. @@ -534,12 +585,11 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { closure.forget(); } - { - let event_name = "mousedown"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { + runner_container.add_event_listener( + &canvas, + "mousedown", + |event: web_sys::MouseEvent, mut runner_lock| { if let Some(button) = button_from_mouse_event(&event) { - let mut runner_lock = runner_ref.0.lock(); let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); let modifiers = runner_lock.input.raw.modifiers; runner_lock @@ -556,16 +606,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { } event.stop_propagation(); // Note: prevent_default breaks VSCode tab focusing, hence why we don't call it here. - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mousemove"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "mousemove", + |event: web_sys::MouseEvent, mut runner_lock| { let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); runner_lock .input @@ -575,17 +622,14 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mouseup"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { + runner_container.add_event_listener( + &canvas, + "mouseup", + |event: web_sys::MouseEvent, mut runner_lock| { if let Some(button) = button_from_mouse_event(&event) { - let mut runner_lock = runner_ref.0.lock(); let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); let modifiers = runner_lock.input.raw.modifiers; runner_lock @@ -600,34 +644,28 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { }); runner_lock.needs_repaint.set_true(); - text_agent::update_text_agent(&runner_lock); + text_agent::update_text_agent(runner_lock); } event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mouseleave"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "mouseleave", + |event: web_sys::MouseEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::PointerGone); runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "touchstart"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "touchstart", + |event: web_sys::TouchEvent, mut runner_lock| { let mut latest_touch_pos_id = runner_lock.input.latest_touch_pos_id; let pos = pos_from_touch_event(runner_lock.canvas_id(), &event, &mut latest_touch_pos_id); @@ -649,16 +687,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "touchmove"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "touchmove", + |event: web_sys::TouchEvent, mut runner_lock| { let mut latest_touch_pos_id = runner_lock.input.latest_touch_pos_id; let pos = pos_from_touch_event(runner_lock.canvas_id(), &event, &mut latest_touch_pos_id); @@ -674,17 +709,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } - - { - let event_name = "touchend"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + }, + )?; + runner_container.add_event_listener( + &canvas, + "touchend", + |event: web_sys::TouchEvent, mut runner_lock| { if let Some(pos) = runner_lock.input.latest_touch_pos { let modifiers = runner_lock.input.raw.modifiers; // First release mouse to click: @@ -708,34 +739,27 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { } // Finally, focus or blur text agent to toggle mobile keyboard: - text_agent::update_text_agent(&runner_lock); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + text_agent::update_text_agent(runner_lock); + }, + )?; - { - let event_name = "touchcancel"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); - push_touches(&mut *runner_lock, egui::TouchPhase::Cancel, &event); + runner_container.add_event_listener( + &canvas, + "touchcancel", + |event: web_sys::TouchEvent, mut runner_lock| { + push_touches(&mut runner_lock, egui::TouchPhase::Cancel, &event); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } - - { - let event_name = "wheel"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::WheelEvent| { - let mut runner_lock = runner_ref.0.lock(); + }, + )?; + runner_container.add_event_listener( + &canvas, + "wheel", + |event: web_sys::WheelEvent, mut runner_lock| { let scroll_multiplier = match event.delta_mode() { web_sys::WheelEvent::DOM_DELTA_PAGE => { - canvas_size_in_points(runner_ref.0.lock().canvas_id()).y + canvas_size_in_points(runner_lock.canvas_id()).y } web_sys::WheelEvent::DOM_DELTA_LINE => { #[allow(clippy::let_and_return)] @@ -771,17 +795,14 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "dragover"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { + runner_container.add_event_listener( + &canvas, + "dragover", + |event: web_sys::DragEvent, mut runner_lock| { if let Some(data_transfer) = event.data_transfer() { - let mut runner_lock = runner_ref.0.lock(); runner_lock.input.raw.hovered_files.clear(); for i in 0..data_transfer.items().length() { if let Some(item) = data_transfer.items().get(i) { @@ -795,35 +816,29 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.stop_propagation(); event.prevent_default(); } - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "dragleave"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "dragleave", + |event: web_sys::DragEvent, mut runner_lock| { runner_lock.input.raw.hovered_files.clear(); runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "drop"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { + runner_container.add_event_listener(&canvas, "drop", { + let runner_ref = runner_container.runner.clone(); + + move |event: web_sys::DragEvent, mut runner_lock| { if let Some(data_transfer) = event.data_transfer() { - { - let mut runner_lock = runner_ref.0.lock(); - runner_lock.input.raw.hovered_files.clear(); - runner_lock.needs_repaint.set_true(); - } + runner_lock.input.raw.hovered_files.clear(); + runner_lock.needs_repaint.set_true(); + // Unlock the runner so it can be locked after a future await point + drop(runner_lock); if let Some(files) = data_transfer.files() { for i in 0..files.length() { @@ -847,7 +862,8 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { bytes.len() ); - let mut runner_lock = runner_ref.0.lock(); + // Re-lock the mutex on the other side of the await point + let mut runner_lock = runner_ref.lock(); runner_lock.input.raw.dropped_files.push( egui::DroppedFile { name, @@ -870,10 +886,8 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.stop_propagation(); event.prevent_default(); } - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + } + })?; Ok(()) } diff --git a/egui_web/src/text_agent.rs b/egui_web/src/text_agent.rs index 2fbaa1b0..ef2af8d8 100644 --- a/egui_web/src/text_agent.rs +++ b/egui_web/src/text_agent.rs @@ -1,7 +1,8 @@ //! The text agent is an `` element used to trigger //! mobile keyboard and IME input. -use crate::{canvas_element, AppRunner, AppRunnerRef}; +use crate::{canvas_element, AppRunner, AppRunnerContainer}; +use egui::mutex::MutexGuard; use std::cell::Cell; use std::rc::Rc; use wasm_bindgen::prelude::*; @@ -21,7 +22,7 @@ pub fn text_agent() -> web_sys::HtmlInputElement { } /// Text event handler, -pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { +pub fn install_text_agent(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { use wasm_bindgen::JsCast; let window = web_sys::window().unwrap(); let document = window.document().unwrap(); @@ -43,61 +44,73 @@ pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { input.set_size(1); input.set_autofocus(true); input.set_hidden(true); - { - // When IME is off + + // When IME is off + runner_container.add_event_listener(&input, "input", { let input_clone = input.clone(); - let runner_ref = runner_ref.clone(); let is_composing = is_composing.clone(); - let on_input = Closure::wrap(Box::new(move |_event: web_sys::InputEvent| { + + move |_event: web_sys::InputEvent, mut runner_lock| { let text = input_clone.value(); if !text.is_empty() && !is_composing.get() { input_clone.set_value(""); - let mut runner_lock = runner_ref.0.lock(); runner_lock.input.raw.events.push(egui::Event::Text(text)); runner_lock.needs_repaint.set_true(); } - }) as Box); - input.add_event_listener_with_callback("input", on_input.as_ref().unchecked_ref())?; - on_input.forget(); - } + } + })?; + { // When IME is on, handle composition event - let input_clone = input.clone(); - let runner_ref = runner_ref.clone(); - let on_compositionend = Closure::wrap(Box::new(move |event: web_sys::CompositionEvent| { - let mut runner_lock = runner_ref.0.lock(); - let opt_event = match event.type_().as_ref() { - "compositionstart" => { - is_composing.set(true); - input_clone.set_value(""); - Some(egui::Event::CompositionStart) - } - "compositionend" => { - is_composing.set(false); - input_clone.set_value(""); - event.data().map(egui::Event::CompositionEnd) - } - "compositionupdate" => event.data().map(egui::Event::CompositionUpdate), - s => { - tracing::error!("Unknown composition event type: {:?}", s); - None - } - }; - if let Some(event) = opt_event { - runner_lock.input.raw.events.push(event); + runner_container.add_event_listener(&input, "compositionstart", { + let input_clone = input.clone(); + let is_composing = is_composing.clone(); + + move |_event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + is_composing.set(true); + input_clone.set_value(""); + + runner_lock + .input + .raw + .events + .push(egui::Event::CompositionStart); runner_lock.needs_repaint.set_true(); } - }) as Box); - let f = on_compositionend.as_ref().unchecked_ref(); - input.add_event_listener_with_callback("compositionstart", f)?; - input.add_event_listener_with_callback("compositionupdate", f)?; - input.add_event_listener_with_callback("compositionend", f)?; - on_compositionend.forget(); + })?; + + runner_container.add_event_listener( + &input, + "compositionupdate", + move |event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + if let Some(event) = event.data().map(egui::Event::CompositionUpdate) { + runner_lock.input.raw.events.push(event); + runner_lock.needs_repaint.set_true(); + } + }, + )?; + + runner_container.add_event_listener(&input, "compositionend", { + let input_clone = input.clone(); + + move |event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + is_composing.set(false); + input_clone.set_value(""); + + if let Some(event) = event.data().map(egui::Event::CompositionEnd) { + runner_lock.input.raw.events.push(event); + runner_lock.needs_repaint.set_true(); + } + } + })?; } - { - // When input lost focus, focus on it again. - // It is useful when user click somewhere outside canvas. - let on_focusout = Closure::wrap(Box::new(move |_event: web_sys::MouseEvent| { + + // When input lost focus, focus on it again. + // It is useful when user click somewhere outside canvas. + runner_container.add_event_listener( + &input, + "focusout", + move |_event: web_sys::MouseEvent, _| { // Delay 10 ms, and focus again. let func = js_sys::Function::new_no_args(&format!( "document.getElementById('{}').focus()", @@ -106,16 +119,16 @@ pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { window .set_timeout_with_callback_and_timeout_and_arguments_0(&func, 10) .unwrap(); - }) as Box); - input.add_event_listener_with_callback("focusout", on_focusout.as_ref().unchecked_ref())?; - on_focusout.forget(); - } + }, + )?; + body.append_child(&input)?; + Ok(()) } /// Focus or blur text agent to toggle mobile keyboard. -pub fn update_text_agent(runner: &AppRunner) -> Option<()> { +pub fn update_text_agent(runner: MutexGuard<'_, AppRunner>) -> Option<()> { use wasm_bindgen::JsCast; use web_sys::HtmlInputElement; let window = web_sys::window()?; @@ -156,7 +169,20 @@ pub fn update_text_agent(runner: &AppRunner) -> Option<()> { } } } else { + // Drop runner lock + drop(runner); + + // Holding the runner lock while calling input.blur() causes a panic. + // This is most probably caused by the browser running the event handler + // for the triggered blur event synchronously, meaning that the mutex + // lock does not get dropped by the time another event handler is called. + // + // Why this didn't exist before #1290 is a mystery to me, but it exists now + // and this apparently is the fix for it + // + // ¯\_(ツ)_/¯ - @DusterTheFirst input.blur().ok()?; + input.set_hidden(true); canvas_style.set_property("position", "absolute").ok()?; canvas_style.set_property("top", "0%").ok()?; // move back to normal position