feat(notification-processing): [PM-19877] System Notification Implementation - Making final touchups.

2026-02-06 11:43:51 +00:00 · 2025-07-21 15:25:54 -04:00
parent 50621218d4
commit 5d01cd4309
4 changed files with 103 additions and 105 deletions
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -130,10 +130,7 @@ import {
  WebPushNotificationsApiService,
  WorkerWebPushConnectionService,
 } from "@bitwarden/common/platform/notifications/internal";
-import {
-  ButtonActions,
-  SystemNotificationsService,
-} from "@bitwarden/common/platform/notifications/system-notifications-service";
+import { SystemNotificationsService } from "@bitwarden/common/platform/notifications/system-notifications-service";
 import { UnsupportedSystemNotificationsService } from "@bitwarden/common/platform/notifications/unsupported-system-notifications.service";
 import { AppIdService } from "@bitwarden/common/platform/services/app-id.service";
 import { ConfigApiService } from "@bitwarden/common/platform/services/config/config-api.service";
@@ -453,7 +450,6 @@ export default class MainBackground {
  private webRequestBackground: WebRequestBackground;

  private syncTimeout: any;
-  private isSafari: boolean;
  private nativeMessagingBackground: NativeMessagingBackground;

  private popupViewCacheBackgroundService: PopupViewCacheBackgroundService;
@@ -1132,22 +1128,11 @@ export default class MainBackground {
      this.webPushConnectionService = new UnsupportedWebPushConnectionService();
    }

-    this.logService.info(
-      `The background service is registered as ${navigator.userAgent} with manifest version ${BrowserApi.manifestVersion}`,
-    );
-
    this.actionsService = new BrowserActionsService(this.logService, this.platformUtilsService);

-    setTimeout(async () => {
-      await this.actionsService.openPopup();
-    }, 1);
-
-    const userAgent = navigator.userAgent;
-
-    const isChrome =
-      userAgent.includes("Chrome") || userAgent.includes("Edge") || userAgent.includes("OPR");
-    const isSafari = userAgent.includes("Safari") && !userAgent.includes("Chrome");
-    const isFirefox = userAgent.includes("Firefox");
+    const isChrome = this.platformUtilsService.isChrome();
+    const isSafari = this.platformUtilsService.isSafari();
+    const isFirefox = this.platformUtilsService.isFirefox();

    if ((isChrome || isFirefox) && !isSafari) {
      this.systemNotificationService = new BrowserSystemNotificationService(
@@ -1158,18 +1143,6 @@ export default class MainBackground {
      this.systemNotificationService = new UnsupportedSystemNotificationsService();
    }

-    setTimeout(async () => {
-      this.logService.info("CREATING NOTIFICATION");
-      await this.systemNotificationService.create({
-        id: Math.random() * 100000 + "",
-        type: ButtonActions.AuthRequestNotification,
-        title: "Test Notification",
-        body: "Body",
-        buttons: [],
-      });
-      this.logService.info("DONE CREATING NOTIFICATION");
-    }, 1);
-
    this.notificationsService = new DefaultNotificationsService(
      this.logService,
      this.syncService,
@@ -1226,9 +1199,6 @@ export default class MainBackground {
      this.logService,
    );

-    // Other fields
-    this.isSafari = this.platformUtilsService.isSafari();
-
    // Background

    this.fido2Background = new Fido2Background(
@@ -1753,7 +1723,7 @@ export default class MainBackground {
      return;
    }

-    if (this.isSafari) {
+    if (this.platformUtilsService.isSafari()) {
      await SafariApp.sendMessageToApp("showPopover", null, true);
    }
  }
--- a/apps/browser/src/platform/actions/browser-actions.service.ts
+++ b/apps/browser/src/platform/actions/browser-actions.service.ts
@@ -15,30 +15,36 @@ export class BrowserActionsService implements ActionsService {
  async openPopup(): Promise<void> {
    const deviceType = this.platformUtilsService.getDevice();

-    switch (deviceType) {
-      case DeviceType.FirefoxExtension:
-      case DeviceType.ChromeExtension: {
-        const browserAction = BrowserApi.getBrowserAction();
+    try {
+      switch (deviceType) {
+        case DeviceType.FirefoxExtension:
+        case DeviceType.ChromeExtension: {
+          const browserAction = BrowserApi.getBrowserAction();

-        // We might get back mv2 or mv3 browserAction, only mv3 supports the openPopup function,
-        // so check for that function existing.
-        if ("openPopup" in browserAction && typeof browserAction.openPopup === "function") {
-          await browserAction.openPopup();
-          return;
-        } else {
-          this.logService.warning(
-            `No openPopup function found on browser actions. On browser: ${deviceType} and manifest version: ${BrowserApi.manifestVersion}`,
-          );
+          // We might get back mv2 or mv3 browserAction, only mv3 supports the openPopup function,
+          // so check for that function existing.
+          if ("openPopup" in browserAction && typeof browserAction.openPopup === "function") {
+            await browserAction.openPopup();
+            return;
+          } else {
+            this.logService.warning(
+              `No openPopup function found on browser actions. On browser: ${deviceType} and manifest version: ${BrowserApi.manifestVersion}`,
+            );
+          }
+          break;
        }
-        break;
+        case DeviceType.SafariExtension:
+          await SafariApp.sendMessageToApp("showPopover", null, true);
+          return;
+        default:
+          this.logService.warning(
+            `Tried to open the popup from an unsupported device type: ${deviceType}`,
+          );
      }
-      case DeviceType.SafariExtension:
-        await SafariApp.sendMessageToApp("showPopover", null, true);
-        return;
-      default:
-        this.logService.warning(
-          `Tried to open the popup from an unsupported device type: ${deviceType}`,
-        );
+    } catch (e) {
+      this.logService.error(
+        `Failed to open the popup on ${deviceType} with manifest ${BrowserApi.manifestVersion} and error: ${e}`,
+      );
    }
  }
 }
--- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts
+++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts
@@ -23,68 +23,74 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ
  }

  async create(createInfo: SystemNotificationCreateInfo): Promise<undefined> {
-    switch (this.platformUtilsService.getDevice()) {
-      case DeviceType.ChromeExtension:
-        chrome.notifications.create(createInfo.id, {
-          iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200",
-          message: createInfo.title,
-          type: "basic",
-          title: createInfo.title,
-          buttons: createInfo.buttons.map((value) => {
-            return { title: value.title };
-          }),
-        });
+    try {
+      switch (this.platformUtilsService.getDevice()) {
+        case DeviceType.ChromeExtension:
+          chrome.notifications.create(createInfo.id, {
+            iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200",
+            message: createInfo.title,
+            type: "basic",
+            title: createInfo.title,
+            buttons: createInfo.buttons.map((value) => {
+              return { title: value.title };
+            }),
+          });

-        // ESLint: Using addListener in the browser popup produces a memory leak in Safari,
-        // use `BrowserApi. addListener` instead (no-restricted-syntax)
-        // eslint-disable-next-line no-restricted-syntax
-        chrome.notifications.onButtonClicked.addListener(
-          (notificationId: string, buttonIndex: number) => {
+          // ESLint: Using addListener in the browser popup produces a memory leak in Safari,
+          // use `BrowserApi. addListener` instead (no-restricted-syntax)
+          // eslint-disable-next-line no-restricted-syntax
+          chrome.notifications.onButtonClicked.addListener(
+            (notificationId: string, buttonIndex: number) => {
+              this.systemNotificationClickedSubject.next({
+                id: notificationId,
+                type: createInfo.type,
+                buttonIdentifier: buttonIndex,
+              });
+            },
+          );
+
+          // eslint-disable-next-line no-restricted-syntax
+          chrome.notifications.onClicked.addListener((notificationId: string) => {
            this.systemNotificationClickedSubject.next({
              id: notificationId,
              type: createInfo.type,
-              buttonIdentifier: buttonIndex,
+              buttonIdentifier: ButtonLocation.NotificationButton,
            });
-          },
-        );
-
-        // eslint-disable-next-line no-restricted-syntax
-        chrome.notifications.onClicked.addListener((notificationId: string) => {
-          this.systemNotificationClickedSubject.next({
-            id: notificationId,
-            type: createInfo.type,
-            buttonIdentifier: ButtonLocation.NotificationButton,
          });
-        });

-        break;
-      case DeviceType.FirefoxExtension:
-        this.logService.info("Creating firefox notification");
+          break;
+        case DeviceType.FirefoxExtension:
+          this.logService.info("Creating firefox notification");

-        await browser.notifications.create(createInfo.id, {
-          iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200",
-          message: createInfo.title,
-          type: "basic",
-          title: createInfo.title,
-        });
+          await browser.notifications.create(createInfo.id, {
+            iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200",
+            message: createInfo.title,
+            type: "basic",
+            title: createInfo.title,
+          });

-        browser.notifications.onButtonClicked.addListener(
-          (notificationId: string, buttonIndex: number) => {
+          browser.notifications.onButtonClicked.addListener(
+            (notificationId: string, buttonIndex: number) => {
+              this.systemNotificationClickedSubject.next({
+                id: notificationId,
+                type: createInfo.type,
+                buttonIdentifier: buttonIndex,
+              });
+            },
+          );
+
+          browser.notifications.onClicked.addListener((notificationId: string) => {
            this.systemNotificationClickedSubject.next({
              id: notificationId,
              type: createInfo.type,
-              buttonIdentifier: buttonIndex,
+              buttonIdentifier: ButtonLocation.NotificationButton,
            });
-          },
-        );
-
-        browser.notifications.onClicked.addListener((notificationId: string) => {
-          this.systemNotificationClickedSubject.next({
-            id: notificationId,
-            type: createInfo.type,
-            buttonIdentifier: ButtonLocation.NotificationButton,
          });
-        });
+      }
+    } catch (e) {
+      this.logService.error(
+        `Failed to create notification on ${this.platformUtilsService.getDevice()} with error: ${e}`,
+      );
    }
  }

--- a/libs/common/src/platform/actions/actions-service.ts
+++ b/libs/common/src/platform/actions/actions-service.ts
@@ -1,6 +1,22 @@
 export abstract class ActionsService {
  /**
-   * Opens the popup.
+   * Opens the popup if it is supported.
+   *
+   * --- Limitations ---
+   *
+   * These are conditions that work where can open a popup programmatically from:
+   *
+   * Safari Web Browser -> Safari Extension
+   *   - Requires gesture
+   * Chrome Web Browser -> Chrome Extension
+   * Chrome Extension Service Worker -> Chrome Extension
+   *
+   * These are conditions that are known to not work:
+   * Firefox Web Browser -> Firefox Extension
+   * Vivaldi Extension Background Service Worker -> Vivaldi Extension
+   * Safari Extension Background Service Worker -> Safari Extension
+   * Firefox Extension Background Service Worker -> Firefox Extension
+   * Opera Extension Background Service Worker -> Opera Extension
   */
  abstract openPopup(): Promise<void>;
 }