From 5ba1c71140a7381c5ace03da59ab04a7c688253d Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 18 Jan 2026 21:30:12 -0500 Subject: [PATCH] Fix grouping issue with sequence ID --- web/public/sw.js | 46 ++++++++++++++++++++++---------- web/src/app/Notifier.js | 12 +++++---- web/src/app/notificationUtils.js | 12 +++++++-- web/src/components/hooks.js | 18 ++++++------- 4 files changed, 58 insertions(+), 30 deletions(-) diff --git a/web/public/sw.js b/web/public/sw.js index 6b298414..c8808145 100644 --- a/web/public/sw.js +++ b/web/public/sw.js @@ -4,7 +4,7 @@ import { NavigationRoute, registerRoute } from "workbox-routing"; import { NetworkFirst } from "workbox-strategies"; import { clientsClaim } from "workbox-core"; import { dbAsync } from "../src/app/db"; -import { badge, icon, messageWithSequenceId, toNotificationParams } from "../src/app/notificationUtils"; +import { badge, icon, messageWithSequenceId, notificationTag, toNotificationParams } from "../src/app/notificationUtils"; import initI18n from "../src/app/i18n"; import { EVENT_MESSAGE, @@ -38,6 +38,13 @@ const handlePushMessage = async (data) => { console.log("[ServiceWorker] Message received", data); + // Look up subscription for baseUrl and topic + const subscription = await db.subscriptions.get(subscriptionId); + if (!subscription) { + console.log("[ServiceWorker] Subscription not found", subscriptionId); + return; + } + // Delete existing notification with same sequence ID (if any) const sequenceId = message.sequence_id || message.id; if (sequenceId) { @@ -65,10 +72,11 @@ const handlePushMessage = async (data) => { await self.registration.showNotification( ...toNotificationParams({ - subscriptionId, message, defaultTitle: message.topic, topicRoute: new URL(message.topic, self.location.origin).toString(), + baseUrl: subscription.baseUrl, + topic: subscription.topic, }) ); }; @@ -81,18 +89,23 @@ const handlePushMessageDelete = async (data) => { const db = await dbAsync(); console.log("[ServiceWorker] Deleting notification sequence", data); + // Look up subscription for baseUrl and topic + const subscription = await db.subscriptions.get(subscriptionId); + if (!subscription) { + console.log("[ServiceWorker] Subscription not found", subscriptionId); + return; + } + // Delete notification with the same sequence_id const sequenceId = message.sequence_id; if (sequenceId) { await db.notifications.where({ subscriptionId, sequenceId }).delete(); } - // Close browser notification with matching tag - const tag = message.sequence_id || message.id; - if (tag) { - const notifications = await self.registration.getNotifications({ tag }); - notifications.forEach((notification) => notification.close()); - } + // Close browser notification with matching tag (scoped by topic) + const tag = notificationTag(subscription.baseUrl, subscription.topic, message.sequence_id || message.id); + const notifications = await self.registration.getNotifications({ tag }); + notifications.forEach((notification) => notification.close()); // Update subscription last message id (for ?since=... queries) await db.subscriptions.update(subscriptionId, { @@ -108,18 +121,23 @@ const handlePushMessageClear = async (data) => { const db = await dbAsync(); console.log("[ServiceWorker] Marking notification as read", data); + // Look up subscription for baseUrl and topic + const subscription = await db.subscriptions.get(subscriptionId); + if (!subscription) { + console.log("[ServiceWorker] Subscription not found", subscriptionId); + return; + } + // Mark notification as read (set new = 0) const sequenceId = message.sequence_id; if (sequenceId) { await db.notifications.where({ subscriptionId, sequenceId }).modify({ new: 0 }); } - // Close browser notification with matching tag - const tag = message.sequence_id || message.id; - if (tag) { - const notifications = await self.registration.getNotifications({ tag }); - notifications.forEach((notification) => notification.close()); - } + // Close browser notification with matching tag (scoped by topic) + const tag = notificationTag(subscription.baseUrl, subscription.topic, message.sequence_id || message.id); + const notifications = await self.registration.getNotifications({ tag }); + notifications.forEach((notification) => notification.close()); // Update subscription last message id (for ?since=... queries) await db.subscriptions.update(subscriptionId, { diff --git a/web/src/app/Notifier.js b/web/src/app/Notifier.js index f6e47a7c..908df469 100644 --- a/web/src/app/Notifier.js +++ b/web/src/app/Notifier.js @@ -1,5 +1,5 @@ import { playSound, topicDisplayName, topicShortUrl, urlB64ToUint8Array } from "./utils"; -import { toNotificationParams } from "./notificationUtils"; +import { notificationTag, toNotificationParams } from "./notificationUtils"; import prefs from "./Prefs"; import routes from "../components/routes"; @@ -23,21 +23,23 @@ class Notifier { const registration = await this.serviceWorkerRegistration(); await registration.showNotification( ...toNotificationParams({ - subscriptionId: subscription.id, message: notification, defaultTitle, topicRoute: new URL(routes.forSubscription(subscription), window.location.origin).toString(), + baseUrl: subscription.baseUrl, + topic: subscription.topic, }) ); } - async cancel(notification) { + async cancel(subscription, notification) { if (!this.supported()) { return; } try { - const tag = notification.sequence_id || notification.id; - console.log(`[Notifier] Cancelling notification with ${tag}`); + const sequenceId = notification.sequence_id || notification.id; + const tag = notificationTag(subscription.baseUrl, subscription.topic, sequenceId); + console.log(`[Notifier] Cancelling notification with tag ${tag}`); const registration = await this.serviceWorkerRegistration(); const notifications = await registration.getNotifications({ tag }); notifications.forEach((n) => n.close()); diff --git a/web/src/app/notificationUtils.js b/web/src/app/notificationUtils.js index a9b8f8ff..a3025f67 100644 --- a/web/src/app/notificationUtils.js +++ b/web/src/app/notificationUtils.js @@ -50,8 +50,16 @@ export const isImage = (attachment) => { export const icon = "/static/images/ntfy.png"; export const badge = "/static/images/mask-icon.svg"; -export const toNotificationParams = ({ message, defaultTitle, topicRoute }) => { +/** + * Computes a unique notification tag scoped by baseUrl, topic, and sequence ID. + * This ensures notifications from different topics with the same sequence ID don't collide. + */ +export const notificationTag = (baseUrl, topic, sequenceId) => `${baseUrl}/${topic}/${sequenceId}`; + +export const toNotificationParams = ({ message, defaultTitle, topicRoute, baseUrl, topic }) => { const image = isImage(message.attachment) ? message.attachment.url : undefined; + const sequenceId = message.sequence_id || message.id; + const tag = notificationTag(baseUrl, topic, sequenceId); // https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API return [ @@ -62,7 +70,7 @@ export const toNotificationParams = ({ message, defaultTitle, topicRoute }) => { icon, image, timestamp: message.time * 1000, - tag: message.sequence_id || message.id, // Update notification if there is a sequence ID + tag, // Scoped by baseUrl/topic/sequenceId to avoid cross-topic collisions renotify: true, silent: false, // This is used by the notification onclick event diff --git a/web/src/components/hooks.js b/web/src/components/hooks.js index a9268dd2..1b4a78b7 100644 --- a/web/src/components/hooks.js +++ b/web/src/components/hooks.js @@ -51,7 +51,7 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop } }; - const handleNotification = async (subscriptionId, notification) => { + const handleNotification = async (subscription, notification) => { // This logic is (partially) duplicated in // - Android: SubscriberService::onNotificationReceived() // - Android: FirebaseService::onMessageReceived() @@ -59,20 +59,20 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop // - Web app: sw.js:handleMessage(), sw.js:handleMessageClear(), ... if (notification.event === EVENT_MESSAGE_DELETE && notification.sequence_id) { - await subscriptionManager.deleteNotificationBySequenceId(subscriptionId, notification.sequence_id); - await notifier.cancel(notification); + await subscriptionManager.deleteNotificationBySequenceId(subscription.id, notification.sequence_id); + await notifier.cancel(subscription, notification); } else if (notification.event === EVENT_MESSAGE_CLEAR && notification.sequence_id) { - await subscriptionManager.markNotificationReadBySequenceId(subscriptionId, notification.sequence_id); - await notifier.cancel(notification); + await subscriptionManager.markNotificationReadBySequenceId(subscription.id, notification.sequence_id); + await notifier.cancel(subscription, notification); } else { // Regular message: delete existing and add new const sequenceId = notification.sequence_id || notification.id; if (sequenceId) { - await subscriptionManager.deleteNotificationBySequenceId(subscriptionId, sequenceId); + await subscriptionManager.deleteNotificationBySequenceId(subscription.id, sequenceId); } - const added = await subscriptionManager.addNotification(subscriptionId, notification); + const added = await subscriptionManager.addNotification(subscription.id, notification); if (added) { - await subscriptionManager.notify(subscriptionId, notification); + await subscriptionManager.notify(subscription.id, notification); } } }; @@ -89,7 +89,7 @@ export const useConnectionListeners = (account, subscriptions, users, webPushTop if (subscription.internal) { await handleInternalMessage(message); } else { - await handleNotification(subscriptionId, message); + await handleNotification(subscription, message); } };