Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/notifications.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
default: patch
---

Fix notification routing, in-app banners for All Messages rooms, and background audio for loud non-DM rooms.
90 changes: 90 additions & 0 deletions docs/NOTIFICATIONS_FIXES.md
Original file line number Diff line number Diff line change
@@ -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
35 changes: 35 additions & 0 deletions src/app/hooks/timeline/useTimelineSync.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
20 changes: 9 additions & 11 deletions src/app/hooks/timeline/useTimelineSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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) {
Comment on lines +516 to +520
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(
Expand Down
8 changes: 3 additions & 5 deletions src/app/hooks/useNotificationJumper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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];
Expand All @@ -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(() => {
Expand Down
23 changes: 15 additions & 8 deletions src/app/pages/client/BackgroundNotifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +384 to +388

if (!shouldNotify) {
debugLog.debug('notification', 'Event filtered - no push action match', {
Expand All @@ -395,14 +399,17 @@ 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,
roomId: room.roomId,
eventType,
isDM,
isHighlight,
loud: loudByRule,
loud: isLoud,
});

const senderName =
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 22 additions & 12 deletions src/app/pages/client/ClientNonUIFeatures.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ function InviteNotifications() {

const playSound = useCallback(() => {
const audioElement = audioRef.current;
audioElement?.play();
audioElement?.play().catch(() => {});
clearMediaSessionQuickly();
}, []);

Expand Down Expand Up @@ -271,7 +271,7 @@ function MessageNotifications() {

const playSound = useCallback(() => {
const audioElement = audioRef.current;
audioElement?.play();
audioElement?.play().catch(() => {});
clearMediaSessionQuickly();
}, []);

Expand Down Expand Up @@ -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;
Comment on lines +385 to +388
const shouldNotify =
pushActions?.notify || shouldForceDMNotification || shouldForceRoomLoudNotification;

// If we shouldn't notify based on rules/settings, skip everything
if (!shouldNotify) return;
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 () => {
Expand Down
19 changes: 18 additions & 1 deletion src/app/utils/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ 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;
};

Expand Down Expand Up @@ -312,6 +312,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 };
Comment on lines +319 to +328
}
}

let total = room.getUnreadNotificationCount(NotificationCountType.Total);
const highlight = room.getUnreadNotificationCount(NotificationCountType.Highlight);

Expand Down
Loading