From 37523ca66b6f4394c58d0a7f48e7cd3b3b547f94 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:00:47 -0400 Subject: [PATCH] [PM-1900] Run Process Reload on Firefox (#10149) * Remove Firefox Exemption * Do Not Open Sidebar On Install * Run `chrome.runtime.reload` on All Browsers * Update Docs & Test Name * Should Probably Call The Sut In Test * Update Doc Comment * Update apps/browser/src/platform/browser/browser-api.ts Co-authored-by: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> --------- Co-authored-by: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> --- .../browser/src/background/main.background.ts | 6 +---- .../background/nativeMessaging.background.ts | 2 +- apps/browser/src/manifest.json | 1 + apps/browser/src/manifest.v3.json | 3 ++- .../src/platform/browser/browser-api.spec.ts | 22 ++----------------- .../src/platform/browser/browser-api.ts | 13 ++--------- apps/browser/src/popup/app.component.ts | 11 ---------- 7 files changed, 9 insertions(+), 49 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 5dc5afb479..cf393e0a44 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1028,12 +1028,8 @@ export default class MainBackground { ); const systemUtilsServiceReloadCallback = async () => { - const forceWindowReload = - this.platformUtilsService.isSafari() || - this.platformUtilsService.isFirefox() || - this.platformUtilsService.isOpera(); await this.taskSchedulerService.clearAllScheduledTasks(); - BrowserApi.reloadExtension(forceWindowReload ? self : null); + BrowserApi.reloadExtension(); }; this.systemService = new SystemService( diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index 613fe777ef..8f2cac7915 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -87,7 +87,7 @@ export class NativeMessagingBackground { // Reload extension to activate nativeMessaging chrome.permissions.onAdded.addListener((permissions) => { if (permissions.permissions?.includes("nativeMessaging")) { - BrowserApi.reloadExtension(null); + BrowserApi.reloadExtension(); } }); } diff --git a/apps/browser/src/manifest.json b/apps/browser/src/manifest.json index b13a98e7a4..121897b0ce 100644 --- a/apps/browser/src/manifest.json +++ b/apps/browser/src/manifest.json @@ -135,6 +135,7 @@ "default_title": "Bitwarden", "default_panel": "popup/index.html?uilocation=sidebar", "default_icon": "images/icon19.png", + "open_at_install": false, "browser_style": false }, "storage": { diff --git a/apps/browser/src/manifest.v3.json b/apps/browser/src/manifest.v3.json index c3420e1c6a..52e21a0936 100644 --- a/apps/browser/src/manifest.v3.json +++ b/apps/browser/src/manifest.v3.json @@ -143,7 +143,8 @@ "sidebar_action": { "default_title": "Bitwarden", "default_panel": "popup/index.html?uilocation=sidebar", - "default_icon": "images/icon19.png" + "default_icon": "images/icon19.png", + "open_at_install": false }, "storage": { "managed_schema": "managed_schema.json" diff --git a/apps/browser/src/platform/browser/browser-api.spec.ts b/apps/browser/src/platform/browser/browser-api.spec.ts index a5948f4959..6e8a0f3002 100644 --- a/apps/browser/src/platform/browser/browser-api.spec.ts +++ b/apps/browser/src/platform/browser/browser-api.spec.ts @@ -276,26 +276,8 @@ describe("BrowserApi", () => { }); describe("reloadExtension", () => { - it("reloads the window location if the passed globalContext is for the window", () => { - const windowMock = mock({ - location: { reload: jest.fn() }, - }) as unknown as Window & typeof globalThis; - - BrowserApi.reloadExtension(windowMock); - - expect(windowMock.location.reload).toHaveBeenCalled(); - }); - - it("reloads the extension runtime if the passed globalContext is not for the window", () => { - const globalMock = mock({}) as any; - BrowserApi.reloadExtension(globalMock); - - expect(chrome.runtime.reload).toHaveBeenCalled(); - }); - - it("reloads the extension runtime if a null value is passed as the globalContext", () => { - BrowserApi.reloadExtension(null); - + it("forwards call to extension runtime", () => { + BrowserApi.reloadExtension(); expect(chrome.runtime.reload).toHaveBeenCalled(); }); }); diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index ee4368eda7..a5ca7a65ff 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -412,18 +412,9 @@ export class BrowserApi { } /** - * Handles reloading the extension, either by calling the window location - * to reload or by calling the extension's runtime to reload. - * - * @param globalContext - The global context to use for the reload. + * Handles reloading the extension using the underlying functionality exposed by the browser API. */ - static reloadExtension(globalContext: (Window & typeof globalThis) | null) { - // The passed globalContext might be a ServiceWorkerGlobalScope, as a result - // we need to check if the location object exists before calling reload on it. - if (typeof globalContext?.location?.reload === "function") { - return (globalContext as any).location.reload(true); - } - + static reloadExtension() { return chrome.runtime.reload(); } diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 65fed72138..84a9ee264c 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -20,7 +20,6 @@ import { ToastService, } from "@bitwarden/components"; -import { BrowserApi } from "../platform/browser/browser-api"; import { PopupViewCacheService } from "../platform/popup/view-cache/popup-view-cache.service"; import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service"; import { BrowserSendStateService } from "../tools/popup/services/browser-send-state.service"; @@ -128,16 +127,6 @@ export class AppComponent implements OnInit, OnDestroy { this.showNativeMessagingFingerprintDialog(msg); } else if (msg.command === "showToast") { this.toastService._showToast(msg); - } else if (msg.command === "reloadProcess") { - const forceWindowReload = - this.platformUtilsService.isSafari() || - this.platformUtilsService.isFirefox() || - this.platformUtilsService.isOpera(); - // Wait to make sure background has reloaded first. - window.setTimeout( - () => BrowserApi.reloadExtension(forceWindowReload ? window : null), - 2000, - ); } else if (msg.command === "reloadPopup") { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises