From 8f169cdcb06e680831976dc5866c3a95039e2080 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 7 Sep 2024 22:01:06 +0200 Subject: [PATCH] Don't expose macOS implementation details in the public API The `show_context_menu_for_nsview` method previously used the `objc::runtime::Object` type, which means that the library's dependence on `objc` is exposed as part of the public API. Additionally, since the method is taking a raw pointer (and as such would be unsound if passing e.g. `ptr::dangling()`), I took the opportunity to also mark the method as `unsafe`. This is done in preparation for transitioning to `objc2`. --- .changes/make-macos-impl-details-private.md | 5 ++++ README.md | 2 +- examples/tao.rs | 4 +++- .../windows-common-controls-v6/src/main.rs | 2 +- examples/winit.rs | 2 +- examples/wry.rs | 4 +++- src/items/submenu.rs | 14 +++++++---- src/lib.rs | 17 +++++++++++--- src/menu.rs | 15 ++++++++---- src/platform_impl/macos/mod.rs | 23 ++++++++++++++----- 10 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 .changes/make-macos-impl-details-private.md diff --git a/.changes/make-macos-impl-details-private.md b/.changes/make-macos-impl-details-private.md new file mode 100644 index 00000000..5b58b68f --- /dev/null +++ b/.changes/make-macos-impl-details-private.md @@ -0,0 +1,5 @@ +--- +"muda": minor +--- + +**Breaking Change** Changed the type of the pointer passed in `show_context_menu_for_nsview` to `c_void`, and make the method `unsafe`. diff --git a/README.md b/README.md index 82d460ab..55ddb39f 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ menu.show_context_menu_for_hwnd(window.hwnd() as isize, Some(position.into())); #[cfg(target_os = "linux")] menu.show_context_menu_for_gtk_window(>k_window, Some(position.into())); #[cfg(target_os = "macos")] -menu.show_context_menu_for_nsview(nsview, Some(position.into())); +unsafe { menu.show_context_menu_for_nsview(nsview, Some(position.into())) }; ``` ## Processing menu events diff --git a/examples/tao.rs b/examples/tao.rs index 22cfb4a6..de29f15d 100644 --- a/examples/tao.rs +++ b/examples/tao.rs @@ -225,7 +225,9 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option

