Merge branch 'fix/notification-delivery-bugs' (PR 7w1#252)

Author: Evie Gauthier

.changeset/fix-notification-delivery-bugs.md

@@ -0,0 +1,11 @@
+---
+sable: patch
+---
+
+fix: notification delivery bugs
+
+- **Push notifications always silent** — `resolveSilent` in the service worker now always returns `false`, leaving sound/vibration decisions entirely to the OS and Sygnal push gateway. The in-app sound setting no longer affects push sound.
+- **In-app banner showing "sent an encrypted message"** — Events reaching the banner are already decrypted by the SDK. `isEncryptedRoom: false` is now passed so the actual message body is always shown when message content preview is enabled.
+- **Desktop OS notifications not firing when page is hidden** — The OS notification block now runs before the `visibilityState !== 'visible'` guard, which only gates the in-app banner and audio. Notifications now fire even when the browser window is minimised.
+- **iOS lock screen media player after notification sound** — `mediaSession.playbackState` is cleared after a short delay following `play()`, dismissing the lock screen widget. If in-app media has since registered its own metadata, the session is left untouched.
+- **In-app banner not appearing on desktop** — The banner was gated behind a `mobileOrTablet()` check; it now fires on all platforms when In-App Notifications is enabled.

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,21 +306,52 @@ 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;
-
       // Record as notified to prevent duplicate banners (e.g. re-emitted decrypted events).
       notifiedEventsRef.current.add(eventId);
       if (notifiedEventsRef.current.size > 200) {
         const first = notifiedEventsRef.current.values().next().value;
         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)) {
+      // On desktop: fire an OS notification so the user is alerted even when the
+      // browser window is minimised or the tab is not active.
+      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,
+          }),
+          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();
+        };
+      }
+
+      // 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 +362,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 +394,7 @@ function MessageNotifications() {
           roomAvatar,
           username: resolvedSenderName,
           previewText,
-          silent: !notificationSound,
+          silent: !notificationSound || !loudByRule,
           eventId,
         });
         const { roomId } = room;
@@ -371,45 +418,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 +507,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 +532,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 <audio> playback above.
     const payload = {
       type: 'setNotificationSettings' as const,
-      notificationSoundEnabled: notificationSound,
       showMessageContent,
       showEncryptedMessageContent,
       clearNotificationsOnRead,
@@ -536,13 +546,7 @@ function SyncNotificationSettingsWithServiceWorker() {
     navigator.serviceWorker.ready.then((registration) => {
       registration.active?.postMessage(payload);
     });
-  }, [
-    notificationSound,
-    usePushNotifications,
-    showMessageContent,
-    showEncryptedMessageContent,
-    clearNotificationsOnRead,
-  ]);
+  }, [showMessageContent, showEncryptedMessageContent, clearNotificationsOnRead]);
 
   return null;
 }

src/sw.ts

@@ -16,7 +16,6 @@ let showMessageContent = false;
 let showEncryptedMessageContent = false;
 let clearNotificationsOnRead = false;
 const { handlePushNotificationPushData } = createPushNotifications(self, () => ({
-  notificationSoundEnabled,
   showMessageContent,
   showEncryptedMessageContent,
 }));

src/sw/pushNotification.ts

@@ -7,7 +7,6 @@ import {
 } from '../app/utils/notificationStyle';
 
 type NotificationSettings = {
-  notificationSoundEnabled: boolean;
   showMessageContent: boolean;
   showEncryptedMessageContent: boolean;
 };
@@ -16,13 +15,10 @@ export const createPushNotifications = (
   self: ServiceWorkerGlobalScope,
   getNotificationSettings: () => NotificationSettings
 ) => {
-  const resolveSilent = (silent: unknown, tweakSound?: unknown): boolean => {
-    if (typeof silent === 'boolean') return silent;
-    // If the push rule doesn't request a sound tweak, the notification should be silent
-    // (no sound), regardless of the user's global sound preference.
-    if (!tweakSound) return true;
-    return !getNotificationSettings().notificationSoundEnabled;
-  };
+  // Push notification sound is always controlled by the OS/device settings.
+  // We never explicitly silence push notifications — the user's device notification
+  // preferences (volume, Do Not Disturb, per-app settings) handle that instead.
+  const resolveSilent = (): boolean => false;
 
   const showNotificationWithData = async (
     title: string,
@@ -73,7 +69,7 @@ export const createPushNotifications = (
         showMessageContent: getNotificationSettings().showMessageContent,
         showEncryptedMessageContent: getNotificationSettings().showEncryptedMessageContent,
       }),
-      silent: resolveSilent(pushData?.silent, pushData?.tweaks?.sound),
+      silent: resolveSilent(),
       eventId: pushData?.event_id,
       recipientId: typeof pushData?.user_id === 'string' ? pushData.user_id : undefined,
       data,
@@ -108,7 +104,7 @@ export const createPushNotifications = (
         showMessageContent: getNotificationSettings().showMessageContent,
         showEncryptedMessageContent: getNotificationSettings().showEncryptedMessageContent,
       }),
-      silent: resolveSilent(pushData?.silent, pushData?.tweaks?.sound),
+      silent: resolveSilent(),
       eventId: pushData?.event_id,
       recipientId: typeof pushData?.user_id === 'string' ? pushData.user_id : undefined,
       data,
@@ -141,7 +137,7 @@ export const createPushNotifications = (
       ...pushData.data,
     };
 
-    await showNotificationWithData('New Invitation', body, data, resolveSilent(pushData?.silent));
+    await showNotificationWithData('New Invitation', body, data, resolveSilent());
   };
 
   const handlePushNotificationPushData = async (pushData: any) => {