fix: route native notification clicks back to the page - #1394
Merged
Merged
Conversation
Pake replaces `window.Notification` with a bridge to `tauri-plugin-notification`, but that plugin is fire-and-forget on desktop: `show()` hands the notification to `notify_rust` and drops the handle, so nothing ever reports which notification the user clicked. The page's own click routing therefore never ran, and a packaged Slack never jumped to the conversation a notification came from. macOS now delivers notifications itself through `NSUserNotificationCenter` with a delegate, tagging each one with `<window label>|<id>`. A click reveals the originating window through the same unminimize + show + `reapply_window_icon` + focus sequence as every other hidden-to-visible path (tw93#1323), then hands the exact id back to that webview. Availability is probed once on the main thread during setup, so a process without a bundle identifier or a future macOS without the deprecated API falls back to the plugin path instead of dereferencing nil. Other platforms keep the plugin path unchanged. The page-side notification is now a real `EventTarget` rather than a bare object with an `onclick` slot, so `addEventListener("click", ...)` works and `event.target` is the notification itself. It also carries the standard `tag`/`data`/`icon` fields, lets a same-tag notification replace its predecessor, and caps how many stay click-addressable. The focus heuristic that used to stand in for a click callback is now a fallback only where the platform reports nothing: it fires once, requires the notification to have been raised while the window was unfocused, expires after 60s, and is cancelled by any in-page interaction. Rust reports `nativeClick` so macOS disables the heuristic entirely and an ordinary app switch cannot fire a phantom click. Ids are minted by the packaged (untrusted) page and echoed into a native identifier and a webview `eval`, so they are validated against a strict charset and length in Rust. Covered by tests/unit/notification-bridge.test.js. The three harnesses that load event.js now provide real `Event`/`EventTarget`, and the injected bridge bails out early when they are missing rather than aborting the rest of the script.
Owner
|
@yhcharles Thanks for the contribution. This is merged and shipped in pake-cli 3.17.3: on macOS a notification click now reopens and focuses the window that raised it, and Windows and Linux keep the focus-based fallback. Your follow-up in #1408 landed in the same release. Run |
Withdraw closed and replaced macOS notifications, ignore stale IPC acknowledgements, and dispatch close events asynchronously to avoid lifecycle reentry. Keep native clicks tied to the originating window and notification.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Pake replaces
window.Notificationwith a bridge totauri-plugin-notification, but that plugin is fire-and-forget on desktop:show()hands the notification tonotify_rustand drops the handle, so nothing ever reports which notification the user clicked. The page's own click routing therefore never runs, and a packaged Slack/Discord/etc. never jumps to the conversation a notification came from — arguably one of the most common reasons someone would package a chat site with Pake in the first place.Fix
macOS now delivers notifications itself through
NSUserNotificationCenterwith a delegate, tagging each one with<window label>|<id>. A click reveals the originating window through the same unminimize + show +reapply_window_icon+ focus sequence as every other hidden-to-visible path (#1323), then hands the exact id back to that webview. Availability is probed once on the main thread during setup, so a process without a bundle identifier (e.g.pnpm run dev) or a future macOS without the deprecated API falls back to the plugin path instead of dereferencing nil. Other platforms keep the existing plugin path unchanged — this PR is macOS-first, since that's where the native click-report API is available today.The page-side notification is now a real
EventTargetrather than a bare object with anonclickslot, soaddEventListener("click", ...)works andevent.targetis the notification itself — matching the standardNotificationAPI pages actually code against. It also carries the standardtag/data/iconfields, lets a same-tag notification replace its predecessor, and caps how many stay click-addressable.The focus heuristic Pake used before (treat regaining focus shortly after a notification as a click) is now a fallback only where the platform reports nothing: it fires once, requires the notification to have been raised while the window was unfocused, expires after 60s, and is cancelled by any in-page interaction. Rust reports
nativeClickso macOS disables the heuristic entirely and an ordinary app switch cannot fire a phantom click.Notification ids are minted by the packaged (untrusted) page and echoed into a native identifier and a webview
eval, so they're validated against a strict charset and length on the Rust side before use.Why
NSUserNotificationand notUserNotifications.frameworkThe whole
NSUserNotificationfamily is deprecated in favor ofUserNotifications.framework, but that framework needs a provisioned bundle and a permission prompt that a webpage wrapper can't meaningfully ask for. It's also the APItauri-plugin-notificationalready reaches throughnotify-rust, so this isn't a new deprecated surface for the project — and the code probes for the class at startup, so a macOS version that finally drops it falls back to the plugin path rather than crashing.Verification
cargo test --lib— 50 passed, including new coverage insrc-tauri/src/app/notification.rsnpx vitest run— 510 passed, includingtests/unit/notification-bridge.test.js(new)cargo clippy --all-targets— no new warningscargo fmt --check,prettier --check— clean