Skip to content

Commit

Permalink
Don't expose macOS implementation details in the public API
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
madsmtm committed Sep 7, 2024
1 parent d3fdaf2 commit 8f169cd
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 22 deletions.
5 changes: 5 additions & 0 deletions .changes/make-macos-impl-details-private.md
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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(&gtk_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
Expand Down
4 changes: 3 additions & 1 deletion examples/tao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option<P
#[cfg(target_os = "linux")]
menu.show_context_menu_for_gtk_window(window.gtk_window().as_ref(), position);
#[cfg(target_os = "macos")]
menu.show_context_menu_for_nsview(window.ns_view() as _, position);
unsafe {
menu.show_context_menu_for_nsview(window.ns_view() as _, position);
}
}

fn load_icon(path: &std::path::Path) -> muda::Icon {
Expand Down
2 changes: 1 addition & 1 deletion examples/windows-common-controls-v6/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option<P
{
use tao::rwh_06::*;
if let RawWindowHandle::AppKit(handle) = window.window_handle().unwrap().as_raw() {
menu.show_context_menu_for_nsview(handle.ns_view.as_ptr() as _, position);
unsafe { menu.show_context_menu_for_nsview(handle.ns_view.as_ptr() as _, position) };
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/winit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option<P
{
use winit::raw_window_handle::*;
if let RawWindowHandle::AppKit(handle) = window.window_handle().unwrap().as_raw() {
menu.show_context_menu_for_nsview(handle.ns_view.as_ptr() as _, position);
unsafe { menu.show_context_menu_for_nsview(handle.ns_view.as_ptr() as _, position) };
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion examples/wry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option<P
#[cfg(target_os = "linux")]
menu.show_context_menu_for_gtk_window(window.gtk_window().as_ref(), position);
#[cfg(target_os = "macos")]
menu.show_context_menu_for_nsview(window.ns_view() as _, position);
unsafe {
menu.show_context_menu_for_nsview(window.ns_view() as _, position);
}
}

fn load_icon(path: &std::path::Path) -> muda::Icon {
Expand Down
14 changes: 10 additions & 4 deletions src/items/submenu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Position>) {
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<Position>,
) {
unsafe {
self.inner
.borrow_mut()
.show_context_menu_for_nsview(view, position)
}
}

#[cfg(target_os = "macos")]
Expand Down
17 changes: 14 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@
//! # #[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")]
//! menu.show_context_menu_for_hwnd(window_hwnd, Some(position.into()));
//! #[cfg(target_os = "linux")]
//! menu.show_context_menu_for_gtk_window(&gtk_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
//!
Expand Down Expand Up @@ -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<dpi::Position>);
unsafe fn show_context_menu_for_nsview(
&self,
view: *const std::ffi::c_void,
position: Option<dpi::Position>,
);

/// 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;
}
Expand Down
15 changes: 11 additions & 4 deletions src/menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Position>) {
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<Position>,
) {
// SAFETY: Upheld by caller
unsafe {
self.inner
.borrow_mut()
.show_context_menu_for_nsview(view, position)
}
}

#[cfg(target_os = "macos")]
Expand Down
23 changes: 17 additions & 6 deletions src/platform_impl/macos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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<Position>) {
show_context_menu(self.ns_menu.1, view, position)
pub unsafe fn show_context_menu_for_nsview(
&self,
view: *const c_void,
position: Option<Position>,
) {
// SAFETY: Upheld by caller
unsafe { show_context_menu(self.ns_menu.1, view, position) }
}

pub fn ns_menu(&self) -> *mut std::ffi::c_void {
Expand Down Expand Up @@ -655,8 +660,13 @@ impl MenuChild {
.collect()
}

pub fn show_context_menu_for_nsview(&self, view: id, position: Option<Position>) {
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<Position>,
) {
// 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) {
Expand Down Expand Up @@ -1066,8 +1076,9 @@ fn menuitem_set_native_icon(menuitem: id, icon: Option<NativeIcon>) {
}
}

fn show_context_menu(ns_menu: id, view: id, position: Option<Position>) {
unsafe fn show_context_menu(ns_menu: id, view: *const c_void, position: Option<Position>) {
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)) {
Expand Down

0 comments on commit 8f169cd

Please sign in to comment.