Fix/browser autofill crash

packages/core/src/events.rs

@@ -654,7 +654,26 @@ impl<T> ListenerCallback<T> {
     pub fn call(&self, event: Event<dyn Any>) {
         let runtime = Runtime::current().expect("ListenerCallback must be called within a runtime");
         runtime.with_scope_on_stack(self.origin, || {
-            (self.callback.borrow_mut())(event);
+            // Use try_borrow_mut to handle cases where the callback is already borrowed
+            // This provides additional safety for edge cases where events might be nested
+            match self.callback.try_borrow_mut() {
+                Ok(mut callback) => {
+                    callback(event);
+                }
+                Err(_) => {
+                    // If we can't borrow mutably, it means we're in a reentrant call
+                    // This should be very rare now that we've fixed the main borrow issue,
+                    // but we defer the event to be safe
+                    let callback = self.callback.clone();
+                    let event = event.clone();
+
+                    crate::spawn(async move {
+                        if let Ok(mut cb) = callback.try_borrow_mut() {
+                            cb(event);
+                        }
+                    });
+                }
+            }
         });
     }
 

packages/core/src/runtime.rs

@@ -315,9 +315,15 @@ impl Runtime {
     #[instrument(skip(self, event), level = "trace", name = "Runtime::handle_event")]
     pub fn handle_event(self: &Rc<Self>, name: &str, event: Event<dyn Any>, element: ElementId) {
         let _runtime = RuntimeGuard::new(self.clone());
-        let elements = self.elements.borrow();
 
-        if let Some(Some(parent_path)) = elements.get(element.0).copied() {
+        // Collect the parent path information first, then drop the borrow
+        // This prevents BorrowMutError when event handlers modify the DOM
+        let parent_path = {
+            let elements = self.elements.borrow();
+            elements.get(element.0).copied().flatten()
+        };
+
+        if let Some(parent_path) = parent_path {
             if event.propagates() {
                 self.handle_bubbling_event(parent_path, name, event);
             } else {
@@ -353,61 +359,75 @@ impl Runtime {
         name = "VirtualDom::handle_bubbling_event"
     )]
     fn handle_bubbling_event(&self, parent: ElementRef, name: &str, uievent: Event<dyn Any>) {
-        let mounts = self.mounts.borrow();
-
-        // If the event bubbles, we traverse through the tree until we find the target element.
-        // Loop through each dynamic attribute (in a depth first order) in this template before moving up to the template's parent.
-        let mut parent = Some(parent);
-        while let Some(path) = parent {
-            let mut listeners = vec![];
-
-            let Some(mount) = mounts.get(path.mount.0) else {
-                // If the node is suspended and not mounted, we can just ignore the event
-                return;
-            };
-            let el_ref = &mount.node;
-            let node_template = el_ref.template;
-            let target_path = path.path;
-
-            // Accumulate listeners into the listener list bottom to top
-            for (idx, this_path) in node_template.attr_paths.iter().enumerate() {
-                let attrs = &*el_ref.dynamic_attrs[idx];
-
-                for attr in attrs.iter() {
-                    // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
-                    if attr.name.get(2..) == Some(name) && target_path.is_descendant(this_path) {
-                        listeners.push(&attr.value);
-
-                        // Break if this is the exact target element.
-                        // This means we won't call two listeners with the same name on the same element. This should be
-                        // documented, or be rejected from the rsx! macro outright
-                        if target_path == this_path {
-                            break;
+        // First, collect all listeners from the bubble chain without keeping borrows active
+        // This prevents BorrowMutError when event handlers modify the DOM
+        // Cloning AttributeValue::Listener is cheap since it just increments Rc counters
+        let all_listeners = {
+            let mounts = self.mounts.borrow();
+            let mut all_listeners = Vec::new();
+
+            // If the event bubbles, we traverse through the tree until we find the target element.
+            // Loop through each dynamic attribute (in a depth first order) in this template before moving up to the template's parent.
+            let mut parent = Some(parent);
+            while let Some(path) = parent {
+                let mut listeners = vec![];
+
+                let Some(mount) = mounts.get(path.mount.0) else {
+                    // If the node is suspended and not mounted, we can just ignore the event
+                    return;
+                };
+                let el_ref = &mount.node;
+                let node_template = el_ref.template;
+                let target_path = path.path;
+
+                // Accumulate listeners into the listener list bottom to top
+                for (idx, this_path) in node_template.attr_paths.iter().enumerate() {
+                    let attrs = &*el_ref.dynamic_attrs[idx];
+
+                    for attr in attrs.iter() {
+                        // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
+                        if attr.name.get(2..) == Some(name) && target_path.is_descendant(this_path)
+                        {
+                            listeners.push(attr.value.clone());
+
+                            // Break if this is the exact target element.
+                            // This means we won't call two listeners with the same name on the same element. This should be
+                            // documented, or be rejected from the rsx! macro outright
+                            if target_path == this_path {
+                                break;
+                            }
                         }
                     }
                 }
-            }
 
-            // Now that we've accumulated all the parent attributes for the target element, call them in reverse order
-            // We check the bubble state between each call to see if the event has been stopped from bubbling
-            tracing::event!(
-                tracing::Level::TRACE,
-                "Calling {} listeners",
-                listeners.len()
-            );
-            for listener in listeners.into_iter().rev() {
-                if let AttributeValue::Listener(listener) = listener {
-                    listener.call(uievent.clone());
-                    let metadata = uievent.metadata.borrow();
-
-                    if !metadata.propagates {
-                        return;
-                    }
+                // Add listeners in reverse order for this level
+                for listener in listeners.into_iter().rev() {
+                    all_listeners.push(listener);
                 }
+
+                // Move to parent
+                let mount_id = el_ref.mount.get().as_usize();
+                parent = mount_id.and_then(|id| mounts.get(id).and_then(|el| el.parent));
             }
 
-            let mount = el_ref.mount.get().as_usize();
-            parent = mount.and_then(|id| mounts.get(id).and_then(|el| el.parent));
+            all_listeners
+        }; // mounts borrow is dropped here
+
+        // Now call all listeners without any active borrows
+        tracing::event!(
+            tracing::Level::TRACE,
+            "Calling {} listeners",
+            all_listeners.len()
+        );
+        for listener in all_listeners {
+            if let AttributeValue::Listener(listener) = listener {
+                listener.call(uievent.clone());
+                let metadata = uievent.metadata.borrow();
+
+                if !metadata.propagates {
+                    return;
+                }
+            }
         }
     }
 
@@ -418,28 +438,43 @@ impl Runtime {
         name = "VirtualDom::handle_non_bubbling_event"
     )]
     fn handle_non_bubbling_event(&self, node: ElementRef, name: &str, uievent: Event<dyn Any>) {
-        let mounts = self.mounts.borrow();
-        let Some(mount) = mounts.get(node.mount.0) else {
-            // If the node is suspended and not mounted, we can just ignore the event
-            return;
-        };
-        let el_ref = &mount.node;
-        let node_template = el_ref.template;
-        let target_path = node.path;
-
-        for (idx, this_path) in node_template.attr_paths.iter().enumerate() {
-            let attrs = &*el_ref.dynamic_attrs[idx];
-
-            for attr in attrs.iter() {
-                // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
-                // Only call the listener if this is the exact target element.
-                if attr.name.get(2..) == Some(name) && target_path == this_path {
-                    if let AttributeValue::Listener(listener) = &attr.value {
-                        listener.call(uievent.clone());
-                        break;
+        // First, collect the listener without keeping borrow active
+        // This prevents BorrowMutError when event handlers modify the DOM
+        // Cloning AttributeValue::Listener is cheap since it just increments Rc counters
+        let listener = {
+            let mounts = self.mounts.borrow();
+            let Some(mount) = mounts.get(node.mount.0) else {
+                // If the node is suspended and not mounted, we can just ignore the event
+                return;
+            };
+            let el_ref = &mount.node;
+            let node_template = el_ref.template;
+            let target_path = node.path;
+
+            let mut found_listener = None;
+            for (idx, this_path) in node_template.attr_paths.iter().enumerate() {
+                let attrs = &*el_ref.dynamic_attrs[idx];
+
+                for attr in attrs.iter() {
+                    // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
+                    // Only call the listener if this is the exact target element.
+                    if attr.name.get(2..) == Some(name) && target_path == this_path {
+                        if let AttributeValue::Listener(listener) = &attr.value {
+                            found_listener = Some(listener.clone());
+                            break;
+                        }
                     }
                 }
+                if found_listener.is_some() {
+                    break;
+                }
             }
+            found_listener
+        }; // mounts borrow is dropped here
+
+        // Now call the listener without any active borrows
+        if let Some(listener) = listener {
+            listener.call(uievent);
         }
     }
 }

packages/html/src/events/animation.rs

@@ -36,6 +36,12 @@ impl AnimationData {
     }
 }
 
+impl Default for AnimationData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasAnimationData> From<E> for AnimationData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }

packages/html/src/events/clipboard.rs

@@ -6,6 +6,12 @@ pub struct ClipboardData {
     inner: Box<dyn HasClipboardData>,
 }
 
+impl Default for ClipboardData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasClipboardData> From<E> for ClipboardData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }

packages/html/src/events/composition.rs

@@ -14,6 +14,12 @@ impl std::fmt::Debug for CompositionData {
     }
 }
 
+impl Default for CompositionData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasCompositionData> From<E> for CompositionData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }

packages/html/src/events/drag.rs

@@ -17,6 +17,12 @@ pub struct DragData {
     inner: Box<dyn HasDragData>,
 }
 
+impl Default for DragData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasDragData + 'static> From<E> for DragData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }

packages/html/src/events/focus.rs

@@ -6,6 +6,12 @@ pub struct FocusData {
     inner: Box<dyn HasFocusData>,
 }
 
+impl Default for FocusData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasFocusData> From<E> for FocusData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }

packages/html/src/events/keyboard.rs

@@ -19,6 +19,12 @@ pub struct KeyboardData {
     inner: Box<dyn HasKeyboardData>,
 }
 
+impl Default for KeyboardData {
+    fn default() -> Self {
+        Self::new(crate::events::empty::EmptyEvent)
+    }
+}
+
 impl<E: HasKeyboardData> From<E> for KeyboardData {
     fn from(e: E) -> Self {
         Self { inner: Box::new(e) }
@@ -105,9 +111,13 @@ pub struct SerializedKeyboardData {
     key_code: KeyCode,
     #[serde(deserialize_with = "resilient_deserialize_code")]
     code: Code,
+    #[serde(default)]
     alt_key: bool,
+    #[serde(default)]
     ctrl_key: bool,
+    #[serde(default)]
     meta_key: bool,
+    #[serde(default)]
     shift_key: bool,
     location: usize,
     repeat: bool,