diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 0d85743bba7..01d713742a5 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -356,7 +356,7 @@ const routes: Routes = [ { path: "edit-send", component: SendAddEditV2Component, - canActivate: [authGuard, filePickerPopoutGuard()], + canActivate: [authGuard], data: { elevation: 1 } satisfies RouteDataProperties, }, { diff --git a/apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts index 2f100ab67f2..64b7ac673c1 100644 --- a/apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts +++ b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts @@ -269,23 +269,21 @@ describe("filePickerPopoutGuard", () => { inSidebarSpy.mockReturnValue(false); }); - it.each([ - { route: "/import" }, - { route: "/add-send" }, - { route: "/edit-send" }, - { route: "/attachments" }, - ])("should open popout for $route route", async ({ route }) => { - const importState: RouterStateSnapshot = { - url: route, - } as RouterStateSnapshot; + it.each([{ route: "/import" }, { route: "/add-send" }, { route: "/attachments" }])( + "should open popout for $route route", + async ({ route }) => { + const importState: RouterStateSnapshot = { + url: route, + } as RouterStateSnapshot; - const guard = filePickerPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, importState)); + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, importState)); - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#" + route); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - }); + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#" + route); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }, + ); }); describe("Url handling", () => { @@ -354,30 +352,6 @@ describe("filePickerPopoutGuard", () => { expect(result).toBe(true); }, ); - - it.each([ - { deviceType: DeviceType.FirefoxExtension, name: "Firefox" }, - { deviceType: DeviceType.SafariExtension, name: "Safari" }, - { deviceType: DeviceType.ChromeExtension, name: "Chrome" }, - { deviceType: DeviceType.EdgeExtension, name: "Edge" }, - ])("should allow navigation for editing text Sends on $name", async ({ deviceType }) => { - getDeviceSpy.mockReturnValue(deviceType); - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - - const editTextSendState: RouterStateSnapshot = { - url: "/edit-send?sendId=abc123&type=0", - } as RouterStateSnapshot; - - const guard = filePickerPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => - guard(mockRoute, editTextSendState), - ); - - expect(openPopoutSpy).not.toHaveBeenCalled(); - expect(closePopupSpy).not.toHaveBeenCalled(); - expect(result).toBe(true); - }); }); describe("File Sends (type=1)", () => { @@ -487,115 +461,6 @@ describe("filePickerPopoutGuard", () => { } }, ); - - it.each([ - { - deviceType: DeviceType.FirefoxExtension, - name: "Firefox", - os: "Mac", - userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", - expectPopout: true, - }, - { - deviceType: DeviceType.FirefoxExtension, - name: "Firefox", - os: "Linux", - userAgent: "Mozilla/5.0 (X11; Linux x86_64)", - expectPopout: true, - }, - { - deviceType: DeviceType.FirefoxExtension, - name: "Firefox", - os: "Windows", - userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", - expectPopout: true, - }, - { - deviceType: DeviceType.SafariExtension, - name: "Safari", - os: "Mac", - userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", - expectPopout: true, - }, - { - deviceType: DeviceType.ChromeExtension, - name: "Chrome", - os: "Mac", - userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", - expectPopout: true, - }, - { - deviceType: DeviceType.ChromeExtension, - name: "Chrome", - os: "Linux", - userAgent: "Mozilla/5.0 (X11; Linux x86_64)", - expectPopout: true, - }, - { - deviceType: DeviceType.ChromeExtension, - name: "Chrome", - os: "Windows", - userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", - expectPopout: false, - }, - { - deviceType: DeviceType.EdgeExtension, - name: "Edge", - os: "Mac", - userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", - expectPopout: true, - }, - { - deviceType: DeviceType.EdgeExtension, - name: "Edge", - os: "Linux", - userAgent: "Mozilla/5.0 (X11; Linux x86_64)", - expectPopout: true, - }, - { - deviceType: DeviceType.EdgeExtension, - name: "Edge", - os: "Windows", - userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", - expectPopout: false, - }, - ])( - "should require popout for editing a file Send on $name $os", - async ({ deviceType, userAgent, expectPopout }) => { - getDeviceSpy.mockReturnValue(deviceType); - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - - if (userAgent) { - Object.defineProperty(window, "navigator", { - value: { userAgent, appVersion: userAgent }, - configurable: true, - writable: true, - }); - } - - const editFileSendState: RouterStateSnapshot = { - url: "/edit-send?sendId=abc123&type=1", - } as RouterStateSnapshot; - - const guard = filePickerPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => - guard(mockRoute, editFileSendState), - ); - - if (expectPopout === false) { - expect(openPopoutSpy).not.toHaveBeenCalled(); - expect(closePopupSpy).not.toHaveBeenCalled(); - expect(result).toBe(true); - } else { - expect(openPopoutSpy).toHaveBeenCalledWith( - "popup/index.html#/edit-send?sendId=abc123&type=1", - ); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - } - }, - ); }); describe("Send routes without type parameter", () => { diff --git a/apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts index 900ff328ac8..48a73ed780f 100644 --- a/apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts +++ b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts @@ -24,9 +24,9 @@ import { SendType } from "@bitwarden/common/tools/send/types/send-type"; */ export function filePickerPopoutGuard(): CanActivateFn { return async (_route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => { - // Check if this is a text Send route (no file picker needed) - if (isTextOnlySendRoute(state.url)) { - return true; // Allow navigation without popout + // Text Sends have no file picker — never require a popout regardless of browser + if (isTextSendRoute(state.url)) { + return true; } // Check if browser is one that needs popout for file pickers @@ -81,29 +81,16 @@ export function filePickerPopoutGuard(): CanActivateFn { } /** - * Determines if the route is for a text Send that doesn't require file picker display. - * - * @param url The route URL with query parameters - * @returns true if this is a Send route with explicitly text type (SendType.Text = 0) + * Returns true when the add-send route targets a Text Send (type=0). + * Text Sends have no file picker and never require a popout window. */ -function isTextOnlySendRoute(url: string): boolean { - // Only apply to Send routes - if (!url.includes("/add-send") && !url.includes("/edit-send")) { +function isTextSendRoute(url: string): boolean { + if (!url.includes("/add-send")) { return false; } - - // Parse query parameters to check Send type - const queryStartIndex = url.indexOf("?"); - if (queryStartIndex === -1) { - // No query params - default to requiring popout for safety + const queryStart = url.indexOf("?"); + if (queryStart === -1) { return false; } - - const queryString = url.substring(queryStartIndex + 1); - const params = new URLSearchParams(queryString); - const typeParam = params.get("type"); - - // Only skip popout for explicitly text-based Sends (SendType.Text = 0) - // If type is missing, null, or not text, default to requiring popout - return typeParam === String(SendType.Text); + return new URLSearchParams(url.substring(queryStart + 1)).get("type") === String(SendType.Text); } diff --git a/apps/browser/src/tools/popup/guards/firefox-popout.guard.spec.ts b/apps/browser/src/tools/popup/guards/firefox-popout.guard.spec.ts deleted file mode 100644 index df04b965d4c..00000000000 --- a/apps/browser/src/tools/popup/guards/firefox-popout.guard.spec.ts +++ /dev/null @@ -1,194 +0,0 @@ -import { TestBed } from "@angular/core/testing"; -import { ActivatedRouteSnapshot, RouterStateSnapshot } from "@angular/router"; - -import { DeviceType } from "@bitwarden/common/enums"; - -import { BrowserApi } from "../../../platform/browser/browser-api"; -import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; -import { BrowserPlatformUtilsService } from "../../../platform/services/platform-utils/browser-platform-utils.service"; - -import { firefoxPopoutGuard } from "./firefox-popout.guard"; - -describe("firefoxPopoutGuard", () => { - let getDeviceSpy: jest.SpyInstance; - let inPopoutSpy: jest.SpyInstance; - let inSidebarSpy: jest.SpyInstance; - let openPopoutSpy: jest.SpyInstance; - let closePopupSpy: jest.SpyInstance; - - const mockRoute = {} as ActivatedRouteSnapshot; - const mockState: RouterStateSnapshot = { - url: "/import?param=value", - } as RouterStateSnapshot; - - beforeEach(() => { - getDeviceSpy = jest.spyOn(BrowserPlatformUtilsService, "getDevice"); - inPopoutSpy = jest.spyOn(BrowserPopupUtils, "inPopout"); - inSidebarSpy = jest.spyOn(BrowserPopupUtils, "inSidebar"); - openPopoutSpy = jest.spyOn(BrowserPopupUtils, "openPopout").mockImplementation(); - closePopupSpy = jest.spyOn(BrowserApi, "closePopup").mockImplementation(); - - TestBed.configureTestingModule({}); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - describe("when browser is Firefox", () => { - beforeEach(() => { - getDeviceSpy.mockReturnValue(DeviceType.FirefoxExtension); - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - }); - - it("should open popout and block navigation when not already in popout or sidebar", async () => { - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); - - expect(getDeviceSpy).toHaveBeenCalledWith(window); - expect(inPopoutSpy).toHaveBeenCalledWith(window); - expect(inSidebarSpy).toHaveBeenCalledWith(window); - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/import?param=value"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - }); - - it("should not add autoClosePopout parameter to the url", async () => { - const guard = firefoxPopoutGuard(); - await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/import?param=value"); - expect(openPopoutSpy).not.toHaveBeenCalledWith(expect.stringContaining("autoClosePopout")); - }); - - it("should allow navigation when already in popout", async () => { - inPopoutSpy.mockReturnValue(true); - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); - - expect(openPopoutSpy).not.toHaveBeenCalled(); - expect(closePopupSpy).not.toHaveBeenCalled(); - expect(result).toBe(true); - }); - - it("should allow navigation when already in sidebar", async () => { - inSidebarSpy.mockReturnValue(true); - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); - - expect(openPopoutSpy).not.toHaveBeenCalled(); - expect(closePopupSpy).not.toHaveBeenCalled(); - expect(result).toBe(true); - }); - }); - - describe("when browser is not Firefox", () => { - beforeEach(() => { - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - }); - - it.each([ - { deviceType: DeviceType.ChromeExtension, name: "ChromeExtension" }, - { deviceType: DeviceType.EdgeExtension, name: "EdgeExtension" }, - { deviceType: DeviceType.OperaExtension, name: "OperaExtension" }, - { deviceType: DeviceType.SafariExtension, name: "SafariExtension" }, - { deviceType: DeviceType.VivaldiExtension, name: "VivaldiExtension" }, - ])( - "should allow navigation without opening popout when device is $name", - async ({ deviceType }) => { - getDeviceSpy.mockReturnValue(deviceType); - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); - - expect(getDeviceSpy).toHaveBeenCalledWith(window); - expect(openPopoutSpy).not.toHaveBeenCalled(); - expect(closePopupSpy).not.toHaveBeenCalled(); - expect(result).toBe(true); - }, - ); - }); - - describe("file picker routes", () => { - beforeEach(() => { - getDeviceSpy.mockReturnValue(DeviceType.FirefoxExtension); - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - }); - - it("should open popout for /import route", async () => { - const importState: RouterStateSnapshot = { - url: "/import", - } as RouterStateSnapshot; - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, importState)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/import"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - }); - - it("should open popout for /add-send route", async () => { - const addSendState: RouterStateSnapshot = { - url: "/add-send", - } as RouterStateSnapshot; - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, addSendState)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - }); - - it("should open popout for /edit-send route", async () => { - const editSendState: RouterStateSnapshot = { - url: "/edit-send", - } as RouterStateSnapshot; - - const guard = firefoxPopoutGuard(); - const result = await TestBed.runInInjectionContext(() => guard(mockRoute, editSendState)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/edit-send"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - expect(result).toBe(false); - }); - }); - - describe("url handling", () => { - beforeEach(() => { - getDeviceSpy.mockReturnValue(DeviceType.FirefoxExtension); - inPopoutSpy.mockReturnValue(false); - inSidebarSpy.mockReturnValue(false); - }); - - it("should preserve query parameters in the popout url", async () => { - const stateWithQuery: RouterStateSnapshot = { - url: "/import?foo=bar&baz=qux", - } as RouterStateSnapshot; - - const guard = firefoxPopoutGuard(); - await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithQuery)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/import?foo=bar&baz=qux"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - }); - - it("should handle urls without query parameters", async () => { - const stateWithoutQuery: RouterStateSnapshot = { - url: "/simple-path", - } as RouterStateSnapshot; - - const guard = firefoxPopoutGuard(); - await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithoutQuery)); - - expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/simple-path"); - expect(closePopupSpy).toHaveBeenCalledWith(window); - }); - }); -}); diff --git a/apps/browser/src/tools/popup/guards/firefox-popout.guard.ts b/apps/browser/src/tools/popup/guards/firefox-popout.guard.ts deleted file mode 100644 index 821f1b7a5bc..00000000000 --- a/apps/browser/src/tools/popup/guards/firefox-popout.guard.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { ActivatedRouteSnapshot, CanActivateFn, RouterStateSnapshot } from "@angular/router"; - -import { BrowserApi } from "@bitwarden/browser/platform/browser/browser-api"; -import BrowserPopupUtils from "@bitwarden/browser/platform/browser/browser-popup-utils"; -import { BrowserPlatformUtilsService } from "@bitwarden/browser/platform/services/platform-utils/browser-platform-utils.service"; -import { DeviceType } from "@bitwarden/common/enums"; - -/** - * Guard that forces a popout window on Firefox browser when a file picker could be exposed. - * Necessary to avoid a crash: https://bugzilla.mozilla.org/show_bug.cgi?id=1292701 - * Also disallows the user from closing a popout and re-opening the view exposing the file picker. - * - * @returns CanActivateFn that opens popout and blocks navigation on Firefox - */ -export function firefoxPopoutGuard(): CanActivateFn { - return async (_route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => { - // Check if browser is Firefox using the platform utils service - const deviceType = BrowserPlatformUtilsService.getDevice(window); - const isFirefox = deviceType === DeviceType.FirefoxExtension; - - // Check if already in popout/sidebar - const inPopout = BrowserPopupUtils.inPopout(window); - const inSidebar = BrowserPopupUtils.inSidebar(window); - - // Open popout if on Firefox and not already in popout/sidebar - if (isFirefox && !inPopout && !inSidebar) { - // Don't add autoClosePopout for file picker scenarios - user should manually close - await BrowserPopupUtils.openPopout(`popup/index.html#${state.url}`); - - // Close the original popup window - BrowserApi.closePopup(window); - - return false; // Block navigation - popout will reload - } - - return true; // Allow navigation - }; -} diff --git a/apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog-container.component.ts b/apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog-container.component.ts deleted file mode 100644 index e69de29bb2d..00000000000