Skip to content

Commit

Permalink
Fix borrow_mut() panic in accesskit code when accesskit requests sett…
Browse files Browse the repository at this point in the history
…ing focus to an item

... which in turn requires a tree update with the new focus node, which requires &mut self.

Fixes #6922
  • Loading branch information
tronical committed Nov 27, 2024
1 parent 35f1909 commit 0d7f4ec
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 16 deletions.
50 changes: 36 additions & 14 deletions internal/backends/winit/accesskit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use accesskit::{Action, ActionRequest, Node, NodeId, Role, Toggled, Tree, TreeUp
use i_slint_core::accessibility::{
AccessibilityAction, AccessibleStringProperty, SupportedAccessibilityAction,
};
use i_slint_core::api::Window;
use i_slint_core::item_tree::{ItemTreeRc, ItemTreeRef, ItemTreeWeak};
use i_slint_core::items::{ItemRc, WindowItem};
use i_slint_core::lengths::ScaleFactor;
Expand Down Expand Up @@ -81,7 +82,10 @@ impl AccessKitAdapter {
self.inner.process_event(window, event);
}

pub fn process_accesskit_event(&mut self, window_event: accesskit_winit::WindowEvent) {
pub fn process_accesskit_event(
&mut self,
window_event: accesskit_winit::WindowEvent,
) -> Option<DeferredAccessKitAction> {
match window_event {
accesskit_winit::WindowEvent::InitialTreeRequested => {
self.inner.update_if_active(|| {
Expand All @@ -90,9 +94,10 @@ impl AccessKitAdapter {
self.global_property_tracker.as_ref(),
)
});
None
}
accesskit_winit::WindowEvent::ActionRequested(r) => self.handle_request(r),
accesskit_winit::WindowEvent::AccessibilityDeactivated => (),
accesskit_winit::WindowEvent::AccessibilityDeactivated => None,
}
}

Expand All @@ -104,20 +109,19 @@ impl AccessKitAdapter {
})
}

fn handle_request(&self, request: ActionRequest) {
let Some(window_adapter) = self.window_adapter_weak.upgrade() else { return };
fn handle_request(&self, request: ActionRequest) -> Option<DeferredAccessKitAction> {
let a = match request.action {
Action::Click => AccessibilityAction::Default,
Action::Focus => {
if let Some(item) = self.nodes.item_rc_for_node_id(request.target) {
WindowInner::from_pub(window_adapter.window()).set_focus_item(&item, true);
}
return;
return self
.nodes
.item_rc_for_node_id(request.target)
.map(DeferredAccessKitAction::SetFocus);
}
Action::Decrement => AccessibilityAction::Decrement,
Action::Increment => AccessibilityAction::Increment,
Action::ReplaceSelectedText => {
let Some(accesskit::ActionData::Value(v)) = request.data else { return };
let Some(accesskit::ActionData::Value(v)) = request.data else { return None };
AccessibilityAction::ReplaceSelectedText(SharedString::from(&*v))
}
Action::SetValue => match request.data.unwrap() {
Expand All @@ -127,13 +131,13 @@ impl AccessKitAdapter {
accesskit::ActionData::NumericValue(v) => {
AccessibilityAction::SetValue(i_slint_core::format!("{v}"))
}
_ => return,
_ => return None,
},
_ => return,
_ => return None,
};
if let Some(item) = self.nodes.item_rc_for_node_id(request.target) {
item.accessible_action(&a);
}
self.nodes
.item_rc_for_node_id(request.target)
.map(|item| DeferredAccessKitAction::InvokeAccessibleAction(item, a))
}

pub fn reload_tree(&mut self) {
Expand Down Expand Up @@ -555,3 +559,21 @@ impl From<accesskit_winit::Event> for SlintUserEvent {
SlintUserEvent(crate::event_loop::CustomEvent::Accesskit(value))
}
}

pub enum DeferredAccessKitAction {
SetFocus(ItemRc),
InvokeAccessibleAction(ItemRc, AccessibilityAction),
}

impl DeferredAccessKitAction {
pub fn invoke(&self, window: &Window) {
match self {
DeferredAccessKitAction::SetFocus(item) => {
WindowInner::from_pub(window).set_focus_item(item, true);
}
DeferredAccessKitAction::InvokeAccessibleAction(item, accessibility_action) => {
item.accessible_action(accessibility_action);
}
}
}
}
8 changes: 6 additions & 2 deletions internal/backends/winit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,12 +551,16 @@ impl winit::application::ApplicationHandler<SlintUserEvent> for EventLoopState {
#[cfg(enable_accesskit)]
CustomEvent::Accesskit(accesskit_winit::Event { window_id, window_event }) => {
if let Some(window) = window_by_id(window_id) {
window
let deferred_action = window
.accesskit_adapter()
.expect("internal error: accesskit adapter must exist when window exists")
.borrow_mut()
.process_accesskit_event(window_event);
};
// access kit adapter not borrowed anymore, now invoke the deferred action
if let Some(deferred_action) = deferred_action {
deferred_action.invoke(&window.window());
}
}
}
#[cfg(target_arch = "wasm32")]
CustomEvent::WakeEventLoopWorkaround => {
Expand Down

0 comments on commit 0d7f4ec

Please sign in to comment.