diff --git a/crates/nsh-base/src/menu.rs b/crates/nsh-base/src/menu.rs index 12b3834e22..715482bab3 100644 --- a/crates/nsh-base/src/menu.rs +++ b/crates/nsh-base/src/menu.rs @@ -138,30 +138,12 @@ impl Items { let order = watch_order( move || listed.borrow().iter().map(|e| e.node).collect(), move |order| { - let pick = { + { let mut list = sorted.borrow_mut(); let mut taken: Vec> = list.drain(..).map(Some).collect(); list.extend(order.iter().filter_map(|&i| taken[i].take())); - let Some(how) = keyed.get() else { - return; - }; - let (level, last) = match how { - Keyed::First => (0, false), - Keyed::Last => (0, true), - Keyed::Submenu(level) => (level, false), - }; - let mut enabled = list - .iter() - .filter(|e| e.level == level && !e.disabled.get_untracked()); - match last { - false => enabled.next().copied(), - true => enabled.next_back().copied(), - } - }; - if let Some(entry) = pick { - keyed.replace(None); - entry.node.focus(); } + Self::focus_pending_in(&sorted, keyed); }, ); Self { @@ -172,11 +154,43 @@ impl Items { } } + /// Layout runs even when an order watch finds the items already in + /// order (and therefore does not call its change handler). + fn focus_pending(&self) { + Self::focus_pending_in(&self.list, self.keyed); + } + + fn focus_pending_in(list: &RefCell>, keyed: Stored>) { + let Some(how) = keyed.get() else { + return; + }; + let (level, last) = match how { + Keyed::First => (0, false), + Keyed::Last => (0, true), + Keyed::Submenu(level) => (level, false), + }; + let pick = { + let list = list.borrow(); + let mut enabled = list + .iter() + .filter(|e| e.level == level && !e.disabled.get_untracked()); + match last { + false => enabled.next().copied(), + true => enabled.next_back().copied(), + } + }; + if let Some(entry) = pick { + keyed.replace(None); + entry.node.focus(); + } + } + /// Focuses the first enabled item of submenu `level` once its items /// are there (now, when they already are). fn focus_first(&self, level: u32) { self.keyed.replace(Some(Keyed::Submenu(level))); self.order.invalidate(); + self.focus_pending(); } /// Adds an item of submenu `level`, which the menu forgets when the @@ -905,6 +919,7 @@ pub fn MenuItem(props: MenuItemProps, cx: Scope) -> impl IntoElem let menu = use_context::(); let hover = menu.clone(); let chosen = menu.clone(); + let laid_out = menu.clone(); let sheet = menu.filter(|m| m.sheet); let (checked, disabled) = (props.checked, props.disabled); let node = element_ref(); @@ -987,6 +1002,11 @@ pub fn MenuItem(props: MenuItemProps, cx: Scope) -> impl IntoElem ..AccessibilityState::default() }) .element_ref(&node) + .on_layout(move |_: &Bounds| { + if let Some(m) = &laid_out { + m.items.focus_pending(); + } + }) .checked_style(props.checked_style) .disabled(disabled) .optional_label(props.accessibility_label) diff --git a/crates/nsh-platform-web/web/nsh.js b/crates/nsh-platform-web/web/nsh.js index 96cf909dd4..d400d98ad6 100644 --- a/crates/nsh-platform-web/web/nsh.js +++ b/crates/nsh-platform-web/web/nsh.js @@ -6535,7 +6535,7 @@ class Player { * menu); true when a view took it. The browser raises `contextmenu` for * right clicks, Control-clicks on macOS, the keyboard's Menu key and * Shift+F10 (on the focused element), and screen readers' "show menu" - * action. Those come with no pointer (the DOM's `pointerType` `""`): the + * action. Those come with no pointer (an empty `pointerType` or no button): the * event names none (`pointerType: null`) and is placed at the view's * bottom-start corner, where a menu opened below it starts. */ @@ -6546,8 +6546,9 @@ class Player { if (el._events?.has("onContextMenu")) { e.preventDefault(); const r = el.getBoundingClientRect(); - // A `MouseEvent` (no `pointerType`) is a browser's older mouse one. - const pointerType = e.pointerType === undefined ? "mouse" : e.pointerType || null; + // Chromium can name a mouse for keyboard context menus, but no + // button (-1). Older MouseEvents without pointerType are mouse input. + const pointerType = e.button === -1 ? null : e.pointerType === undefined ? "mouse" : e.pointerType || null; let at = e; if (pointerType === null) { const rtl = getComputedStyle(el).direction === "rtl"; diff --git a/crates/nsh-platform-web/web/nsh.test.mjs b/crates/nsh-platform-web/web/nsh.test.mjs index 40b486c884..00aa707989 100644 --- a/crates/nsh-platform-web/web/nsh.test.mjs +++ b/crates/nsh-platform-web/web/nsh.test.mjs @@ -53,6 +53,43 @@ const failing = () => { const events = (sent) => sent.filter(([name, p]) => name === "onHost" && p.type === "event").map(([, p]) => p.event); +test("keyboard context menus have no pointer even when Chromium names a mouse", () => { + const { player } = fakePlayer(); + const el = { + _id: 2, + _events: new Set(["onContextMenu"]), + getBoundingClientRect: () => ({ left: 10, right: 90, top: 20, bottom: 60 }), + }; + player.viewOf = () => el; + player.ancestors = () => [el]; + player.point = (at) => [at.clientX, at.clientY]; + let payload; + player.event = (_, name, value) => { assert.equal(name, "onContextMenu"); payload = value; }; + const saved = globalThis.getComputedStyle; + globalThis.getComputedStyle = () => ({ direction: "ltr" }); + try { + for (const [pointerType, button, keyboard] of [ + ["mouse", -1, true], + ["", -1, true], + ["", 0, true], + ["mouse", 2, false], + [undefined, 2, false], + ["mouse", 0, false], + ]) { + let prevented = false; + assert.equal(player.contextMenu({ + target: el, pointerType, button, clientX: 40, clientY: 30, + preventDefault() { prevented = true; }, + }), true); + assert.equal(prevented, true); + assert.equal(payload.pointerType, keyboard ? null : "mouse"); + assert.deepEqual(payload.pagePosition, keyboard ? { x: 10, y: 60 } : { x: 40, y: 30 }); + } + } finally { + globalThis.getComputedStyle = saved; + } +}); + test("input autofill hints are set, changed and cleared on both field kinds", () => { const { player } = fakePlayer(); const attrs = new Map(); diff --git a/crates/nsh-platform-windows/src/notifications.rs b/crates/nsh-platform-windows/src/notifications.rs index 4c6bd3b511..13e60b41d4 100644 --- a/crates/nsh-platform-windows/src/notifications.rs +++ b/crates/nsh-platform-windows/src/notifications.rs @@ -44,7 +44,7 @@ use windows::{ NotificationSetting, ToastNotification, ToastNotificationManager, ToastNotifier, }, Win32::{ - Foundation::{HWND, RECT}, + Foundation::{ERROR_NOT_FOUND, HWND, RECT}, Graphics::Gdi::{ ANTIALIASED_QUALITY, BI_RGB, BITMAPINFO, BITMAPINFOHEADER, CLIP_DEFAULT_PRECIS, CreateBitmap, CreateCompatibleDC, CreateDIBSection, CreateFontW, DEFAULT_CHARSET, @@ -234,7 +234,7 @@ fn set_value(key: &str, name: Option<&str>, value: &str) -> Result<()> { fn permission() -> NotificationPermission { let setting = match notifier() { None => return NotificationPermission::Unavailable, - Some(notifier) => notifier.and_then(|n| n.Setting()), + Some(notifier) => notifier.and_then(|n| setting(&n)), }; match setting { Ok(NotificationSetting::Enabled) => NotificationPermission::Granted, @@ -246,13 +246,24 @@ fn permission() -> NotificationPermission { } } +/// The user's setting for the app's notifications. An app that has not +/// shown one yet has no entry in Settings > System > Notifications, and +/// `Setting` fails with `ERROR_NOT_FOUND` until the first toast makes it: +/// nothing turned off, so they are on. +fn setting(notifier: &ToastNotifier) -> Result { + match notifier.Setting() { + Err(err) if err.code() == ERROR_NOT_FOUND.to_hresult() => Ok(NotificationSetting::Enabled), + setting => setting, + } +} + /// Whether the system took `notification`. fn show(notification: &Notification) -> Result { let Some(notifier) = notifier() else { return Ok(false); }; let notifier = notifier?; - if notifier.Setting()? != NotificationSetting::Enabled { + if setting(¬ifier)? != NotificationSetting::Enabled { return Ok(false); } let document = XmlDocument::new()?;