Skip to content

Commit ec938e2

Browse files
author
Evie Gauthier
committed
fix(notifications): suppress OS notification when app is focused; wrap in try/catch
Two bugs caused in-app notifications to silently vanish on desktop when the window was focused: 1. window.Notification() was called unconditionally even when the window had focus. In some browsers/environments (certain Chrome versions, Electron, macOS DnD mode) calling the Notification constructor throws or is silently rejected when the tab is active. Without a try/catch the uncaught exception aborted the entire RoomEvent.Timeline handler before setInAppBanner was ever reached — so neither the OS notification NOR the in-app banner appeared. InviteNotifications already used try/catch for the same reason; MessageNotifications did not. 2. The OS notification was semantically wrong when focused: the in-app banner is the correct alert while the window is active. Added a !document.hasFocus() guard to match the comment's stated intent ("alert when minimised or tab not active") and to prevent a duplicate OS+in-app banner when another window is simply on top. Mobile is unaffected: mobileOrTablet() already gates the OS block. The in-app banner path (visibilityState === 'visible' → setInAppBanner) is unchanged and continues to fire correctly on both desktop (focused) and mobile (app visible in browser).
1 parent fa01529 commit ec938e2

1 file changed

Lines changed: 47 additions & 32 deletions

File tree

src/app/pages/client/ClientNonUIFeatures.tsx

Lines changed: 47 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -322,38 +322,53 @@ function MessageNotifications() {
322322
if (first) notifiedEventsRef.current.delete(first);
323323
}
324324

325-
// On desktop: fire an OS notification so the user is alerted even when the
326-
// browser window is minimised or the tab is not active.
327-
if (!mobileOrTablet() && showSystemNotifications && notificationPermission('granted')) {
328-
const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption);
329-
const avatarMxc =
330-
room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl();
331-
const osPayload = buildRoomMessageNotification({
332-
roomName: room.name ?? 'Unknown',
333-
roomAvatar: avatarMxc
334-
? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined)
335-
: undefined,
336-
username:
337-
getMemberDisplayName(room, sender, nicknamesRef.current) ??
338-
getMxIdLocalPart(sender) ??
339-
sender,
340-
previewText: resolveNotificationPreviewText({
341-
content: mEvent.getContent(),
342-
eventType: mEvent.getType(),
343-
isEncryptedRoom,
344-
showMessageContent,
345-
showEncryptedMessageContent,
346-
}),
347-
silent: !notificationSound || !isLoud,
348-
eventId,
349-
});
350-
const noti = new window.Notification(osPayload.title, osPayload.options);
351-
const { roomId } = room;
352-
noti.onclick = () => {
353-
window.focus();
354-
setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined });
355-
noti.close();
356-
};
325+
// On desktop: fire an OS notification when the window is not focused so
326+
// the user is alerted while the browser is minimised or the tab is in the
327+
// background. When the window IS focused the in-app banner (below) is the
328+
// appropriate alert — we skip the OS notification to avoid a duplicate.
329+
// The whole block is wrapped in try/catch: window.Notification() can throw
330+
// in sandboxed environments, browsers with DnD active, or Electron — and
331+
// an uncaught exception here would abort the handler before setInAppBanner
332+
// is reached, causing in-app notifications to silently vanish too.
333+
if (
334+
!mobileOrTablet() &&
335+
!document.hasFocus() &&
336+
showSystemNotifications &&
337+
notificationPermission('granted')
338+
) {
339+
try {
340+
const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption);
341+
const avatarMxc =
342+
room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl();
343+
const osPayload = buildRoomMessageNotification({
344+
roomName: room.name ?? 'Unknown',
345+
roomAvatar: avatarMxc
346+
? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined)
347+
: undefined,
348+
username:
349+
getMemberDisplayName(room, sender, nicknamesRef.current) ??
350+
getMxIdLocalPart(sender) ??
351+
sender,
352+
previewText: resolveNotificationPreviewText({
353+
content: mEvent.getContent(),
354+
eventType: mEvent.getType(),
355+
isEncryptedRoom,
356+
showMessageContent,
357+
showEncryptedMessageContent,
358+
}),
359+
silent: !notificationSound || !isLoud,
360+
eventId,
361+
});
362+
const noti = new window.Notification(osPayload.title, osPayload.options);
363+
const { roomId } = room;
364+
noti.onclick = () => {
365+
window.focus();
366+
setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined });
367+
noti.close();
368+
};
369+
} catch {
370+
// window.Notification unavailable or blocked (sandboxed context, DnD, etc.)
371+
}
357372
}
358373

359374
// Everything below requires the page to be visible (in-app UI + audio).

0 commit comments

Comments
 (0)