From e4725bf50fb46e830fa5265d765ee596b80e3085 Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Thu, 31 Mar 2022 20:18:49 +0200 Subject: [PATCH] fix(Windows): fix random chars when changing menu item title (#361) --- .changes/windows-fix-menu-item-set-title.md | 5 + src/platform_impl/windows/dark_mode.rs | 4 +- src/platform_impl/windows/event_loop.rs | 2 +- src/platform_impl/windows/menu.rs | 282 ++++++++++---------- src/platform_impl/windows/system_tray.rs | 2 +- src/platform_impl/windows/util.rs | 11 +- src/platform_impl/windows/window.rs | 2 +- 7 files changed, 154 insertions(+), 154 deletions(-) create mode 100644 .changes/windows-fix-menu-item-set-title.md diff --git a/.changes/windows-fix-menu-item-set-title.md b/.changes/windows-fix-menu-item-set-title.md new file mode 100644 index 000000000..4f7dfc05e --- /dev/null +++ b/.changes/windows-fix-menu-item-set-title.md @@ -0,0 +1,5 @@ +--- +"tao": "patch" +--- + +* On Windows, Fix random characters when changing menu items title through `CustomMenunItem::set_title`. \ No newline at end of file diff --git a/src/platform_impl/windows/dark_mode.rs b/src/platform_impl/windows/dark_mode.rs index 41a661625..16f0dd0ee 100644 --- a/src/platform_impl/windows/dark_mode.rs +++ b/src/platform_impl/windows/dark_mode.rs @@ -65,8 +65,8 @@ lazy_static! { } }; - static ref DARK_THEME_NAME: Vec = util::to_wstring("DarkMode_Explorer"); - static ref LIGHT_THEME_NAME: Vec = util::to_wstring(""); + static ref DARK_THEME_NAME: Vec = util::encode_wide("DarkMode_Explorer"); + static ref LIGHT_THEME_NAME: Vec = util::encode_wide(""); } /// Attempt to set a theme on a window, if necessary. diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index c24a43b2e..6450dad1d 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -589,7 +589,7 @@ lazy_static! { RegisterWindowMessageA("Tao::SetRetainMaximized") }; static ref THREAD_EVENT_TARGET_WINDOW_CLASS: Vec = unsafe { - let mut class_name= util::to_wstring("Tao Thread Event Target"); + let mut class_name= util::encode_wide("Tao Thread Event Target"); let class = WNDCLASSEXW { cbSize: mem::size_of::() as u32, diff --git a/src/platform_impl/windows/menu.rs b/src/platform_impl/windows/menu.rs index 141ed7859..79c281f8a 100644 --- a/src/platform_impl/windows/menu.rs +++ b/src/platform_impl/windows/menu.rs @@ -4,7 +4,7 @@ use std::{collections::HashMap, fmt, sync::Mutex}; use windows::Win32::{ - Foundation::{HWND, LPARAM, LRESULT, PSTR, PWSTR, WPARAM}, + Foundation::{HWND, LPARAM, LRESULT, PWSTR, WPARAM}, UI::{ Input::KeyboardAndMouse::*, Shell::*, @@ -75,7 +75,7 @@ impl MenuHandler { } #[derive(Debug, Clone)] -pub struct MenuItemAttributes(pub(crate) u16, HMENU); +pub struct MenuItemAttributes(pub(crate) u16, HMENU, Option); impl MenuItemAttributes { pub fn id(&self) -> MenuId { @@ -94,15 +94,20 @@ impl MenuItemAttributes { } } pub fn set_title(&mut self, title: &str) { + let mut title = title.to_string(); + if let Some(accelerator) = &self.2 { + title.push('\t'); + title.push_str(accelerator.to_string().as_str()); + } unsafe { - let info = MENUITEMINFOA { - cbSize: std::mem::size_of::() as _, + let info = MENUITEMINFOW { + cbSize: std::mem::size_of::() as _, fMask: MIIM_STRING, - dwTypeData: PSTR(String::from(title).as_mut_ptr()), + dwTypeData: PWSTR(util::encode_wide(title).as_mut_ptr()), ..Default::default() }; - SetMenuItemInfoA(self.1, self.0 as u32, false, &info); + SetMenuItemInfoW(self.1, self.0 as u32, false, &info); } } pub fn set_selected(&mut self, selected: bool) { @@ -174,7 +179,7 @@ impl Menu { &mut self, menu_id: MenuId, title: &str, - accelerators: Option, + accelerator: Option, enabled: bool, selected: bool, _menu_type: MenuType, @@ -188,27 +193,31 @@ impl Menu { flags |= MF_CHECKED; } - let mut anno_title = title.to_string(); - // format title - if let Some(accelerators) = accelerators.clone() { - anno_title.push('\t'); - format_hotkey(accelerators, &mut anno_title); - } - // NOTE(amrbashir): The title must be a null-terminated string. Otherwise, it will display some gibberish characters at the end. - if !anno_title.ends_with("\0") { - anno_title.push_str("\0"); + let mut title = title.to_string(); + if let Some(accelerator) = &accelerator { + title.push('\t'); + title.push_str(accelerator.to_string().as_str()); } - AppendMenuW(self.hmenu, flags, menu_id.0 as _, anno_title); + AppendMenuW( + self.hmenu, + flags, + menu_id.0 as _, + PWSTR(util::encode_wide(title).as_mut_ptr()), + ); // add our accels - if let Some(accelerators) = accelerators { - if let Some(accelerators) = convert_accelerator(menu_id.0, accelerators) { + if let Some(accelerators) = &accelerator { + if let Some(accelerators) = accelerators.to_accel(menu_id.0) { self.accels.insert(menu_id.0, AccelWrapper(accelerators)); } } MENU_IDS.lock().unwrap().push(menu_id.0 as _); - CustomMenuItem(MenuItemAttributes(menu_id.0, self.hmenu)) + CustomMenuItem(MenuItemAttributes( + menu_id.0, + self.hmenu, + accelerator.clone(), + )) } } @@ -270,22 +279,6 @@ impl Menu { } } -/* - Disabled as menu's seems to be linked to the app - so they are dropped when the app closes. - see discussion here; - - https://github.com/tauri-apps/tao/pull/106#issuecomment-880034210 - - impl Drop for Menu { - fn drop(&mut self) { - unsafe { - DestroyMenu(self.hmenu); - } - } - } -*/ - const MENU_SUBCLASS_ID: usize = 4568; pub fn initialize(menu_builder: Menu, window: HWND, menu_handler: MenuHandler) -> HMENU { @@ -401,119 +394,122 @@ fn execute_edit_command(command: EditCommand) { } } -// Convert a hotkey to an accelerator. -fn convert_accelerator(id: u16, key: Accelerator) -> Option { - let mut virt_key = FVIRTKEY; - let key_mods: ModifiersState = key.mods; - if key_mods.control_key() { - virt_key |= FCONTROL; - } - if key_mods.alt_key() { - virt_key |= FALT; - } - if key_mods.shift_key() { - virt_key |= FSHIFT; - } - - let raw_key = if let Some(vk_code) = key_to_vk(&key.key) { - let mod_code = vk_code >> 8; - if mod_code & 0x1 != 0 { - virt_key |= FSHIFT; - } - if mod_code & 0x02 != 0 { +impl Accelerator { + // Convert a hotkey to an accelerator. + fn to_accel(&self, menu_id: u16) -> Option { + let mut virt_key = FVIRTKEY; + let key_mods: ModifiersState = self.mods; + if key_mods.control_key() { virt_key |= FCONTROL; } - if mod_code & 0x04 != 0 { + if key_mods.alt_key() { virt_key |= FALT; } - vk_code & 0x00ff - } else { - dbg!("Failed to convert key {:?} into virtual key code", key.key); - return None; - }; + if key_mods.shift_key() { + virt_key |= FSHIFT; + } - Some(ACCEL { - fVirt: virt_key as u8, - key: raw_key as u16, - cmd: id, - }) -} + let raw_key = if let Some(vk_code) = key_to_vk(&self.key) { + let mod_code = vk_code >> 8; + if mod_code & 0x1 != 0 { + virt_key |= FSHIFT; + } + if mod_code & 0x02 != 0 { + virt_key |= FCONTROL; + } + if mod_code & 0x04 != 0 { + virt_key |= FALT; + } + vk_code & 0x00ff + } else { + dbg!("Failed to convert key {:?} into virtual key code", self.key); + return None; + }; -// Format the hotkey in a Windows-native way. -fn format_hotkey(key: Accelerator, s: &mut String) { - let key_mods: ModifiersState = key.mods; - if key_mods.control_key() { - s.push_str("Ctrl+"); - } - if key_mods.shift_key() { - s.push_str("Shift+"); + Some(ACCEL { + fVirt: virt_key as u8, + key: raw_key as u16, + cmd: menu_id, + }) } - if key_mods.alt_key() { - s.push_str("Alt+"); - } - if key_mods.super_key() { - s.push_str("Windows+"); - } - match &key.key { - KeyCode::KeyA => s.push('A'), - KeyCode::KeyB => s.push('B'), - KeyCode::KeyC => s.push('C'), - KeyCode::KeyD => s.push('D'), - KeyCode::KeyE => s.push('E'), - KeyCode::KeyF => s.push('F'), - KeyCode::KeyG => s.push('G'), - KeyCode::KeyH => s.push('H'), - KeyCode::KeyI => s.push('I'), - KeyCode::KeyJ => s.push('J'), - KeyCode::KeyK => s.push('K'), - KeyCode::KeyL => s.push('L'), - KeyCode::KeyM => s.push('M'), - KeyCode::KeyN => s.push('N'), - KeyCode::KeyO => s.push('O'), - KeyCode::KeyP => s.push('P'), - KeyCode::KeyQ => s.push('Q'), - KeyCode::KeyR => s.push('R'), - KeyCode::KeyS => s.push('S'), - KeyCode::KeyT => s.push('T'), - KeyCode::KeyU => s.push('U'), - KeyCode::KeyV => s.push('V'), - KeyCode::KeyW => s.push('W'), - KeyCode::KeyX => s.push('X'), - KeyCode::KeyY => s.push('Y'), - KeyCode::KeyZ => s.push('Z'), - KeyCode::Digit0 => s.push('0'), - KeyCode::Digit1 => s.push('1'), - KeyCode::Digit2 => s.push('2'), - KeyCode::Digit3 => s.push('3'), - KeyCode::Digit4 => s.push('4'), - KeyCode::Digit5 => s.push('5'), - KeyCode::Digit6 => s.push('6'), - KeyCode::Digit7 => s.push('7'), - KeyCode::Digit8 => s.push('8'), - KeyCode::Digit9 => s.push('9'), - KeyCode::Comma => s.push(','), - KeyCode::Minus => s.push('-'), - KeyCode::Period => s.push('.'), - KeyCode::Space => s.push_str("Space"), - KeyCode::Equal => s.push('='), - KeyCode::Semicolon => s.push(';'), - KeyCode::Slash => s.push('/'), - KeyCode::Backslash => s.push('\\'), - KeyCode::Quote => s.push('\''), - KeyCode::Backquote => s.push('`'), - KeyCode::BracketLeft => s.push('['), - KeyCode::BracketRight => s.push(']'), - KeyCode::Tab => s.push_str("Tab"), - KeyCode::Escape => s.push_str("Esc"), - KeyCode::Delete => s.push_str("Del"), - KeyCode::Insert => s.push_str("Ins"), - KeyCode::PageUp => s.push_str("PgUp"), - KeyCode::PageDown => s.push_str("PgDn"), - // These names match LibreOffice. - KeyCode::ArrowLeft => s.push_str("Left"), - KeyCode::ArrowRight => s.push_str("Right"), - KeyCode::ArrowUp => s.push_str("Up"), - KeyCode::ArrowDown => s.push_str("Down"), - _ => s.push_str(&format!("{:?}", key.key)), + + fn to_string(&self) -> String { + let mut s = String::new(); + let key_mods: ModifiersState = self.mods; + if key_mods.control_key() { + s.push_str("Ctrl+"); + } + if key_mods.shift_key() { + s.push_str("Shift+"); + } + if key_mods.alt_key() { + s.push_str("Alt+"); + } + if key_mods.super_key() { + s.push_str("Windows+"); + } + match &self.key { + KeyCode::KeyA => s.push('A'), + KeyCode::KeyB => s.push('B'), + KeyCode::KeyC => s.push('C'), + KeyCode::KeyD => s.push('D'), + KeyCode::KeyE => s.push('E'), + KeyCode::KeyF => s.push('F'), + KeyCode::KeyG => s.push('G'), + KeyCode::KeyH => s.push('H'), + KeyCode::KeyI => s.push('I'), + KeyCode::KeyJ => s.push('J'), + KeyCode::KeyK => s.push('K'), + KeyCode::KeyL => s.push('L'), + KeyCode::KeyM => s.push('M'), + KeyCode::KeyN => s.push('N'), + KeyCode::KeyO => s.push('O'), + KeyCode::KeyP => s.push('P'), + KeyCode::KeyQ => s.push('Q'), + KeyCode::KeyR => s.push('R'), + KeyCode::KeyS => s.push('S'), + KeyCode::KeyT => s.push('T'), + KeyCode::KeyU => s.push('U'), + KeyCode::KeyV => s.push('V'), + KeyCode::KeyW => s.push('W'), + KeyCode::KeyX => s.push('X'), + KeyCode::KeyY => s.push('Y'), + KeyCode::KeyZ => s.push('Z'), + KeyCode::Digit0 => s.push('0'), + KeyCode::Digit1 => s.push('1'), + KeyCode::Digit2 => s.push('2'), + KeyCode::Digit3 => s.push('3'), + KeyCode::Digit4 => s.push('4'), + KeyCode::Digit5 => s.push('5'), + KeyCode::Digit6 => s.push('6'), + KeyCode::Digit7 => s.push('7'), + KeyCode::Digit8 => s.push('8'), + KeyCode::Digit9 => s.push('9'), + KeyCode::Comma => s.push(','), + KeyCode::Minus => s.push('-'), + KeyCode::Period => s.push('.'), + KeyCode::Space => s.push_str("Space"), + KeyCode::Equal => s.push('='), + KeyCode::Semicolon => s.push(';'), + KeyCode::Slash => s.push('/'), + KeyCode::Backslash => s.push('\\'), + KeyCode::Quote => s.push('\''), + KeyCode::Backquote => s.push('`'), + KeyCode::BracketLeft => s.push('['), + KeyCode::BracketRight => s.push(']'), + KeyCode::Tab => s.push_str("Tab"), + KeyCode::Escape => s.push_str("Esc"), + KeyCode::Delete => s.push_str("Del"), + KeyCode::Insert => s.push_str("Ins"), + KeyCode::PageUp => s.push_str("PgUp"), + KeyCode::PageDown => s.push_str("PgDn"), + // These names match LibreOffice. + KeyCode::ArrowLeft => s.push_str("Left"), + KeyCode::ArrowRight => s.push_str("Right"), + KeyCode::ArrowUp => s.push_str("Up"), + KeyCode::ArrowDown => s.push_str("Down"), + _ => s.push_str(&format!("{:?}", self.key)), + } + s } } diff --git a/src/platform_impl/windows/system_tray.rs b/src/platform_impl/windows/system_tray.rs index bcb483b6b..d67f6dffe 100644 --- a/src/platform_impl/windows/system_tray.rs +++ b/src/platform_impl/windows/system_tray.rs @@ -51,7 +51,7 @@ impl SystemTrayBuilder { ) -> Result { let hmenu: Option = self.tray_menu.map(|m| m.hmenu()); - let mut class_name = util::to_wstring("tao_system_tray_app"); + let mut class_name = util::encode_wide("tao_system_tray_app"); unsafe { let hinstance = GetModuleHandleA(PSTR::default()); diff --git a/src/platform_impl/windows/util.rs b/src/platform_impl/windows/util.rs index 85fd23ea0..14a3b9c6d 100644 --- a/src/platform_impl/windows/util.rs +++ b/src/platform_impl/windows/util.rs @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - io, mem, + io, + iter::once, + mem, ops::BitAnd, os::windows::prelude::OsStrExt, ptr, slice, @@ -44,11 +46,8 @@ pub fn wchar_ptr_to_string(wchar: PWSTR) -> String { wchar_to_string(wchar_slice) } -pub fn to_wstring(str: &str) -> Vec { - std::ffi::OsStr::new(str) - .encode_wide() - .chain(Some(0).into_iter()) - .collect() +pub fn encode_wide(string: impl AsRef) -> Vec { + string.as_ref().encode_wide().chain(once(0)).collect() } pub unsafe fn status_map BOOL>(mut fun: F) -> Option { diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index e87d5483a..8c8f5b25a 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -969,7 +969,7 @@ unsafe fn register_window_class( window_icon: &Option, taskbar_icon: &Option, ) -> Vec { - let mut class_name = util::to_wstring("Window Class"); + let mut class_name = util::encode_wide("Window Class"); let h_icon = taskbar_icon .as_ref()