Skip to content

Commit

Permalink
- Removed subclassing in both implementations of `show_context_menu_f…
Browse files Browse the repository at this point in the history
…or_hwnd`

- Removed `CONTEXT_MENU_SUBCLASS_ID` and `CONTEXT_SUMENU_SUBCLASS_ID` constants
- Adding entry in `.changes`
  • Loading branch information
npwoods committed Aug 22, 2024
1 parent 884adfc commit b34ed19
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 34 deletions.
13 changes: 13 additions & 0 deletions .changes/win32-context-menu-fixes
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"muda": "patch"
---

These changes are to address a design flaw in how `muda` handles context menus on Windows.

The normal way menus with `muda` are used on Windows is they get attached to a window, and the window is subclassed to intercept commands (`WM_COMMAND`) and some drawing behaviors (I'm not 100% sure on this, but this appears in place to support theming and dark mode). When the `Menu` is dropped, it will be unattached from all windows and the subclasses will be removed.

Context menus seem to follow a slightly different pattern. When `show_context_menu_for_hwnd()` is called, the subclass is attached to the window, but never removed. This behavior is problematic because the menu can be dropped without the subclass removed, causing a crash. Furthermore, having this subclass attached unnecessarily is not necessarily a good idea.

These changes to `muda` refactor menu support on Windows to remove the HWND subclassing when the context menu is active.

These changes also address a memory leak where when an HWND is subclassed, the reference to the menu object was placed in a `Box` seemingly unnecessarily, and this `Box` never seems to be dropped. This was changed to using straight references.
36 changes: 2 additions & 34 deletions src/platform_impl/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,22 +413,7 @@ impl Menu {
}

pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option<Position>) {
unsafe {
SetWindowSubclass(
hwnd as _,
Some(menu_subclass_proc),
CONTEXT_MENU_SUBCLASS_ID,
dwrefdata_from_obj(self),
);
}
let rc = show_context_menu(hwnd as _, self.hpopupmenu, position);
unsafe {
RemoveWindowSubclass(
hwnd as _,
Some(menu_subclass_proc),
CONTEXT_MENU_SUBCLASS_ID,
);
}
if let Some(item) = rc.and_then(|rc| self.find_by_id(rc)) {
unsafe {
menu_selected(hwnd as _, &mut item.borrow_mut());
Expand Down Expand Up @@ -935,22 +920,7 @@ impl MenuChild {
}

pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option<Position>) {
unsafe {
SetWindowSubclass(
hwnd as _,
Some(menu_subclass_proc),
CONTEXT_SUBMENU_SUBCLASS_ID,
dwrefdata_from_obj(self),
);
}
let rc = show_context_menu(hwnd as _, self.hpopupmenu, position);
unsafe {
RemoveWindowSubclass(
hwnd as _,
Some(menu_subclass_proc),
CONTEXT_SUBMENU_SUBCLASS_ID,
);
}
if let Some(item) = rc.and_then(|rc| self.find_by_id(rc)) {
unsafe {
menu_selected(hwnd as _, &mut item.borrow_mut());
Expand Down Expand Up @@ -1083,8 +1053,6 @@ unsafe fn obj_from_dwrefdata<T>(dwrefdata: usize) -> &'static mut T {
const MENU_SUBCLASS_ID: usize = 200;
const MENU_UPDATE_THEME: u32 = 201;
const SUBMENU_SUBCLASS_ID: usize = 202;
const CONTEXT_MENU_SUBCLASS_ID: usize = 203;
const CONTEXT_SUBMENU_SUBCLASS_ID: usize = 204;

unsafe extern "system" fn menu_subclass_proc(
hwnd: windows_sys::Win32::Foundation::HWND,
Expand All @@ -1106,11 +1074,11 @@ unsafe extern "system" fn menu_subclass_proc(
let id = util::LOWORD(wparam as _) as u32;

let item = match uidsubclass {
MENU_SUBCLASS_ID | CONTEXT_MENU_SUBCLASS_ID => {
MENU_SUBCLASS_ID => {
let menu = obj_from_dwrefdata::<Menu>(dwrefdata);
menu.find_by_id(id)
}
SUBMENU_SUBCLASS_ID | CONTEXT_SUBMENU_SUBCLASS_ID => {
SUBMENU_SUBCLASS_ID => {
let menu = obj_from_dwrefdata::<MenuChild>(dwrefdata);
menu.find_by_id(id)
}
Expand Down

0 comments on commit b34ed19

Please sign in to comment.