Skip to content

fix: clear clicked notifications and collapse per-window duplicates - #1408

Merged
tw93 merged 2 commits into
tw93:mainfrom
yhcharles:chy/notification-dismiss-dedupe
Oct 5, 2026
Merged

tw93 merged 2 commits into
tw93:mainfrom
yhcharles:chy/notification-dismiss-dedupe

Conversation

@yhcharles

Copy link
Copy Markdown
Contributor

Follow-up to #1394, covering the two pieces of the original notification work that did not land with it.

Problem

  1. A clicked notification stays in Notification Center. NSUserNotificationCenter keeps an activated notification until it is explicitly withdrawn; fix: route native notification clicks back to the page聽#1394 withdraws on close() and tag replacement, but not on activation.
  2. With --multi-window, every window runs its own instance of the site, so one incoming message raises one identical notification per window. They all compete for the same click, are indistinguishable in Notification Center, and each one increments the dock badge, so the badge multiplies by the window count.

Fix

  • didActivateNotification: removes the activated notification from the center before dispatching the click.
  • send keeps a small, time-bounded registry keyed on title+body. A different window repeating the same message within 1s is reported back as suppressed instead of being delivered. A site legitimately repeating a message inside one window still gets every notification, and a repeat after the window has passed is delivered normally.
  • Page side: a suppressed outcome forgets the notification (no click can ever reach it), fires no show, and does not touch the badge. This sits after the existing stale-reply check, so the close/replacement ordering guarantees from fix: route native notification clicks back to the page聽#1394 are unchanged.
  • objc2-foundation now declares the NSUserNotification and NSArray features notification.rs uses. They currently reach the module only through feature unification from other crates, so an unrelated dependency change could break the build with no local cause.

A suppressed window's notification is not clickable, so a click always lands in the window whose copy was delivered. That matches today's behavior, where each window's notification routes to that window.

Verification

  • cargo test --lib: the 5 new dedupe tests in notification.rs pass. Locally, two existing redirect tests in invoke.rs (redirects_keep_the_limit_relative_locations_and_statuses, redirects_recheck_cookies_and_never_restore_full_referrer) fail on my machine; they fail identically on unmodified main, while Quality & Testing is green on main, so they look environment-specific and unrelated to this change.
  • npx vitest run: 538 passed, including 3 new cases in tests/unit/notification-bridge.test.js (suppressed notification is not clickable and fires no show; suppressed does not increment the badge; delivered does).
  • cargo clippy --all-targets: no new warnings. cargo fmt --check, prettier --check: clean.
  • Not verified by hand: a real multi-window click-through on macOS.

yhcharles and others added 2 commits October 4, 2026 18:03
A clicked notification stayed in Notification Center, and with `--multi-window` every Pake window raised its own copy of the same message.

`NSUserNotificationCenter` keeps an activated notification until it is explicitly withdrawn, so `didActivateNotification:` now removes it, the same way `close()` and tag replacement already withdraw theirs.

With `--multi-window` every window runs its own instance of the site, so a single incoming message produced one identical notification per window, all competing for the same click and all indistinguishable; the page counted each one too, multiplying the dock badge by the window count. A *different* window repeating the same title+body within 1s now collapses into the first notification, while a site legitimately repeating a message inside one window still gets every notification. Suppression is reported back to the page as `suppressed`, so it stops tracking a notification no click can reach, fires no `show`, and leaves the badge alone.

Also declares the objc2-foundation features this module actually uses: `NSUserNotification` and `NSArray` were only reaching it through feature unification from other crates, so an unrelated dependency change could break the build with no local cause.

Dedupe is covered by unit tests in the module; the page-side suppression and badge paths by tests/unit/notification-bridge.test.js.
The dock badge is one count for the whole app. When a window's copy of a
message is suppressed, the window that delivered it has already counted it,
but the suppressed window never marked its auto badge as active, so clicking
or typing there left the shared badge in place. A suppressed reply now arms
the clear without incrementing, under the same unread and page-managed rules
as a delivered one.

Cross-window dedupe is also limited to the macOS native-click path. Only that
path routes a click to the exact window that raised the notification. The
Windows/Linux fallback (and macOS without a native center) arms a focus click
per window, so suppressing a copy there loses the click whenever the OS
activates the suppressed window, while the owner keeps a stale focus target
that can fire a phantom click later.
@tw93
tw93 merged commit 37e49aa into tw93:main Oct 5, 2026
8 checks passed
@tw93

tw93 commented Oct 5, 2026

Copy link
Copy Markdown
Owner

@yhcharles Thanks for following up on #1394. This is merged and shipped in pake-cli 3.17.3. Before merging I pushed one fix onto your branch: a window whose copy was suppressed can now still clear the shared dock badge when you click or type in it. I also limited the cross-window dedupe to the macOS native click path, because Windows and Linux cannot tell which window a toast belongs to, so collapsing copies there could lose the click.

Run npm install -g pake-cli@3.17.3 and rebuild an app to get it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants