diff --git a/.changeset/notifications.md b/.changeset/notifications.md new file mode 100644 index 000000000..13b1d7f7a --- /dev/null +++ b/.changeset/notifications.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix notification routing, in-app banners for All Messages rooms, and background audio for loud non-DM rooms. diff --git a/docs/NOTIFICATIONS_FIXES.md b/docs/NOTIFICATIONS_FIXES.md new file mode 100644 index 000000000..0ed5bdf38 --- /dev/null +++ b/docs/NOTIFICATIONS_FIXES.md @@ -0,0 +1,90 @@ +# Notifications & Service Worker Fixes + +This document tracks notification and service worker issues that need to be addressed in `feat/notifications`. + +## Issue #2: SW connection dropout after idle period + +**Problem**: The service worker doesn't seem to stay connected after a period of time, causing notifications to fail. + +**Root Cause**: + +- Service worker becomes inactive after idle period +- Push subscription may expire or lose connection +- Background sync registration not persisting +- SW messaging channel disconnected after tab becomes inactive + +**Proposed Fix**: + +- Implement periodic SW keepalive ping from active tabs +- Re-establish push subscription on SW activation +- Add SW connection health monitoring +- Implement exponential backoff reconnection logic +- Persist notification state in IndexedDB for SW access + +**Implementation Notes**: + +- Add `postMessage` keepalive every 30s from active tab +- Listen for SW `activate` event to restore connections +- Use `navigator.serviceWorker.ready` to ensure registration +- Test with Chrome DevTools → Application → Service Workers → "Update on reload" disabled + +## Issue #5: Media loading failures until hard reset + +**Problem**: Loading media (either media I just sent, or other media), including URL previews, fails until the app is hard reset. + +**Root Cause**: + +- Media authentication tokens not being refreshed/passed correctly +- CORS issues with media URLs after session changes +- Service worker caching stale auth headers +- Blob URL revocation before media loads + +**Proposed Fix**: + +- Implement media auth token refresh mechanism +- Clear SW media cache on auth token changes +- Add retry logic with fresh auth for media requests +- Use stable blob URLs with reference counting +- Add authentication headers to media fetch requests in SW + +**Implementation Notes**: + +- Store media auth tokens in SW cache with expiry +- Listen for Matrix client auth token changes +- Invalidate SW media cache on token refresh +- Add `Authorization` header to fetch requests in SW +- Test with private media enabled homeserver + +## Issue #6: Phantom unread favicon badges/dots + +**Problem**: There are phantom unread favicon badges/dots that don't correspond to actual unread messages. + +**Root Cause**: + +- Unread count calculation includes muted/low-priority rooms +- Favicon update race condition with sync state +- Notification count not clearing after room visit +- Thread notifications counting separately from main timeline + +**Proposed Fix**: + +- Recalculate unread count from room notification state only +- Exclude muted rooms and low-priority notifications +- Clear favicon badge immediately on room focus +- Consolidate thread + main timeline counts correctly +- Add debouncing to favicon updates during rapid sync + +**Implementation Notes**: + +- Use `room.getUnreadNotificationCount()` with proper filters +- Check `room.notificationCounts` and respect notification level +- Update favicon only when total unread count actually changes +- Test with various notification settings (All, Mentions, Muted) + +**Related Files**: + +- Service worker message handling +- Push notification registration +- Favicon update logic +- Media authentication +- Notification badge counting diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index b9d253c6a..06a1597ad 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -75,6 +75,41 @@ function createRoom( } describe('useTimelineSync', () => { + it('reloads event context on TimelineReset when eventId is set', async () => { + const { room, timelineSet } = createRoom(); + const scrollToBottom = vi.fn<() => void>(); + + // mx.getEventTimeline is intentionally absent — loadEventTimeline will + // reject silently (void), leaving the timeline state unchanged. + const { result } = renderHook(() => + useTimelineSync({ + room: room as Room, + mx: { getUserId: () => '@alice:test' } as never, + eventId: '$linked:event', + isAtBottom: false, + isAtBottomRef: { current: false }, + scrollToBottom, + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }) + ); + + const timelineBefore = result.current.timeline.linkedTimelines; + + await act(async () => { + timelineSet.emit(RoomEvent.TimelineReset); + await Promise.resolve(); + }); + + // The live timeline should NOT have replaced the event-context timeline — + // the eventId branch in useLiveTimelineRefresh returns early after calling + // loadEventTimeline and never calls setTimeline with the live timeline. + expect(result.current.timeline.linkedTimelines).toBe(timelineBefore); + expect(scrollToBottom).not.toHaveBeenCalled(); + }); + it('does not snap a non-bottom user to latest after TimelineReset', async () => { const { room, timelineSet, events } = createRoom(); const scrollToBottom = vi.fn<() => void>(); diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index c10b762b8..3d1a247d4 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -63,16 +63,6 @@ const useEventTimelineLoader = ( Sentry.startSpan({ name: 'timeline.jump_load', op: 'matrix.timeline' }, async () => { const jumpLoadStart = performance.now(); - if (!room.getUnfilteredTimelineSet().getTimelineForEvent(eventId)) { - await withTimeout( - mx.roomInitialSync(room.roomId, PAGINATION_LIMIT), - EVENT_TIMELINE_LOAD_TIMEOUT_MS - ); - await withTimeout( - mx.getLatestTimeline(room.getUnfilteredTimelineSet()), - EVENT_TIMELINE_LOAD_TIMEOUT_MS - ); - } const [err, replyEvtTimeline] = await to( withTimeout( mx.getEventTimeline(room.getUnfilteredTimelineSet(), eventId), @@ -523,13 +513,21 @@ export function useTimelineSync({ useLiveTimelineRefresh( room, useCallback(() => { + // If the user arrived via a notification event link, reload that event's + // context rather than dropping back to the live timeline — otherwise a + // sync gap (TimelineReset / TimelineRefresh) would silently discard the + // loaded context and the notification event would no longer be visible. + if (eventId) { + void loadEventTimeline(eventId); + return; + } const wasAtBottom = isAtBottomRef.current; resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); if (wasAtBottom) { scrollToBottom('instant'); } - }, [room, isAtBottomRef, scrollToBottom]) + }, [eventId, loadEventTimeline, room, isAtBottomRef, scrollToBottom]) ); useRelationUpdate( diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index a03342fc5..531079405 100644 --- a/src/app/hooks/useNotificationJumper.ts +++ b/src/app/hooks/useNotificationJumper.ts @@ -8,15 +8,13 @@ import { useSyncState } from './useSyncState'; import { useMatrixClient } from './useMatrixClient'; import { getCanonicalAliasOrRoomId } from '../utils/matrix'; import { getDirectRoomPath, getHomeRoomPath, getSpaceRoomPath } from '../pages/pathUtils'; -import { getOrphanParents, guessPerfectParent } from '../utils/room'; -import { roomToParentsAtom } from '../state/room/roomToParents'; +import { getOrphanParents, getRoomToParents, guessPerfectParent } from '../utils/room'; import { createLogger } from '../utils/debug'; export function NotificationJumper() { const [pending, setPending] = useAtom(pendingNotificationAtom); const activeSessionId = useAtomValue(activeSessionIdAtom); const mDirects = useAtomValue(mDirectAtom); - const roomToParents = useAtomValue(roomToParentsAtom); const mx = useMatrixClient(); const navigate = useNavigate(); const log = createLogger('NotificationJumper'); @@ -66,7 +64,7 @@ export function NotificationJumper() { // Use getOrphanParents + guessPerfectParent (same as useRoomNavigate) so // we always navigate to a root-level space, not a subspace — subspace // paths are not recognised by the router and land on JoinBeforeNavigate. - const orphanParents = getOrphanParents(roomToParents, pending.roomId); + const orphanParents = getOrphanParents(getRoomToParents(mx), pending.roomId); if (orphanParents.length > 0) { const parentSpace = guessPerfectParent(mx, pending.roomId, orphanParents) ?? orphanParents[0]; @@ -90,7 +88,7 @@ export function NotificationJumper() { membership: room?.getMyMembership(), }); } - }, [pending, activeSessionId, mx, mDirects, roomToParents, navigate, setPending, log]); + }, [pending, activeSessionId, mx, mDirects, navigate, setPending, log]); // Reset the guard only when pending is replaced (new notification or cleared). useEffect(() => { diff --git a/src/app/pages/client/BackgroundNotifications.tsx b/src/app/pages/client/BackgroundNotifications.tsx index e7fdff6d7..22b1f8486 100644 --- a/src/app/pages/client/BackgroundNotifications.tsx +++ b/src/app/pages/client/BackgroundNotifications.tsx @@ -381,7 +381,11 @@ export function BackgroundNotifications() { // For "Mention & Keywords": respect the push rule (only notify if it matches). const shouldForceDMNotification = isDM && notificationType !== NotificationType.MentionsAndKeywords; - const shouldNotify = pushActions?.notify || shouldForceDMNotification; + // For rooms explicitly set to "All Messages": force-notify, mirroring the DM bypass. + const shouldForceRoomLoudNotification = + !isDM && notificationType === NotificationType.AllMessages; + const shouldNotify = + pushActions?.notify || shouldForceDMNotification || shouldForceRoomLoudNotification; if (!shouldNotify) { debugLog.debug('notification', 'Event filtered - no push action match', { @@ -395,6 +399,9 @@ export function BackgroundNotifications() { const loudByRule = Boolean(pushActions.tweaks?.sound); const isHighlight = Boolean(pushActions.tweaks?.highlight); + // Treat DMs and "All Messages" rooms as inherently loud when the push rule lacks a + // sound tweak (common with sliding sync where room_member_count conditions fail). + const isLoud = loudByRule || isDM || shouldForceRoomLoudNotification; debugLog.info('notification', 'Processing notification event', { eventId, @@ -402,7 +409,7 @@ export function BackgroundNotifications() { eventType, isDM, isHighlight, - loud: loudByRule, + loud: isLoud, }); const senderName = @@ -429,7 +436,7 @@ export function BackgroundNotifications() { }); // Silent-rule events: unread badge updated above; no OS notification or sound. - if (!loudByRule && !isHighlight) { + if (!isLoud && !isHighlight) { debugLog.debug('notification', 'Silent notification - badge updated only', { eventId, roomId: room.roomId, @@ -458,8 +465,8 @@ export function BackgroundNotifications() { showMessageContent: showMessageContentRef.current, showEncryptedMessageContent: showEncryptedMessageContentRef.current, }), - // Play sound only if the push rule requests it and the user has sounds enabled. - silent: !notificationSoundRef.current || !loudByRule, + // Play sound only if the event is loud and the user has sounds enabled. + silent: !notificationSoundRef.current || !isLoud, eventId, data: { type: mEvent.getType(), @@ -502,9 +509,9 @@ export function BackgroundNotifications() { icon: notificationPayload.options.icon, onClick: notifOnClick, }); - } else if (loudByRule) { - // App is backgrounded or in-app notifications disabled — fire an OS notification. - // Only send for loud (sound-tweak) rules; highlight-only events are silently counted. + } else if (isLoud) { + // App is backgrounded or in-app notifications disabled — fire an OS notification. + // Send for loud events (includes DMs and rooms set to "All Messages"). debugLog.info('notification', 'Sending OS notification', { eventId, roomId: room.roomId, diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index f847e0856..0838ca447 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -197,7 +197,7 @@ function InviteNotifications() { const playSound = useCallback(() => { const audioElement = audioRef.current; - audioElement?.play(); + audioElement?.play().catch(() => {}); clearMediaSessionQuickly(); }, []); @@ -271,7 +271,7 @@ function MessageNotifications() { const playSound = useCallback(() => { const audioElement = audioRef.current; - audioElement?.play(); + audioElement?.play().catch(() => {}); clearMediaSessionQuickly(); }, []); @@ -381,7 +381,13 @@ function MessageNotifications() { // For "Mention & Keywords": respect the push rule (only notify if it matches). const shouldForceDMNotification = isDM && notificationType !== NotificationType.MentionsAndKeywords; - const shouldNotify = pushActions?.notify || shouldForceDMNotification; + // For rooms explicitly set to "All Messages": also force-notify, mirroring the DM + // bypass above. Push-rule evaluation can silently return notify=false when the + // room-specific rule was written by another client with a different action format. + const shouldForceRoomLoudNotification = + !isDM && notificationType === NotificationType.AllMessages; + const shouldNotify = + pushActions?.notify || shouldForceDMNotification || shouldForceRoomLoudNotification; // If we shouldn't notify based on rules/settings, skip everything if (!shouldNotify) return; @@ -395,7 +401,10 @@ function MessageNotifications() { // messages fall through to .m.rule.message which carries no sound tweak — // leaving loudByRule=false. Treat known DMs as inherently loud so that // the OS notification and badge are consistent with the DM context. - const isLoud = loudByRule || isDM; + // Similarly, rooms explicitly set to "All Messages" are treated as loud + // even when the room-specific push rule was written by another client + // without a sound tweak, or when push-rule evaluation fails silently. + const isLoud = loudByRule || isDM || shouldForceRoomLoudNotification; // Record as notified to prevent duplicate banners (e.g. re-emitted decrypted events). notifiedEventsRef.current.add(eventId); @@ -452,13 +461,17 @@ function MessageNotifications() { } } - // Everything below requires the page to be visible (in-app UI + audio). + // In-app audio plays regardless of tab visibility — sound doesn't require the page to be visible. + if (notificationSound && isLoud) { + playSound(); + } + + // Everything below requires the page to be visible (in-app UI only). if (document.visibilityState !== 'visible') return; // Page is visible — show the themed in-app notification banner. - // For non-DM rooms, only show banner for highlighted messages (mentions/keywords). - // For DMs, show banner for all messages. - if (showNotifications && (isHighlightByRule || isDM)) { + // Show for DMs, highlighted messages (mentions/keywords), or any loud push-rule match. + if (showNotifications && (isHighlightByRule || isDM || isLoud)) { const avatarMxc = room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl(); const roomAvatar = avatarMxc @@ -530,10 +543,7 @@ function MessageNotifications() { }); } - // In-app audio: play when notification sounds are enabled AND this notification is loud. - if (notificationSound && isLoud) { - playSound(); - } + // (Audio is handled above the visibility gate.) }; mx.on(RoomEvent.Timeline, handleTimelineEvent); return () => { diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index e15630c79..17a61eeef 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -209,7 +209,8 @@ export const getNotificationType = (mx: MatrixClient, roomId: string): Notificat return NotificationType.Default; } - if ((roomPushRule.actions[0] as string) === 'notify') return NotificationType.AllMessages; + if ((roomPushRule.actions as string[]).some((a) => a === 'notify')) + return NotificationType.AllMessages; return NotificationType.MentionsAndKeywords; }; @@ -312,6 +313,23 @@ export const getUnreadInfo = (room: Room, options?: UnreadInfoOptions): UnreadIn } } + // If the user's own message is the most recent event in the live timeline they + // implicitly read everything before it when they composed that reply. Return zero + // to suppress phantom unread badges that arise from stale SDK counters in sliding + // sync when no explicit read receipt is present. + if (userId && !room.getEventReadUpTo(userId)) { + const liveEvents = room.getLiveTimeline().getEvents(); + const latestEvent = liveEvents[liveEvents.length - 1]; + if ( + latestEvent && + !latestEvent.isSending() && + latestEvent.getSender() === userId && + isNotificationEvent(latestEvent) + ) { + return { roomId: room.roomId, highlight: 0, total: 0 }; + } + } + let total = room.getUnreadNotificationCount(NotificationCountType.Total); const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight);