From b34ed1959ccaaef8992ebe856a1f3574bfbe2b40 Mon Sep 17 00:00:00 2001 From: "U-ALDEBERAN\\Nate" Date: Thu, 22 Aug 2024 07:54:19 -0400 Subject: [PATCH] - Removed subclassing in both implementations of `show_context_menu_for_hwnd` - Removed `CONTEXT_MENU_SUBCLASS_ID` and `CONTEXT_SUMENU_SUBCLASS_ID` constants - Adding entry in `.changes` --- .changes/win32-context-menu-fixes | 13 +++++++++++ src/platform_impl/windows/mod.rs | 36 ++----------------------------- 2 files changed, 15 insertions(+), 34 deletions(-) create mode 100644 .changes/win32-context-menu-fixes diff --git a/.changes/win32-context-menu-fixes b/.changes/win32-context-menu-fixes new file mode 100644 index 00000000..6c5862d5 --- /dev/null +++ b/.changes/win32-context-menu-fixes @@ -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. diff --git a/src/platform_impl/windows/mod.rs b/src/platform_impl/windows/mod.rs index 7190ed8a..9f3382bd 100644 --- a/src/platform_impl/windows/mod.rs +++ b/src/platform_impl/windows/mod.rs @@ -413,22 +413,7 @@ impl Menu { } pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { - 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()); @@ -935,22 +920,7 @@ impl MenuChild { } pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { - 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()); @@ -1083,8 +1053,6 @@ unsafe fn obj_from_dwrefdata(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, @@ -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::(dwrefdata); menu.find_by_id(id) } - SUBMENU_SUBCLASS_ID | CONTEXT_SUBMENU_SUBCLASS_ID => { + SUBMENU_SUBCLASS_ID => { let menu = obj_from_dwrefdata::(dwrefdata); menu.find_by_id(id) }