From c6cb9ef27b301ab3409fc8bdeff71b6ad0a83887 Mon Sep 17 00:00:00 2001 From: Arnold Loubriat Date: Fri, 23 Oct 2026 17:34:09 +0200 Subject: [PATCH] fix: Fully drop iOS SubclassingAdapter if KVO is detected --- Cargo.lock | 1 + adapters/ios/Cargo.toml | 8 + adapters/ios/src/subclass.rs | 173 +++++++++---- adapters/ios/tests/integration/harness.rs | 38 +++ adapters/ios/tests/integration/main.rs | 12 + .../tests/integration/subclassing_adapter.rs | 230 ++++++++++++++++++ adapters/ios/tests/integration/util.rs | 55 +++++ 7 files changed, 467 insertions(+), 50 deletions(-) create mode 100644 adapters/ios/tests/integration/harness.rs create mode 100644 adapters/ios/tests/integration/main.rs create mode 100644 adapters/ios/tests/integration/subclassing_adapter.rs create mode 100644 adapters/ios/tests/integration/util.rs diff --git a/Cargo.lock b/Cargo.lock index 82b0e8dac..77c766a05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -68,6 +68,7 @@ dependencies = [ "accesskit", "accesskit_consumer", "hashbrown", + "libtest-mimic", "objc2 0.5.2", "objc2-foundation 0.2.2", "objc2-ui-kit", diff --git a/adapters/ios/Cargo.toml b/adapters/ios/Cargo.toml index ae6385c95..0511c3e44 100644 --- a/adapters/ios/Cargo.toml +++ b/adapters/ios/Cargo.toml @@ -41,3 +41,11 @@ objc2-ui-kit = { version = "0.2.0", features = [ "UIViewController", "UIWindow", ] } + +[dev-dependencies] +libtest-mimic = "0.8.2" + +# UIKit must be used from the main thread, where libtest doesn't run tests. +[[test]] +name = "integration" +harness = false diff --git a/adapters/ios/src/subclass.rs b/adapters/ios/src/subclass.rs index e3b0e00f6..055990340 100644 --- a/adapters/ios/src/subclass.rs +++ b/adapters/ios/src/subclass.rs @@ -74,26 +74,55 @@ fn associated_object(view: &UIView) -> Option<&AssociatedObject> { } } +// Unlike `[view class]`, this sees the subclass that key-value observing +// hides by overriding `class`. +fn runtime_class(view: &UIView) -> &'static AnyClass { + let object: &AnyObject = view; + object.class() +} + +// Key-value observing may have subclassed our subclass in turn. +fn installed_subclass(view: &UIView) -> Option<(&'static AnyClass, &'static AnyClass)> { + let subclasses = SUBCLASSES.lock().unwrap(); + let mut class: Option<&AnyClass> = Some(runtime_class(view)); + while let Some(current) = class { + if let Some(entry) = subclasses + .iter() + .find(|entry| std::ptr::eq(entry.1, current)) + { + return Some(*entry); + } + class = current.superclass(); + } + None +} + +fn prev_class(view: &UIView) -> &'static AnyClass { + match associated_object(view) { + Some(associated) => associated.ivars().prev_class, + None => installed_subclass(view).unwrap().0, + } +} + // Some view classes assume that they are the lowest subclass, // and call [self superclass] to get their superclass. // Give them the answer they need. unsafe extern "C" fn superclass(this: &UIView, _cmd: Sel) -> Option<&AnyClass> { - let associated = associated_object(this)?; - associated.ivars().prev_class.superclass() + prev_class(this).superclass() } // UIAccessibilityContainer methods unsafe extern "C" fn is_accessibility_element(this: &UIView, _cmd: Sel) -> Bool { let Some(associated) = associated_object(this) else { - return Bool::YES; + return unsafe { msg_send![super(this, prev_class(this)), isAccessibilityElement] }; }; Bool::new(associated.ivars().adapter.is_accessibility_element()) } unsafe extern "C" fn accessibility_elements(this: &UIView, _cmd: Sel) -> *mut NSArray { let Some(associated) = associated_object(this) else { - return Retained::autorelease_return(NSArray::new()); + return unsafe { msg_send![super(this, prev_class(this)), accessibilityElements] }; }; associated.ivars().adapter.accessibility_elements() } @@ -106,7 +135,14 @@ unsafe extern "C" fn accessibility_hit_test( point: CGPoint, ) -> *mut AnyObject { let Some(associated) = associated_object(this) else { - return null_mut(); + let prev_class = prev_class(this); + if prev_class + .instance_method(sel!(accessibilityHitTest:)) + .is_none() + { + return null_mut(); + } + return unsafe { msg_send![super(this, prev_class), accessibilityHitTest: point] }; }; associated.ivars().adapter.hit_test(point) as *mut AnyObject } @@ -114,12 +150,8 @@ unsafe extern "C" fn accessibility_hit_test( // UIView lifecycle unsafe extern "C" fn did_move_to_window(this: &UIView, _cmd: Sel) { - let Some(associated) = associated_object(this) else { - return; - }; - let prev_class = associated.ivars().prev_class; unsafe { - let _: () = msg_send![super(this, prev_class), didMoveToWindow]; + let _: () = msg_send![super(this, prev_class(this)), didMoveToWindow]; } if this.window().is_none() { @@ -134,6 +166,45 @@ unsafe extern "C" fn did_move_to_window(this: &UIView, _cmd: Sel) { } } +fn subclass_of(prev_class: &'static AnyClass) -> &'static AnyClass { + let mut subclasses = SUBCLASSES.lock().unwrap(); + match subclasses + .iter() + .find(|entry| std::ptr::eq(entry.0, prev_class)) + { + Some(entry) => entry.1, + None => { + let name = format!("AccessKitSubclassOf{}", prev_class.name()); + let mut builder = ClassBuilder::new(&name, prev_class).unwrap(); + unsafe { + builder.add_method( + sel!(superclass), + superclass as unsafe extern "C" fn(_, _) -> _, + ); + builder.add_method( + sel!(isAccessibilityElement), + is_accessibility_element as unsafe extern "C" fn(_, _) -> _, + ); + builder.add_method( + sel!(accessibilityElements), + accessibility_elements as unsafe extern "C" fn(_, _) -> _, + ); + builder.add_method( + sel!(accessibilityHitTest:), + accessibility_hit_test as unsafe extern "C" fn(_, _, _) -> _, + ); + builder.add_method( + sel!(didMoveToWindow), + did_move_to_window as unsafe extern "C" fn(_, _), + ); + } + let class = builder.register(); + subclasses.push((prev_class, class)); + class + } + } +} + /// Uses dynamic Objective-C subclassing to implement the `UIView` /// accessibility methods when normal subclassing isn't an option. pub struct SubclassingAdapter { @@ -144,7 +215,8 @@ pub struct SubclassingAdapter { impl SubclassingAdapter { /// Create an adapter that dynamically subclasses the specified view. /// This must be done before the view is shown or focused for - /// the first time. + /// the first time, and before anything observes the view with + /// key-value observing. /// /// All handlers will always be called on the main thread. /// @@ -182,6 +254,22 @@ impl SubclassingAdapter { { panic!("subclassing adapter already instantiated on view {view:?}"); } + // A previous adapter may have left its subclass on the view, + // if key-value observing had subclassed it in turn. + let installed = installed_subclass(unsafe { &*view }); + if installed.is_none() { + // Key-value observing crashes when it adds or removes an observer + // while our subclass sits on top of its own. + let reported_class: &AnyClass = unsafe { msg_send![view, class] }; + let actual_class = runtime_class(unsafe { &*view }); + if !std::ptr::eq(reported_class, actual_class) { + panic!( + "view {view:?} is already observed with key-value observing (its class {} reports itself as {}); create the subclassing adapter before anything observes the view", + actual_class.name(), + reported_class.name() + ); + } + } let adapter = unsafe { Adapter::new( view as *mut c_void, @@ -190,10 +278,10 @@ impl SubclassingAdapter { deactivation_handler, ) }; - // Cast to a pointer and back to force the lifetime to 'static - // SAFETY: We know the class will live as long as the instance, - // and we only use this reference while the instance is alive. - let prev_class = unsafe { &*((*view).class() as *const AnyClass) }; + let (prev_class, subclass) = installed.unwrap_or_else(|| { + let prev_class = runtime_class(unsafe { &*view }); + (prev_class, subclass_of(prev_class)) + }); let associated = AssociatedObject::new(adapter, prev_class, mtm); unsafe { objc_setAssociatedObject( @@ -203,43 +291,12 @@ impl SubclassingAdapter { OBJC_ASSOCIATION_RETAIN_NONATOMIC, ) }; - let mut subclasses = SUBCLASSES.lock().unwrap(); - let subclass = match subclasses.iter().find(|entry| entry.0 == prev_class) { - Some(entry) => entry.1, - None => { - let name = format!("AccessKitSubclassOf{}", prev_class.name()); - let mut builder = ClassBuilder::new(&name, prev_class).unwrap(); - unsafe { - builder.add_method( - sel!(superclass), - superclass as unsafe extern "C" fn(_, _) -> _, - ); - builder.add_method( - sel!(isAccessibilityElement), - is_accessibility_element as unsafe extern "C" fn(_, _) -> _, - ); - builder.add_method( - sel!(accessibilityElements), - accessibility_elements as unsafe extern "C" fn(_, _) -> _, - ); - builder.add_method( - sel!(accessibilityHitTest:), - accessibility_hit_test as unsafe extern "C" fn(_, _, _) -> _, - ); - builder.add_method( - sel!(didMoveToWindow), - did_move_to_window as unsafe extern "C" fn(_, _), - ); - } - let class = builder.register(); - subclasses.push((prev_class, class)); - class - } - }; // SAFETY: Changing the view's class is only safe because // the subclass doesn't add any instance variables; // it uses an associated object instead. - unsafe { object_setClass(view as *mut _, (subclass as *const AnyClass).cast()) }; + if installed.is_none() { + unsafe { object_setClass(view as *mut _, (subclass as *const AnyClass).cast()) }; + } let result = Self { view: retained_view, associated, @@ -311,7 +368,6 @@ impl Drop for SubclassingAdapter { fn drop(&mut self) { let prev_class = self.associated.ivars().prev_class; let view = Retained::as_ptr(&self.view) as *mut UIView; - unsafe { object_setClass(view as *mut _, (prev_class as *const AnyClass).cast()) }; unsafe { objc_setAssociatedObject( view as *mut _, @@ -320,5 +376,22 @@ impl Drop for SubclassingAdapter { OBJC_ASSOCIATION_RETAIN_NONATOMIC, ) }; + // Key-value observing may have subclassed our subclass in turn, + // and resetting the class would then break those observers. + // Our subclass is harmless without the associated object. + let view = unsafe { &*view }; + let is_our_subclass = SUBCLASSES + .lock() + .unwrap() + .iter() + .any(|entry| std::ptr::eq(entry.1, runtime_class(view))); + if is_our_subclass { + unsafe { + object_setClass( + view as *const _ as *mut _, + (prev_class as *const AnyClass).cast(), + ) + }; + } } } diff --git a/adapters/ios/tests/integration/harness.rs b/adapters/ios/tests/integration/harness.rs new file mode 100644 index 000000000..3672e95f5 --- /dev/null +++ b/adapters/ios/tests/integration/harness.rs @@ -0,0 +1,38 @@ +// Copyright 2026 The AccessKit Authors. All rights reserved. +// Licensed under the Apache License, Version 2.0 (found in +// the LICENSE-APACHE file) or the MIT license (found in +// the LICENSE-MIT file), at your option. + +use libtest_mimic::{Arguments, Trial}; +use std::process::ExitCode; + +pub struct Test { + pub name: &'static str, + pub run: fn(), +} + +macro_rules! tests { + ($($name:ident),* $(,)?) => { + &[$($crate::harness::Test { name: stringify!($name), run: $name }),*] + }; +} +pub(crate) use tests; + +pub fn run(suites: &[(&str, &[Test])]) -> ExitCode { + let mut args = Arguments::from_args(); + // libtest-mimic only runs tests on the main thread when it uses a single thread. + args.test_threads = Some(1); + let trials = suites + .iter() + .flat_map(|(suite, tests)| { + tests.iter().map(move |test| { + let run = test.run; + Trial::test(format!("{suite}::{}", test.name), move || { + run(); + Ok(()) + }) + }) + }) + .collect(); + libtest_mimic::run(&args, trials).exit_code() +} diff --git a/adapters/ios/tests/integration/main.rs b/adapters/ios/tests/integration/main.rs new file mode 100644 index 000000000..c756fae50 --- /dev/null +++ b/adapters/ios/tests/integration/main.rs @@ -0,0 +1,12 @@ +// Copyright 2026 The AccessKit Authors. All rights reserved. +// Licensed under the Apache License, Version 2.0 (found in +// the LICENSE-APACHE file) or the MIT license (found in +// the LICENSE-MIT file), at your option. + +mod harness; +mod subclassing_adapter; +mod util; + +fn main() -> std::process::ExitCode { + harness::run(&[("subclassing_adapter", subclassing_adapter::TESTS)]) +} diff --git a/adapters/ios/tests/integration/subclassing_adapter.rs b/adapters/ios/tests/integration/subclassing_adapter.rs new file mode 100644 index 000000000..2e5c672ab --- /dev/null +++ b/adapters/ios/tests/integration/subclassing_adapter.rs @@ -0,0 +1,230 @@ +// Copyright 2026 The AccessKit Authors. All rights reserved. +// Licensed under the Apache License, Version 2.0 (found in +// the LICENSE-APACHE file) or the MIT license (found in +// the LICENSE-MIT file), at your option. + +use std::{ + cell::Cell, + ffi::c_void, + panic::{self, AssertUnwindSafe}, + rc::Rc, +}; + +use accesskit_ios::SubclassingAdapter; +use objc2::declare::ClassBuilder; +use objc2::rc::Retained; +use objc2::runtime::{AnyClass, AnyObject, Bool, Sel}; +use objc2::{class, msg_send, msg_send_id, sel}; +use objc2_foundation::{CGPoint, NSArray, NSObject, NSString}; +use objc2_ui_kit::UIView; + +use crate::harness::{Test, tests}; +use crate::util::*; + +pub const TESTS: &[Test] = tests![ + the_view_is_subclassed, + querying_the_view_requests_the_tree, + dropping_restores_the_view_class, + dropping_releases_the_handlers, + a_view_can_get_a_new_adapter, + observers_can_be_removed_after_dropping, + a_view_left_subclassed_behaves_like_its_original_class, + an_observed_view_can_get_a_new_adapter, + a_view_observed_before_the_adapter_is_rejected, +]; + +fn adapter_for(view: &UIView) -> (SubclassingAdapter, Rc) { + let events = Rc::new(HandlerEvents::default()); + let adapter = unsafe { + SubclassingAdapter::new( + view as *const UIView as *mut c_void, + NoTree(Rc::clone(&events)), + NoActions, + NoDeactivation, + ) + }; + (adapter, events) +} + +fn query_accessibility(view: &UIView) { + let _: Bool = unsafe { msg_send![view, isAccessibilityElement] }; + let _: *mut NSArray = unsafe { msg_send![view, accessibilityElements] }; + // UIKit doesn't implement this method on plain views. + let responds: Bool = + unsafe { msg_send![view, respondsToSelector: sel!(accessibilityHitTest:)] }; + if responds.as_bool() { + let _: *mut AnyObject = + unsafe { msg_send![view, accessibilityHitTest: CGPoint::new(0., 0.)] }; + } + let superclass: &AnyClass = unsafe { msg_send![view, superclass] }; + assert_eq!(superclass.name(), "UIResponder"); +} + +fn query_accessibility_without_adapter(view: &UIView) { + query_accessibility(view); + let _: () = unsafe { msg_send![view, didMoveToWindow] }; +} + +struct Observation { + observer: Retained, + key_path: Retained, +} + +// Apps observe their views' geometry this way. +fn observe(view: &UIView) -> Observation { + let observer = NSObject::new(); + let key_path = NSString::from_str("frame"); + let no_context: *mut c_void = std::ptr::null_mut(); + let _: () = unsafe { + msg_send![view, addObserver: &*observer, forKeyPath: &*key_path, options: 0usize, context: no_context] + }; + Observation { observer, key_path } +} + +fn stop_observing(view: &UIView, observation: Observation) { + let _: () = unsafe { + msg_send![view, removeObserver: &*observation.observer, forKeyPath: &*observation.key_path] + }; +} + +thread_local! { + static MOVED_TO_WINDOW: Cell = const { Cell::new(false) }; +} + +extern "C" fn record_did_move_to_window(_this: &UIView, _cmd: Sel) { + MOVED_TO_WINDOW.set(true); +} + +fn new_recording_view() -> Retained { + let class = AnyClass::get("AccessKitTestRecordingView").unwrap_or_else(|| { + let mut builder = ClassBuilder::new("AccessKitTestRecordingView", class!(UIView)).unwrap(); + unsafe { + builder.add_method( + sel!(didMoveToWindow), + record_did_move_to_window as extern "C" fn(_, _), + ) + }; + builder.register() + }); + unsafe { msg_send_id![class, new] } +} + +fn the_view_is_subclassed() { + let view = new_view(); + let (_adapter, _) = adapter_for(&view); + assert_eq!(class_name(&view), "AccessKitSubclassOfUIView"); +} + +fn querying_the_view_requests_the_tree() { + let view = new_view(); + let (_adapter, events) = adapter_for(&view); + assert!(!events.tree_requested.get()); + query_accessibility(&view); + assert!(events.tree_requested.get()); +} + +fn dropping_restores_the_view_class() { + let view = new_view(); + let (adapter, _) = adapter_for(&view); + drop(adapter); + assert_eq!(class_name(&view), "UIView"); + query_accessibility_without_adapter(&view); +} + +fn dropping_releases_the_handlers() { + let view = new_view(); + let (adapter, events) = adapter_for(&view); + drop(adapter); + assert!(events.dropped.get()); +} + +fn a_view_can_get_a_new_adapter() { + let view = new_view(); + drop(adapter_for(&view)); + let (adapter, events) = adapter_for(&view); + assert_eq!(class_name(&view), "AccessKitSubclassOfUIView"); + query_accessibility(&view); + assert!(events.tree_requested.get()); + drop(adapter); + assert_eq!(class_name(&view), "UIView"); +} + +fn observers_can_be_removed_after_dropping() { + let view = new_view(); + let (adapter, events) = adapter_for(&view); + let observation = observe(&view); + assert_eq!( + class_name(&view), + "NSKVONotifying_AccessKitSubclassOfUIView" + ); + + drop(adapter); + assert!(events.dropped.get()); + assert_eq!( + class_name(&view), + "NSKVONotifying_AccessKitSubclassOfUIView" + ); + query_accessibility_without_adapter(&view); + + stop_observing(&view, observation); + assert_eq!(class_name(&view), "AccessKitSubclassOfUIView"); + query_accessibility_without_adapter(&view); +} + +fn a_view_left_subclassed_behaves_like_its_original_class() { + let view = new_recording_view(); + let (adapter, _) = adapter_for(&view); + let observation = observe(&view); + drop(adapter); + assert_eq!( + class_name(&view), + "NSKVONotifying_AccessKitSubclassOfAccessKitTestRecordingView" + ); + + let is_element: Bool = unsafe { msg_send![&view, isAccessibilityElement] }; + assert!(!is_element.as_bool()); + let elements: *mut NSArray = unsafe { msg_send![&view, accessibilityElements] }; + assert!(elements.is_null()); + MOVED_TO_WINDOW.set(false); + let _: () = unsafe { msg_send![&view, didMoveToWindow] }; + assert!(MOVED_TO_WINDOW.get()); + + stop_observing(&view, observation); +} + +fn an_observed_view_can_get_a_new_adapter() { + let view = new_view(); + let (adapter, _) = adapter_for(&view); + let observation = observe(&view); + drop(adapter); + + let (adapter, events) = adapter_for(&view); + assert_eq!( + class_name(&view), + "NSKVONotifying_AccessKitSubclassOfUIView" + ); + query_accessibility(&view); + assert!(events.tree_requested.get()); + drop(adapter); + assert!(events.dropped.get()); + + stop_observing(&view, observation); + let (adapter, events) = adapter_for(&view); + assert_eq!(class_name(&view), "AccessKitSubclassOfUIView"); + query_accessibility(&view); + assert!(events.tree_requested.get()); + drop(adapter); + assert_eq!(class_name(&view), "UIView"); +} + +fn a_view_observed_before_the_adapter_is_rejected() { + let view = new_view(); + let observation = observe(&view); + let result = panic::catch_unwind(AssertUnwindSafe(|| adapter_for(&view))); + assert!(result.is_err()); + assert_eq!(class_name(&view), "NSKVONotifying_UIView"); + + stop_observing(&view, observation); + assert_eq!(class_name(&view), "UIView"); + query_accessibility_without_adapter(&view); +} diff --git a/adapters/ios/tests/integration/util.rs b/adapters/ios/tests/integration/util.rs new file mode 100644 index 000000000..cb48ac562 --- /dev/null +++ b/adapters/ios/tests/integration/util.rs @@ -0,0 +1,55 @@ +// Copyright 2026 The AccessKit Authors. All rights reserved. +// Licensed under the Apache License, Version 2.0 (found in +// the LICENSE-APACHE file) or the MIT license (found in +// the LICENSE-MIT file), at your option. + +use std::{cell::Cell, rc::Rc}; + +use accesskit::{ActionHandler, ActionRequest, ActivationHandler, DeactivationHandler, TreeUpdate}; +use objc2::rc::Retained; +use objc2::runtime::AnyObject; +use objc2::{class, msg_send_id}; +use objc2_ui_kit::UIView; + +#[derive(Default)] +pub struct HandlerEvents { + pub tree_requested: Cell, + pub dropped: Cell, +} + +pub struct NoTree(pub Rc); + +impl ActivationHandler for NoTree { + fn request_initial_tree(&mut self) -> Option { + self.0.tree_requested.set(true); + None + } +} + +impl Drop for NoTree { + fn drop(&mut self) { + self.0.dropped.set(true); + } +} + +pub struct NoActions; + +impl ActionHandler for NoActions { + fn do_action(&mut self, _request: ActionRequest) {} +} + +pub struct NoDeactivation; + +impl DeactivationHandler for NoDeactivation { + fn deactivate_accessibility(&mut self) {} +} + +pub fn new_view() -> Retained { + unsafe { msg_send_id![class!(UIView), new] } +} + +// KVO overrides the `class` method to hide its own subclass. +pub fn class_name(view: &UIView) -> String { + let object: &AnyObject = view; + object.class().name().to_owned() +}