muda::Icon { diff --git a/examples/windows-common-controls-v6/src/main.rs b/examples/windows-common-controls-v6/src/main.rs index 480746d8..bb811b2c 100644 --- a/examples/windows-common-controls-v6/src/main.rs +++ b/examples/windows-common-controls-v6/src/main.rs @@ -213,7 +213,7 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option

muda::Icon { diff --git a/src/items/submenu.rs b/src/items/submenu.rs index 55fc70ad..0e250c25 100644 --- a/src/items/submenu.rs +++ b/src/items/submenu.rs @@ -246,10 +246,16 @@ impl ContextMenu for Submenu { } #[cfg(target_os = "macos")] - fn show_context_menu_for_nsview(&self, view: cocoa::base::id, position: Option) { - self.inner - .borrow_mut() - .show_context_menu_for_nsview(view, position) + unsafe fn show_context_menu_for_nsview( + &self, + view: *const std::ffi::c_void, + position: Option, + ) { + unsafe { + self.inner + .borrow_mut() + .show_context_menu_for_nsview(view, position) + } } #[cfg(target_os = "macos")] diff --git a/src/lib.rs b/src/lib.rs index 75b8293e..f3a75b38 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,7 +98,7 @@ //! # #[cfg(target_os = "linux")] //! # let gtk_window = gtk::Window::builder().build(); //! # #[cfg(target_os = "macos")] -//! # let nsview = 0 as *mut objc::runtime::Object; +//! # let nsview = std::ptr::null(); //! // --snip-- //! let position = muda::dpi::PhysicalPosition { x: 100., y: 120. }; //! #[cfg(target_os = "windows")] @@ -106,7 +106,7 @@ //! #[cfg(target_os = "linux")] //! menu.show_context_menu_for_gtk_window(>k_window, Some(position.into())); //! #[cfg(target_os = "macos")] -//! menu.show_context_menu_for_nsview(nsview, Some(position.into())); +//! unsafe { menu.show_context_menu_for_nsview(nsview, Some(position.into())) }; //! ``` //! # Processing menu events //! @@ -336,10 +336,21 @@ pub trait ContextMenu { /// Shows this menu as a context menu for the specified `NSView`. /// /// - `position` is relative to the window top-left corner, if `None`, the cursor position is used. + /// + /// # Safety + /// + /// The view must be a pointer to a valid `NSView`. #[cfg(target_os = "macos")] - fn show_context_menu_for_nsview(&self, view: cocoa::base::id, position: Option); + unsafe fn show_context_menu_for_nsview( + &self, + view: *const std::ffi::c_void, + position: Option, + ); /// Get the underlying NSMenu reserved for context menus. + /// + /// The returned pointer is valid for as long as the `ContextMenu` is. If + /// you need it to be alive for longer, retain it. #[cfg(target_os = "macos")] fn ns_menu(&self) -> *mut std::ffi::c_void; } diff --git a/src/menu.rs b/src/menu.rs index 429b146f..589dfab9 100644 --- a/src/menu.rs +++ b/src/menu.rs @@ -370,10 +370,17 @@ impl ContextMenu for Menu { } #[cfg(target_os = "macos")] - fn show_context_menu_for_nsview(&self, view: cocoa::base::id, position: Option) { - self.inner - .borrow_mut() - .show_context_menu_for_nsview(view, position) + unsafe fn show_context_menu_for_nsview( + &self, + view: *const std::ffi::c_void, + position: Option, + ) { + // SAFETY: Upheld by caller + unsafe { + self.inner + .borrow_mut() + .show_context_menu_for_nsview(view, position) + } } #[cfg(target_os = "macos")] diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 3bde139f..16ccca9c 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -8,7 +8,7 @@ mod util; pub(crate) use icon::PlatformIcon; -use std::{cell::RefCell, collections::HashMap, rc::Rc, sync::Once}; +use std::{cell::RefCell, collections::HashMap, ffi::c_void, rc::Rc, sync::Once}; use cocoa::{ appkit::{self, CGFloat, NSApp, NSApplication, NSEventModifierFlags, NSMenu, NSMenuItem}, @@ -179,8 +179,13 @@ impl Menu { unsafe { NSApp().setMainMenu_(NSMenu::new(nil) as _) } } - pub fn show_context_menu_for_nsview(&self, view: id, position: Option) { - show_context_menu(self.ns_menu.1, view, position) + pub unsafe fn show_context_menu_for_nsview( + &self, + view: *const c_void, + position: Option, + ) { + // SAFETY: Upheld by caller + unsafe { show_context_menu(self.ns_menu.1, view, position) } } pub fn ns_menu(&self) -> *mut std::ffi::c_void { @@ -655,8 +660,13 @@ impl MenuChild { .collect() } - pub fn show_context_menu_for_nsview(&self, view: id, position: Option) { - show_context_menu(self.ns_menu.as_ref().unwrap().1, view, position) + pub unsafe fn show_context_menu_for_nsview( + &self, + view: *const c_void, + position: Option, + ) { + // SAFETY: Upheld by caller + unsafe { show_context_menu(self.ns_menu.as_ref().unwrap().1, view, position) } } pub fn set_as_windows_menu_for_nsapp(&self) { @@ -1066,8 +1076,9 @@ fn menuitem_set_native_icon(menuitem: id, icon: Option) { } } -fn show_context_menu(ns_menu: id, view: id, position: Option) { +unsafe fn show_context_menu(ns_menu: id, view: *const c_void, position: Option) { unsafe { + let view = view as id; let window: id = msg_send![view, window]; let scale_factor: CGFloat = msg_send![window, backingScaleFactor]; let (location, in_view) = if let Some(pos) = position.map(|p| p.to_logical(scale_factor)) {