diff --git a/.changeset/fix-notification-delivery-bugs.md b/.changeset/fix-notification-delivery-bugs.md new file mode 100644 index 000000000..6adb0ec0e --- /dev/null +++ b/.changeset/fix-notification-delivery-bugs.md @@ -0,0 +1,5 @@ +--- +'sable': patch +--- + +fix: notification delivery bugs diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index 6f7c77689..43c358317 100644 --- a/src/app/hooks/useNotificationJumper.ts +++ b/src/app/hooks/useNotificationJumper.ts @@ -8,6 +8,7 @@ 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 { createLogger } from '../utils/debug'; @@ -62,12 +63,16 @@ export function NotificationJumper() { // If the room lives inside a space, route through the space path so // SpaceRouteRoomProvider can resolve it — HomeRouteRoomProvider only // knows orphan rooms and would show JoinBeforeNavigate otherwise. - const parents = roomToParents.get(pending.roomId); - const firstParentId = parents && [...parents][0]; - if (firstParentId) { + // 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); + if (orphanParents.length > 0) { + const parentSpace = + guessPerfectParent(mx, pending.roomId, orphanParents) ?? orphanParents[0]; navigate( getSpaceRoomPath( - getCanonicalAliasOrRoomId(mx, firstParentId), + getCanonicalAliasOrRoomId(mx, parentSpace), roomIdOrAlias, pending.eventId ) diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index 56c333065..f5a948013 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -40,6 +40,19 @@ import { mobileOrTablet } from '$utils/user-agent'; import { getInboxInvitesPath } from '../pathUtils'; import { BackgroundNotifications } from './BackgroundNotifications'; +function clearMediaSessionQuickly(): void { + if (!('mediaSession' in navigator)) return; + // iOS registers the lock screen media player as a side-effect of + // HTMLAudioElement.play(). We delay slightly so iOS has finished updating + // the media session before we clear it — clearing too early is a no-op. + // We only clear if no real in-app media (video/audio in a room) has since + // registered meaningful metadata; if it has, leave it alone. + setTimeout(() => { + if (navigator.mediaSession.metadata !== null) return; + navigator.mediaSession.playbackState = 'none'; + }, 500); +} + function SystemEmojiFeature() { const [twitterEmoji] = useSetting(settingsAtom, 'twitterEmoji'); @@ -157,6 +170,7 @@ function InviteNotifications() { const playSound = useCallback(() => { const audioElement = audioRef.current; audioElement?.play(); + clearMediaSessionQuickly(); }, []); useEffect(() => { @@ -229,6 +243,7 @@ function MessageNotifications() { const playSound = useCallback(() => { const audioElement = audioRef.current; audioElement?.play(); + clearMediaSessionQuickly(); }, []); useEffect(() => { @@ -291,8 +306,13 @@ function MessageNotifications() { // If neither a loud nor a highlight rule matches, and it's not a DM, nothing to show. if (!isHighlightByRule && !loudByRule && !isDM) return; - // Page hidden: SW (push) handles the OS notification. Nothing to do in-app. - if (document.visibilityState !== 'visible') return; + // With sliding sync we only load m.room.member/$ME in required_state, so + // PushProcessor cannot evaluate the room_member_count == 2 condition on + // .m.rule.room_one_to_one. That rule therefore fails to match, and DM + // 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; // Record as notified to prevent duplicate banners (e.g. re-emitted decrypted events). notifiedEventsRef.current.add(eventId); @@ -301,11 +321,55 @@ function MessageNotifications() { if (first) notifiedEventsRef.current.delete(first); } - // Page is visible — show the themed in-app notification banner for any - // highlighted message (mention / keyword) or loud push rule. - // okay fast patch because that showNotifications setting atom is not getting set correctly or something - if (mobileOrTablet() && showNotifications && (isHighlightByRule || loudByRule || isDM)) { - const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption); + // On desktop: fire an OS notification whenever system notifications are + // enabled and permission is granted — regardless of whether the window is + // focused. When the window is also visible the in-app banner fires too, + // mirroring the behaviour of apps like Discord. + // The whole block is wrapped in try/catch: window.Notification() can throw + // in sandboxed environments, browsers with DnD active, or Electron — and + // an uncaught exception here would abort the handler before setInAppBanner + // is reached, causing in-app notifications to silently vanish too. + if (!mobileOrTablet() && showSystemNotifications && notificationPermission('granted')) { + try { + const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption); + const avatarMxc = + room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl(); + const osPayload = buildRoomMessageNotification({ + roomName: room.name ?? 'Unknown', + roomAvatar: avatarMxc + ? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined) + : undefined, + username: + getMemberDisplayName(room, sender, nicknamesRef.current) ?? + getMxIdLocalPart(sender) ?? + sender, + previewText: resolveNotificationPreviewText({ + content: mEvent.getContent(), + eventType: mEvent.getType(), + isEncryptedRoom, + showMessageContent, + showEncryptedMessageContent, + }), + silent: !notificationSound || !isLoud, + eventId, + }); + const noti = new window.Notification(osPayload.title, osPayload.options); + const { roomId } = room; + noti.onclick = () => { + window.focus(); + setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined }); + noti.close(); + }; + } catch { + // window.Notification unavailable or blocked (sandboxed context, DnD, etc.) + } + } + + // Everything below requires the page to be visible (in-app UI + audio). + if (document.visibilityState !== 'visible') return; + + // Page is visible — show the themed in-app notification banner. + if (showNotifications && (isHighlightByRule || loudByRule || isDM)) { const avatarMxc = room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl(); const roomAvatar = avatarMxc @@ -316,21 +380,22 @@ function MessageNotifications() { getMxIdLocalPart(sender) ?? sender; const content = mEvent.getContent(); + // Events reaching here are already decrypted (m.room.encrypted is skipped + // above). Pass isEncryptedRoom:false so the preview always shows the actual + // message body when showMessageContent is enabled. const previewText = resolveNotificationPreviewText({ content: mEvent.getContent(), eventType: mEvent.getType(), - isEncryptedRoom, + isEncryptedRoom: false, showMessageContent, showEncryptedMessageContent, }); // Build a rich ReactNode body using the same HTML parser as the room - // timeline — this gives full mxc image transforms, mention pills, - // linkify, spoilers, code blocks, etc. + // timeline — mxc images, mention pills, linkify, spoilers, code blocks. let bodyNode: ReactNode; if ( showMessageContent && - (!isEncryptedRoom || showEncryptedMessageContent) && content.format === 'org.matrix.custom.html' && content.formatted_body ) { @@ -347,7 +412,7 @@ function MessageNotifications() { roomAvatar, username: resolvedSenderName, previewText, - silent: !notificationSound, + silent: !notificationSound || !isLoud, eventId, }); const { roomId } = room; @@ -371,45 +436,8 @@ function MessageNotifications() { }); } - // On desktop: also fire an OS notification so the user is alerted even - // if the browser window is minimised (respects System Notifications toggle). - if (!mobileOrTablet() && showSystemNotifications && notificationPermission('granted')) { - const isEncryptedRoom = !!getStateEvent(room, StateEvent.RoomEncryption); - const avatarMxc = - room.getAvatarFallbackMember()?.getMxcAvatarUrl() ?? room.getMxcAvatarUrl(); - const osPayload = buildRoomMessageNotification({ - roomName: room.name ?? 'Unknown', - roomAvatar: avatarMxc - ? (mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined) - : undefined, - username: - getMemberDisplayName(room, sender, nicknamesRef.current) ?? - getMxIdLocalPart(sender) ?? - sender, - previewText: resolveNotificationPreviewText({ - content: mEvent.getContent(), - eventType: mEvent.getType(), - isEncryptedRoom, - showMessageContent, - showEncryptedMessageContent, - }), - // Play sound only if the push rule requests it and the user has sounds enabled. - silent: !notificationSound || !loudByRule, - eventId, - }); - const noti = new window.Notification(osPayload.title, osPayload.options); - const { roomId } = room; - noti.onclick = () => { - window.focus(); - setPending({ roomId, eventId, targetSessionId: mx.getUserId() ?? undefined }); - noti.close(); - }; - } - // In-app audio: play whenever notification sounds are enabled. - // Not gated on loudByRule — that only controls the OS-level silent flag. - // Audio API requires a visible, focused document; skip when hidden. - if (document.visibilityState === 'visible' && notificationSound) { + if (notificationSound) { playSound(); } }; @@ -497,8 +525,6 @@ export function HandleNotificationClick() { } function SyncNotificationSettingsWithServiceWorker() { - const [notificationSound] = useSetting(settingsAtom, 'isNotificationSounds'); - const [usePushNotifications] = useSetting(settingsAtom, 'usePushNotifications'); const [showMessageContent] = useSetting(settingsAtom, 'showMessageContentInNotifications'); const [showEncryptedMessageContent] = useSetting( settingsAtom, @@ -524,9 +550,11 @@ function SyncNotificationSettingsWithServiceWorker() { useEffect(() => { if (!('serviceWorker' in navigator)) return; + // notificationSoundEnabled is intentionally excluded: push notification sound + // is governed by the push rule's tweakSound alone (OS/Sygnal handles it). + // The in-app sound setting only controls the in-page