From 5d01cd4309c30cfba4af0956caf3a7c7b6ea51a3 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Mon, 21 Jul 2025 15:25:54 -0400 Subject: [PATCH] feat(notification-processing): [PM-19877] System Notification Implementation - Making final touchups. --- .../browser/src/background/main.background.ts | 40 +------ .../actions/browser-actions.service.ts | 48 +++++---- .../browser-system-notification.service.ts | 102 +++++++++--------- .../src/platform/actions/actions-service.ts | 18 +++- 4 files changed, 103 insertions(+), 105 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 54361b2f739..70960c07a63 100644 --- 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); } } diff --git a/apps/browser/src/platform/actions/browser-actions.service.ts b/apps/browser/src/platform/actions/browser-actions.service.ts index 402b588ed6d..9156f009e8c 100644 --- 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 { 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}`, + ); } } } diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index 66089fb0df8..2fe3196eb18 100644 --- 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 { - 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}`, + ); } } diff --git a/libs/common/src/platform/actions/actions-service.ts b/libs/common/src/platform/actions/actions-service.ts index ba72e057caa..f8b23472d7c 100644 --- 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; }