From ac37b56ef43c9e97039967a5fd99f0d2dccb5b5a Mon Sep 17 00:00:00 2001 From: Lucas Fernandes Nogueira Date: Fri, 8 Oct 2021 11:38:24 -0300 Subject: [PATCH] fix(core): menu id map not reflecting the current window menu (#2726) --- .changes/fix-menu-ids.md | 5 ++ .changes/get-menu.md | 6 ++ core/tauri-runtime-wry/src/lib.rs | 108 ++++++++++++++--------------- core/tauri-runtime/src/webview.rs | 4 +- core/tauri-runtime/src/window.rs | 33 ++++++++- core/tauri/src/endpoints/window.rs | 4 +- core/tauri/src/manager.rs | 47 ++----------- core/tauri/src/window.rs | 4 +- 8 files changed, 106 insertions(+), 105 deletions(-) create mode 100644 .changes/fix-menu-ids.md create mode 100644 .changes/get-menu.md diff --git a/.changes/fix-menu-ids.md b/.changes/fix-menu-ids.md new file mode 100644 index 000000000..6b87270a0 --- /dev/null +++ b/.changes/fix-menu-ids.md @@ -0,0 +1,5 @@ +--- +"tauri": patch +--- + +Fixes the menu id mapping not reflecting the current window. diff --git a/.changes/get-menu.md b/.changes/get-menu.md new file mode 100644 index 000000000..db34c3bf1 --- /dev/null +++ b/.changes/get-menu.md @@ -0,0 +1,6 @@ +--- +"tauri-runtime": minor +"tauri-runtime-wry": minor +--- + +Replace `WindowBuilder`'s `has_menu` with `get_menu`. diff --git a/core/tauri-runtime-wry/src/lib.rs b/core/tauri-runtime-wry/src/lib.rs index 83b08c64e..c9263cd23 100644 --- a/core/tauri-runtime-wry/src/lib.rs +++ b/core/tauri-runtime-wry/src/lib.rs @@ -9,7 +9,7 @@ use tauri_runtime::{ Request as HttpRequest, RequestParts as HttpRequestParts, Response as HttpResponse, ResponseParts as HttpResponseParts, }, - menu::{CustomMenuItem, Menu, MenuEntry, MenuHash, MenuItem, MenuUpdate, Submenu}, + menu::{CustomMenuItem, Menu, MenuEntry, MenuHash, MenuId, MenuItem, MenuUpdate}, monitor::Monitor, webview::{ FileDropEvent, FileDropHandler, RpcRequest, WebviewRpcHandler, WindowBuilder, WindowBuilderBase, @@ -693,7 +693,7 @@ impl From for UserAttentionTypeWrapper { pub struct WindowBuilderWrapper { inner: WryWindowBuilder, center: bool, - menu: Menu, + menu: Option, } // safe since `menu_items` are read only here @@ -736,7 +736,7 @@ impl WindowBuilder for WindowBuilderWrapper { } fn menu(mut self, menu: Menu) -> Self { - self.menu = convert_menu_id(Menu::new(), menu); + self.menu.replace(menu); self } @@ -857,8 +857,8 @@ impl WindowBuilder for WindowBuilderWrapper { self.inner.window.window_icon.is_some() } - fn has_menu(&self) -> bool { - self.inner.window.window_menu.is_some() + fn get_menu(&self) -> Option<&Menu> { + self.menu.as_ref() } } @@ -1206,6 +1206,7 @@ impl Dispatch for WryDispatcher { ) -> Result> { let (tx, rx) = channel(); let label = pending.label.clone(); + let menu_ids = pending.menu_ids.clone(); let context = self.context.clone(); send_user_message( @@ -1223,7 +1224,11 @@ impl Dispatch for WryDispatcher { window_id, context: self.context.clone(), }; - Ok(DetachedWindow { label, dispatcher }) + Ok(DetachedWindow { + label, + dispatcher, + menu_ids, + }) } fn set_resizable(&self, resizable: bool) -> Result<()> { @@ -1451,7 +1456,7 @@ impl WindowHandle { pub struct WindowWrapper { label: String, inner: WindowHandle, - menu_items: HashMap, + menu_items: Option>, } /// A Tauri [`Runtime`] wrapper around wry. @@ -1509,6 +1514,7 @@ impl RuntimeHandle for WryHandle { ) -> Result> { let (tx, rx) = channel(); let label = pending.label.clone(); + let menu_ids = pending.menu_ids.clone(); let context = self.context.clone(); send_user_message( &self.context, @@ -1525,7 +1531,11 @@ impl RuntimeHandle for WryHandle { window_id, context: self.context.clone(), }; - Ok(DetachedWindow { label, dispatcher }) + Ok(DetachedWindow { + label, + dispatcher, + menu_ids, + }) } fn run_on_main_thread(&self, f: F) -> Result<()> { @@ -1632,6 +1642,7 @@ impl Runtime for Wry { fn create_window(&self, pending: PendingWindow) -> Result> { let label = pending.label.clone(); + let menu_ids = pending.menu_ids.clone(); let proxy = self.event_loop.create_proxy(); let webview = create_webview( &self.event_loop, @@ -1717,7 +1728,11 @@ impl Runtime for Wry { .unwrap() .insert(webview.inner.window().id(), webview); - Ok(DetachedWindow { label, dispatcher }) + Ok(DetachedWindow { + label, + dispatcher, + menu_ids, + }) } #[cfg(feature = "system-tray")] @@ -2006,17 +2021,16 @@ fn handle_user_message( let _ = window.drag_window(); } WindowMessage::UpdateMenuItem(id, update) => { - let item = webview - .menu_items - .get_mut(&id) - .expect("menu item not found"); - match update { - MenuUpdate::SetEnabled(enabled) => item.set_enabled(enabled), - MenuUpdate::SetTitle(title) => item.set_title(&title), - MenuUpdate::SetSelected(selected) => item.set_selected(selected), - #[cfg(target_os = "macos")] - MenuUpdate::SetNativeImage(image) => { - item.set_native_image(NativeImageWrapper::from(image).0) + if let Some(menu_items) = webview.menu_items.as_mut() { + let item = menu_items.get_mut(&id).expect("menu item not found"); + match update { + MenuUpdate::SetEnabled(enabled) => item.set_enabled(enabled), + MenuUpdate::SetTitle(title) => item.set_title(&title), + MenuUpdate::SetSelected(selected) => item.set_selected(selected), + #[cfg(target_os = "macos")] + MenuUpdate::SetNativeImage(image) => { + item.set_native_image(NativeImageWrapper::from(image).0) + } } } } @@ -2464,38 +2478,6 @@ fn center_window(window: &Window, window_size: WryPhysicalSize) -> Result<( } } -fn convert_menu_id(mut new_menu: Menu, menu: Menu) -> Menu { - for item in menu.items { - match item { - MenuEntry::CustomItem(c) => { - let mut item = CustomMenuItem::new(c.id_str, c.title); - #[cfg(target_os = "macos")] - if let Some(native_image) = c.native_image { - item = item.native_image(native_image); - } - if let Some(accelerator) = c.keyboard_accelerator { - item = item.accelerator(accelerator); - } - if !c.enabled { - item = item.disabled(); - } - if c.selected { - item = item.selected(); - } - new_menu = new_menu.add_item(item); - } - MenuEntry::NativeItem(i) => { - new_menu = new_menu.add_native_item(i); - } - MenuEntry::Submenu(submenu) => { - let new_submenu = convert_menu_id(Menu::new(), submenu.inner); - new_menu = new_menu.add_submenu(Submenu::new(submenu.title, new_submenu)); - } - } - } - new_menu -} - fn to_wry_menu( custom_menu_items: &mut HashMap, menu: Menu, @@ -2544,15 +2526,18 @@ fn create_webview( file_drop_handler, label, url, + menu_ids, .. } = pending; let is_window_transparent = window_builder.inner.window.transparent; - let menu_items = { + let menu_items = if let Some(menu) = window_builder.menu { let mut menu_items = HashMap::new(); - let menu = to_wry_menu(&mut menu_items, window_builder.menu); + let menu = to_wry_menu(&mut menu_items, menu); window_builder.inner = window_builder.inner.with_menu(menu); - menu_items + Some(menu_items) + } else { + None }; let window = window_builder.inner.build(event_loop).unwrap(); @@ -2577,13 +2562,18 @@ fn create_webview( .unwrap() // safe to unwrap because we validate the URL beforehand .with_transparent(is_window_transparent); if let Some(handler) = rpc_handler { - webview_builder = - webview_builder.with_rpc_handler(create_rpc_handler(context.clone(), label.clone(), handler)); + webview_builder = webview_builder.with_rpc_handler(create_rpc_handler( + context.clone(), + label.clone(), + menu_ids.clone(), + handler, + )); } if let Some(handler) = file_drop_handler { webview_builder = webview_builder.with_file_drop_handler(create_file_drop_handler( context, label.clone(), + menu_ids, handler, )); } @@ -2639,6 +2629,7 @@ fn create_webview( fn create_rpc_handler( context: Context, label: String, + menu_ids: HashMap, handler: WebviewRpcHandler, ) -> Box Option + 'static> { Box::new(move |window, request| { @@ -2649,6 +2640,7 @@ fn create_rpc_handler( context: context.clone(), }, label: label.clone(), + menu_ids: menu_ids.clone(), }, RpcRequestWrapper(request).into(), ); @@ -2660,6 +2652,7 @@ fn create_rpc_handler( fn create_file_drop_handler( context: Context, label: String, + menu_ids: HashMap, handler: FileDropHandler, ) -> Box bool + 'static> { Box::new(move |window, event| { @@ -2671,6 +2664,7 @@ fn create_file_drop_handler( context: context.clone(), }, label: label.clone(), + menu_ids: menu_ids.clone(), }, ) }) diff --git a/core/tauri-runtime/src/webview.rs b/core/tauri-runtime/src/webview.rs index 36b2f16b2..523efe81b 100644 --- a/core/tauri-runtime/src/webview.rs +++ b/core/tauri-runtime/src/webview.rs @@ -156,8 +156,8 @@ pub trait WindowBuilder: WindowBuilderBase { /// Whether the icon was set or not. fn has_icon(&self) -> bool; - /// Whether the menu was set or not. - fn has_menu(&self) -> bool; + /// Gets the window menu. + fn get_menu(&self) -> Option<&Menu>; } /// Rpc request. diff --git a/core/tauri-runtime/src/window.rs b/core/tauri-runtime/src/window.rs index e77ed6de5..5776091b1 100644 --- a/core/tauri-runtime/src/window.rs +++ b/core/tauri-runtime/src/window.rs @@ -6,6 +6,7 @@ use crate::{ http::{Request as HttpRequest, Response as HttpResponse}, + menu::{Menu, MenuEntry, MenuHash, MenuId}, webview::{FileDropHandler, WebviewAttributes, WebviewRpcHandler}, Dispatch, Runtime, WindowBuilder, }; @@ -61,6 +62,18 @@ pub struct MenuEvent { pub menu_item_id: u16, } +fn get_menu_ids(map: &mut HashMap, menu: &Menu) { + for item in &menu.items { + match item { + MenuEntry::CustomItem(c) => { + map.insert(c.id, c.id_str.clone()); + } + MenuEntry::Submenu(s) => get_menu_ids(map, &s.inner), + _ => {} + } + } +} + /// A webview window that has yet to be built. pub struct PendingWindow { /// The label that the window will be named. @@ -82,6 +95,9 @@ pub struct PendingWindow { /// The resolved URL to load on the webview. pub url: String, + + /// Maps runtime id to a string menu id. + pub menu_ids: HashMap, } impl PendingWindow { @@ -91,6 +107,10 @@ impl PendingWindow { webview_attributes: WebviewAttributes, label: impl Into, ) -> Self { + let mut menu_ids = HashMap::new(); + if let Some(menu) = window_builder.get_menu() { + get_menu_ids(&mut menu_ids, menu); + } Self { window_builder, webview_attributes, @@ -99,6 +119,7 @@ impl PendingWindow { rpc_handler: None, file_drop_handler: None, url: "tauri://localhost".to_string(), + menu_ids, } } @@ -108,14 +129,20 @@ impl PendingWindow { webview_attributes: WebviewAttributes, label: impl Into, ) -> Self { + let window_builder = <::WindowBuilder>::with_config(window_config); + let mut menu_ids = HashMap::new(); + if let Some(menu) = window_builder.get_menu() { + get_menu_ids(&mut menu_ids, menu); + } Self { - window_builder: <::WindowBuilder>::with_config(window_config), + window_builder, webview_attributes, uri_scheme_protocols: Default::default(), label: label.into(), rpc_handler: None, file_drop_handler: None, url: "tauri://localhost".to_string(), + menu_ids, } } @@ -142,6 +169,9 @@ pub struct DetachedWindow { /// The [`Dispatch`](crate::Dispatch) associated with the window. pub dispatcher: R::Dispatcher, + + /// Maps runtime id to a string menu id. + pub menu_ids: HashMap, } impl Clone for DetachedWindow { @@ -149,6 +179,7 @@ impl Clone for DetachedWindow { Self { label: self.label.clone(), dispatcher: self.dispatcher.clone(), + menu_ids: self.menu_ids.clone(), } } } diff --git a/core/tauri/src/endpoints/window.rs b/core/tauri/src/endpoints/window.rs index 81c443f8e..f57232380 100644 --- a/core/tauri/src/endpoints/window.rs +++ b/core/tauri/src/endpoints/window.rs @@ -90,7 +90,7 @@ pub enum WindowManagerCmd { #[serde(tag = "cmd", content = "data", rename_all = "camelCase")] pub enum Cmd { CreateWebview { - options: WindowConfig, + options: Box, }, Manage { label: Option, @@ -123,7 +123,7 @@ impl Cmd { window .create_window(label.clone(), url, |_, webview_attributes| { ( - <::WindowBuilder>::with_config(options), + <::WindowBuilder>::with_config(*options), webview_attributes, ) })? diff --git a/core/tauri/src/manager.rs b/core/tauri/src/manager.rs index 11634f7f6..d5bdbf16b 100644 --- a/core/tauri/src/manager.rs +++ b/core/tauri/src/manager.rs @@ -29,10 +29,7 @@ use crate::api::path::{resolve_path, BaseDirectory}; use crate::app::{GlobalMenuEventListener, WindowMenuEvent}; -use crate::{ - runtime::menu::{Menu, MenuEntry, MenuHash, MenuId}, - MenuEvent, -}; +use crate::{runtime::menu::Menu, MenuEvent}; use serde::Serialize; use serde_json::Value as JsonValue; @@ -79,8 +76,6 @@ pub struct InnerWindowManager { uri_scheme_protocols: HashMap>>, /// The menu set to all windows. menu: Option, - /// Maps runtime id to a strongly typed menu id. - menu_ids: HashMap, /// Menu event listeners to all windows. menu_event_listeners: Arc>>, /// Window event listeners to all windows. @@ -89,20 +84,14 @@ pub struct InnerWindowManager { impl fmt::Debug for InnerWindowManager { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut s = f.debug_struct("InnerWindowManager"); - #[allow(unused_mut)] - let mut w = s + f.debug_struct("InnerWindowManager") .field("plugins", &self.plugins) .field("state", &self.state) .field("config", &self.config) .field("default_window_icon", &self.default_window_icon) - .field("package_info", &self.package_info); - { - w = w - .field("menu", &self.menu) - .field("menu_ids", &self.menu_ids); - } - w.finish() + .field("package_info", &self.package_info) + .field("menu", &self.menu) + .finish() } } @@ -133,18 +122,6 @@ impl Clone for WindowManager { } } -fn get_menu_ids(map: &mut HashMap, menu: &Menu) { - for item in &menu.items { - match item { - MenuEntry::CustomItem(c) => { - map.insert(c.id, c.id_str.clone()); - } - MenuEntry::Submenu(s) => get_menu_ids(map, &s.inner), - _ => {} - } - } -} - impl WindowManager { #[allow(clippy::too_many_arguments)] pub(crate) fn with_handlers( @@ -170,13 +147,6 @@ impl WindowManager { default_window_icon: context.default_window_icon, package_info: context.package_info, uri_scheme_protocols, - menu_ids: { - let mut map = HashMap::new(); - if let Some(menu) = &menu { - get_menu_ids(&mut map, menu) - } - map - }, menu, menu_event_listeners: Arc::new(menu_event_listeners), window_event_listeners: Arc::new(window_event_listeners), @@ -195,11 +165,6 @@ impl WindowManager { self.inner.state.clone() } - /// Get the menu ids mapper. - pub(crate) fn menu_ids(&self) -> HashMap { - self.inner.menu_ids.clone() - } - /// Get the base path to serve data from. /// /// * In dev mode, this will be based on the `devPath` configuration value. @@ -281,7 +246,7 @@ impl WindowManager { } } - if !pending.window_builder.has_menu() { + if pending.window_builder.get_menu().is_none() { if let Some(menu) = &self.inner.menu { pending.window_builder = pending.window_builder.menu(menu.clone()); } diff --git a/core/tauri/src/window.rs b/core/tauri/src/window.rs index 8db8fa29d..dafed7a32 100644 --- a/core/tauri/src/window.rs +++ b/core/tauri/src/window.rs @@ -316,7 +316,7 @@ impl Window { /// Registers a menu event listener. pub fn on_menu_event(&self, f: F) -> uuid::Uuid { - let menu_ids = self.manager.menu_ids(); + let menu_ids = self.window.menu_ids.clone(); self.window.dispatcher.on_menu_event(move |event| { f(MenuEvent { menu_item_id: menu_ids.get(&event.menu_item_id).unwrap().clone(), @@ -329,7 +329,7 @@ impl Window { /// Gets a handle to the window menu. pub fn menu_handle(&self) -> MenuHandle { MenuHandle { - ids: self.manager.menu_ids(), + ids: self.window.menu_ids.clone(), dispatcher: self.dispatcher(), } }