From d88cb89618fe4f1c15e8d7eff50a312fca9a8c5f Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Thu, 5 Feb 2026 08:41:03 -0700 Subject: [PATCH] PM-23851 False requirement to pop out extension when using send files (#17950) * follow existing popout guard pattern to force popout on firefox when filepicker is exposed * move firefox guard to tools ownership & revert changes to auth owned file * initial refactor to consolidate logic using file-picker-popout.guard * remove safari from guard & disable forced popout in vault import * enforce popout on Safari with test coverage * use userAgent and consistent detection for platform detection * refactor guard tests involving routes * replace imports lost during merge * remove text sends from popout requirement and update tests * add tooltip and screen-reader text describing popout behavior --- apps/browser/src/_locales/en/messages.json | 20 - apps/browser/src/popup/app-routing.module.ts | 10 +- apps/browser/src/popup/app.module.ts | 2 - .../src/popup/services/services.module.ts | 8 - .../file-popout-callout.component.html | 11 - .../file-popout-callout.component.ts | 37 - .../guards/file-picker-popout.guard.spec.ts | 834 ++++++++++++++++++ .../popup/guards/file-picker-popout.guard.ts | 109 +++ .../add-edit/send-add-edit.component.html | 2 - .../add-edit/send-add-edit.component.ts | 2 - ...ile-popout-dialog-container.component.html | 0 ...-file-popout-dialog-container.component.ts | 41 - .../send-file-popout-dialog.component.html | 20 - .../send-file-popout-dialog.component.ts | 26 - .../services/file-popout-utils.service.ts | 71 -- .../open-attachments.component.html | 5 +- .../open-attachments.component.spec.ts | 30 +- .../open-attachments.component.ts | 16 - .../vault-v2/vault-v2.component.spec.ts | 18 +- .../components/vault-v2/vault-v2.component.ts | 5 - .../settings/vault-settings-v2.component.html | 7 +- .../settings/vault-settings-v2.component.ts | 5 - .../new-send-dropdown.component.html | 4 +- 23 files changed, 961 insertions(+), 322 deletions(-) delete mode 100644 apps/browser/src/tools/popup/components/file-popout-callout.component.html delete mode 100644 apps/browser/src/tools/popup/components/file-popout-callout.component.ts create mode 100644 apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts create mode 100644 apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts delete mode 100644 apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog-container.component.html delete mode 100644 apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.html delete mode 100644 apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.ts delete mode 100644 apps/browser/src/tools/popup/services/file-popout-utils.service.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 5026f5e2799..972fd60cc2e 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -3094,29 +3094,9 @@ "message": "Send saved", "description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated." }, - "sendFilePopoutDialogText": { - "message": "Pop out extension?", - "description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated." - }, - "sendFilePopoutDialogDesc": { - "message": "To create a file Send, you need to pop out the extension to a new window.", - "description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated." - }, - "sendLinuxChromiumFileWarning": { - "message": "In order to choose a file, open the extension in the sidebar (if possible) or pop out to a new window by clicking this banner." - }, - "sendFirefoxFileWarning": { - "message": "In order to choose a file using Firefox, open the extension in the sidebar or pop out to a new window by clicking this banner." - }, - "sendSafariFileWarning": { - "message": "In order to choose a file using Safari, pop out to a new window by clicking this banner." - }, "popOut": { "message": "Pop out" }, - "sendFileCalloutHeader": { - "message": "Before you start" - }, "expirationDateIsInvalid": { "message": "The expiration date provided is not valid." }, diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 1f1d4d25b40..7fb466449f2 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -69,7 +69,7 @@ import { popupRouterCacheGuard } from "../platform/popup/view-cache/popup-router import { RouteCacheOptions } from "../platform/services/popup-view-cache-background.service"; import { CredentialGeneratorHistoryComponent } from "../tools/popup/generator/credential-generator-history.component"; import { CredentialGeneratorComponent } from "../tools/popup/generator/credential-generator.component"; -import { firefoxPopoutGuard } from "../tools/popup/guards/firefox-popout.guard"; +import { filePickerPopoutGuard } from "../tools/popup/guards/file-picker-popout.guard"; import { SendAddEditComponent as SendAddEditV2Component } from "../tools/popup/send-v2/add-edit/send-add-edit.component"; import { SendCreatedComponent } from "../tools/popup/send-v2/send-created/send-created.component"; import { SendV2Component } from "../tools/popup/send-v2/send-v2.component"; @@ -248,7 +248,7 @@ const routes: Routes = [ { path: "attachments", component: AttachmentsV2Component, - canActivate: [authGuard], + canActivate: [authGuard, filePickerPopoutGuard()], data: { elevation: 4 } satisfies RouteDataProperties, }, { @@ -266,7 +266,7 @@ const routes: Routes = [ { path: "import", component: ImportBrowserV2Component, - canActivate: [authGuard, firefoxPopoutGuard()], + canActivate: [authGuard, filePickerPopoutGuard()], data: { elevation: 1 } satisfies RouteDataProperties, }, { @@ -350,13 +350,13 @@ const routes: Routes = [ { path: "add-send", component: SendAddEditV2Component, - canActivate: [authGuard, firefoxPopoutGuard()], + canActivate: [authGuard, filePickerPopoutGuard()], data: { elevation: 1 } satisfies RouteDataProperties, }, { path: "edit-send", component: SendAddEditV2Component, - canActivate: [authGuard, firefoxPopoutGuard()], + canActivate: [authGuard, filePickerPopoutGuard()], data: { elevation: 1 } satisfies RouteDataProperties, }, { diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index d178cee2fc3..4ed79dd144d 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -33,7 +33,6 @@ import { PopupFooterComponent } from "../platform/popup/layout/popup-footer.comp import { PopupHeaderComponent } from "../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../platform/popup/layout/popup-page.component"; import { PopupTabNavigationComponent } from "../platform/popup/layout/popup-tab-navigation.component"; -import { FilePopoutCalloutComponent } from "../tools/popup/components/file-popout-callout.component"; import { AppRoutingModule } from "./app-routing.module"; import { AppComponent } from "./app.component"; @@ -67,7 +66,6 @@ import "../platform/popup/locales"; ScrollingModule, ServicesModule, DialogModule, - FilePopoutCalloutComponent, AvatarModule, AccountComponent, ButtonModule, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index a8bfb23d83f..a3caaa95fa2 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -230,7 +230,6 @@ import { isNotificationsSupported, } from "../../platform/system-notifications/browser-system-notification.service"; import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging"; -import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service"; import { BrowserAutofillNudgeService } from "../../vault/popup/services/browser-autofill-nudge.service"; import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service"; import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service"; @@ -502,13 +501,6 @@ const safeProviders: SafeProvider[] = [ }, deps: [PlatformUtilsService], }), - safeProvider({ - provide: FilePopoutUtilsService, - useFactory: (platformUtilsService: PlatformUtilsService) => { - return new FilePopoutUtilsService(platformUtilsService); - }, - deps: [PlatformUtilsService], - }), safeProvider({ provide: DerivedStateProvider, useClass: InlineDerivedStateProvider, diff --git a/apps/browser/src/tools/popup/components/file-popout-callout.component.html b/apps/browser/src/tools/popup/components/file-popout-callout.component.html deleted file mode 100644 index 0df77dc4367..00000000000 --- a/apps/browser/src/tools/popup/components/file-popout-callout.component.html +++ /dev/null @@ -1,11 +0,0 @@ - -
{{ "sendLinuxChromiumFileWarning" | i18n }}
-
{{ "sendFirefoxFileWarning" | i18n }}
-
{{ "sendSafariFileWarning" | i18n }}
- diff --git a/apps/browser/src/tools/popup/components/file-popout-callout.component.ts b/apps/browser/src/tools/popup/components/file-popout-callout.component.ts deleted file mode 100644 index 33044b79351..00000000000 --- a/apps/browser/src/tools/popup/components/file-popout-callout.component.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { CommonModule } from "@angular/common"; -import { Component, OnInit } from "@angular/core"; - -import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { CalloutModule } from "@bitwarden/components"; - -import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; -import { FilePopoutUtilsService } from "../services/file-popout-utils.service"; - -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection -@Component({ - selector: "tools-file-popout-callout", - templateUrl: "file-popout-callout.component.html", - imports: [CommonModule, JslibModule, CalloutModule], -}) -export class FilePopoutCalloutComponent implements OnInit { - protected showFilePopoutMessage: boolean = false; - protected showFirefoxFileWarning: boolean = false; - protected showSafariFileWarning: boolean = false; - protected showChromiumFileWarning: boolean = false; - - constructor(private filePopoutUtilsService: FilePopoutUtilsService) {} - - ngOnInit() { - this.showFilePopoutMessage = this.filePopoutUtilsService.showFilePopoutMessage(window); - this.showFirefoxFileWarning = this.filePopoutUtilsService.showFirefoxFileWarning(window); - this.showSafariFileWarning = this.filePopoutUtilsService.showSafariFileWarning(window); - this.showChromiumFileWarning = this.filePopoutUtilsService.showChromiumFileWarning(window); - } - - popOutWindow() { - // 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 - BrowserPopupUtils.openCurrentPagePopout(window); - } -} 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 new file mode 100644 index 00000000000..2f100ab67f2 --- /dev/null +++ b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts @@ -0,0 +1,834 @@ +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 { filePickerPopoutGuard } from "./file-picker-popout.guard"; + +describe("filePickerPopoutGuard", () => { + 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: "/add-send?type=1", + } 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("Firefox browser", () => { + beforeEach(() => { + getDeviceSpy.mockReturnValue(DeviceType.FirefoxExtension); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should open popout and block navigation when not in popout or sidebar", async () => { + const guard = filePickerPopoutGuard(); + 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#/add-send?type=1"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }); + + it("should allow navigation when already in popout", async () => { + inPopoutSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + 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 = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("Safari browser", () => { + beforeEach(() => { + getDeviceSpy.mockReturnValue(DeviceType.SafariExtension); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should open popout and block navigation when not in popout", async () => { + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(getDeviceSpy).toHaveBeenCalledWith(window); + expect(inPopoutSpy).toHaveBeenCalledWith(window); + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=1"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }); + + it("should allow navigation when already in popout", async () => { + inPopoutSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("should not allow sidebar bypass (Safari doesn't support sidebar)", async () => { + inSidebarSpy.mockReturnValue(true); + inPopoutSpy.mockReturnValue(false); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + // Safari requires popout, sidebar is not sufficient + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=1"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }); + }); + + describe("Chromium browsers on Linux", () => { + beforeEach(() => { + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + Object.defineProperty(window, "navigator", { + value: { + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + appVersion: "5.0 (X11; Linux x86_64)", + }, + configurable: true, + writable: true, + }); + }); + + it.each([ + { deviceType: DeviceType.ChromeExtension, name: "Chrome" }, + { deviceType: DeviceType.EdgeExtension, name: "Edge" }, + { deviceType: DeviceType.OperaExtension, name: "Opera" }, + { deviceType: DeviceType.VivaldiExtension, name: "Vivaldi" }, + ])( + "should open popout and block navigation for $name on Linux when not in popout or sidebar", + async ({ deviceType }) => { + getDeviceSpy.mockReturnValue(deviceType); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=1"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }, + ); + + it("should allow navigation when in popout", async () => { + getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension); + inPopoutSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + 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 in sidebar", async () => { + getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension); + inSidebarSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("Chromium browsers on Mac", () => { + beforeEach(() => { + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + Object.defineProperty(window, "navigator", { + value: { + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + appVersion: "5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + configurable: true, + writable: true, + }); + }); + + it.each([ + { deviceType: DeviceType.ChromeExtension, name: "Chrome" }, + { deviceType: DeviceType.EdgeExtension, name: "Edge" }, + { deviceType: DeviceType.OperaExtension, name: "Opera" }, + { deviceType: DeviceType.VivaldiExtension, name: "Vivaldi" }, + ])( + "should open popout and block navigation for $name on Mac when not in popout or sidebar", + async ({ deviceType }) => { + getDeviceSpy.mockReturnValue(deviceType); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=1"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }, + ); + + it("should allow navigation when in popout", async () => { + getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension); + inPopoutSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + 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 in sidebar", async () => { + getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension); + inSidebarSpy.mockReturnValue(true); + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("Chromium browsers on Windows", () => { + beforeEach(() => { + getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + Object.defineProperty(window, "navigator", { + value: { + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + appVersion: "5.0 (Windows NT 10.0; Win64; x64)", + }, + configurable: true, + writable: true, + }); + }); + + it("should allow navigation without popout on Windows", async () => { + const guard = filePickerPopoutGuard(); + 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.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; + + 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); + }); + }); + + 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 = filePickerPopoutGuard(); + 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 = filePickerPopoutGuard(); + await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithoutQuery)); + + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/simple-path"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + }); + + it("should not add autoClosePopout parameter to the url", async () => { + const guard = filePickerPopoutGuard(); + await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=1"); + expect(openPopoutSpy).not.toHaveBeenCalledWith(expect.stringContaining("autoClosePopout")); + }); + }); + + describe("Send type differentiation", () => { + describe("Text Sends (type=0)", () => { + 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 without popout for new text Sends on $name", + async ({ deviceType }) => { + getDeviceSpy.mockReturnValue(deviceType); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + + const textSendState: RouterStateSnapshot = { + url: "/add-send?type=0&isNew=true", + } as RouterStateSnapshot; + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, textSendState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + 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)", () => { + 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 a new 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 fileSendState: RouterStateSnapshot = { + url: "/add-send?type=1&isNew=true", + } as RouterStateSnapshot; + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, fileSendState)); + + if (expectPopout === false) { + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + } else { + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/add-send?type=1&isNew=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + } + }, + ); + + 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", () => { + it.each([ + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + { + deviceType: DeviceType.SafariExtension, + name: "Safari", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + ])( + "should default to requiring popout on $name $os", + async ({ deviceType, userAgent, os }) => { + getDeviceSpy.mockReturnValue(deviceType); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + + if (userAgent) { + Object.defineProperty(window, "navigator", { + value: { userAgent, appVersion: userAgent }, + configurable: true, + writable: true, + }); + } + + const noTypeState: RouterStateSnapshot = { + url: "/add-send", + } as RouterStateSnapshot; + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, noTypeState)); + + // Windows Chrome/Edge don't need popout + const isChromiumOnWindows = + (deviceType === DeviceType.ChromeExtension || + deviceType === DeviceType.EdgeExtension) && + os === "Windows"; + + if (isChromiumOnWindows) { + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + } else { + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + } + }, + ); + + it.each([ + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.FirefoxExtension, + name: "Firefox", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + { + deviceType: DeviceType.SafariExtension, + name: "Safari", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.ChromeExtension, + name: "Chrome", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Mac", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Linux", + userAgent: "Mozilla/5.0 (X11; Linux x86_64)", + }, + { + deviceType: DeviceType.EdgeExtension, + name: "Edge", + os: "Windows", + userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64)", + }, + ])( + "should default to requiring popout when type is invalid on $name $os", + async ({ deviceType, userAgent, os }) => { + getDeviceSpy.mockReturnValue(deviceType); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + + if (userAgent) { + Object.defineProperty(window, "navigator", { + value: { userAgent, appVersion: userAgent }, + configurable: true, + writable: true, + }); + } + + const invalidTypeState: RouterStateSnapshot = { + url: "/add-send?type=invalid", + } as RouterStateSnapshot; + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => + guard(mockRoute, invalidTypeState), + ); + + // Windows Chrome/Edge don't need popout + const isChromiumOnWindows = + (deviceType === DeviceType.ChromeExtension || + deviceType === DeviceType.EdgeExtension) && + os === "Windows"; + + if (isChromiumOnWindows) { + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + } else { + expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/add-send?type=invalid"); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + } + }, + ); + }); + + describe("non-Send routes", () => { + it.each([ + { deviceType: DeviceType.FirefoxExtension, name: "Firefox", route: "/import" }, + { deviceType: DeviceType.FirefoxExtension, name: "Firefox", route: "/attachments" }, + { deviceType: DeviceType.SafariExtension, name: "Safari", route: "/import" }, + { deviceType: DeviceType.SafariExtension, name: "Safari", route: "/attachments" }, + ])( + "should always require popout for $route on $name regardless of query params", + async ({ deviceType, route }) => { + getDeviceSpy.mockReturnValue(deviceType); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + + const routeState: RouterStateSnapshot = { + url: `${route}?type=0`, + } as RouterStateSnapshot; + + const guard = filePickerPopoutGuard(); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, routeState)); + + expect(openPopoutSpy).toHaveBeenCalledWith(`popup/index.html#${route}?type=0`); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }, + ); + }); + }); +}); 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 new file mode 100644 index 00000000000..900ff328ac8 --- /dev/null +++ b/apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts @@ -0,0 +1,109 @@ +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"; +import { SendType } from "@bitwarden/common/tools/send/types/send-type"; + +/** + * Composite guard that handles file picker popout requirements for all browsers. + * Forces a popout window when file pickers could be exposed on browsers that require it. + * + * Browser-specific requirements: + * - Firefox: Requires sidebar OR popout (crashes with file picker in popup: https://bugzilla.mozilla.org/show_bug.cgi?id=1292701) + * - Safari: Requires popout only + * - All Chromium browsers (Chrome, Edge, Opera, Vivaldi) on Linux/Mac: Requires sidebar OR popout + * - Chromium on Windows: No special requirement + * + * Send-specific behavior: + * - Text Sends: No popout required (no file picker needed) + * - File Sends: Popout required on affected browsers + * + * @returns CanActivateFn that opens popout and blocks navigation when file picker access is needed + */ +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 + } + + // Check if browser is one that needs popout for file pickers + const deviceType = BrowserPlatformUtilsService.getDevice(window); + + // Check current context + const inPopout = BrowserPopupUtils.inPopout(window); + const inSidebar = BrowserPopupUtils.inSidebar(window); + + let needsPopout = false; + + // Firefox: needs sidebar OR popout to avoid crash with file picker + if (deviceType === DeviceType.FirefoxExtension && !inPopout && !inSidebar) { + needsPopout = true; + } + + // Safari: needs popout only (sidebar not available) + if (deviceType === DeviceType.SafariExtension && !inPopout) { + needsPopout = true; + } + + // Chromium on Linux/Mac: needs sidebar OR popout for file picker access + // All Chromium-based browsers (Chrome, Edge, Opera, Vivaldi) + // Brave intentionally reports itself as Chrome for compatibility + const isChromiumBased = [ + DeviceType.ChromeExtension, + DeviceType.EdgeExtension, + DeviceType.OperaExtension, + DeviceType.VivaldiExtension, + ].includes(deviceType); + + const isLinux = window?.navigator?.userAgent?.includes("Linux"); + const isMac = window?.navigator?.userAgent?.includes("Mac OS X"); + + if (isChromiumBased && (isLinux || isMac) && !inPopout && !inSidebar) { + needsPopout = true; + } + + // Open popout if needed + if (needsPopout) { + // 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 + }; +} + +/** + * 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) + */ +function isTextOnlySendRoute(url: string): boolean { + // Only apply to Send routes + if (!url.includes("/add-send") && !url.includes("/edit-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 + 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); +} diff --git a/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.html b/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.html index a72847a5bf2..2d588a9ee78 100644 --- a/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.html +++ b/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.html @@ -10,8 +10,6 @@ > - - - - - diff --git a/apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.ts b/apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.ts deleted file mode 100644 index 23fa744995a..00000000000 --- a/apps/browser/src/tools/popup/send-v2/send-file-popout-dialog/send-file-popout-dialog.component.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { CommonModule } from "@angular/common"; -import { Component } from "@angular/core"; - -import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { ButtonModule, DialogModule, DialogService, TypographyModule } from "@bitwarden/components"; - -import BrowserPopupUtils from "../../../../platform/browser/browser-popup-utils"; - -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection -@Component({ - selector: "send-file-popout-dialog", - templateUrl: "./send-file-popout-dialog.component.html", - imports: [JslibModule, CommonModule, DialogModule, ButtonModule, TypographyModule], -}) -export class SendFilePopoutDialogComponent { - constructor(private dialogService: DialogService) {} - - async popOutWindow() { - await BrowserPopupUtils.openCurrentPagePopout(window); - } - - close() { - this.dialogService.closeAll(); - } -} diff --git a/apps/browser/src/tools/popup/services/file-popout-utils.service.ts b/apps/browser/src/tools/popup/services/file-popout-utils.service.ts deleted file mode 100644 index 9a04d4b8f23..00000000000 --- a/apps/browser/src/tools/popup/services/file-popout-utils.service.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { Injectable } from "@angular/core"; - -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; - -import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; - -/** - * Service for determining whether to display file popout callout messages. - */ -@Injectable() -export class FilePopoutUtilsService { - /** - * Creates an instance of FilePopoutUtilsService. - */ - constructor(private platformUtilsService: PlatformUtilsService) {} - - /** - * Determines whether to show any file popout callout message in the current browser. - * @param win - The window context in which the check should be performed. - * @returns True if a file popout callout message should be displayed; otherwise, false. - */ - showFilePopoutMessage(win: Window): boolean { - return ( - this.showFirefoxFileWarning(win) || - this.showSafariFileWarning(win) || - this.showChromiumFileWarning(win) - ); - } - - /** - * Determines whether to show a file popout callout message for the Firefox browser - * @param win - The window context in which the check should be performed. - * @returns True if the extension is not in a sidebar or popout; otherwise, false. - */ - showFirefoxFileWarning(win: Window): boolean { - return ( - this.platformUtilsService.isFirefox() && - !(BrowserPopupUtils.inSidebar(win) || BrowserPopupUtils.inPopout(win)) - ); - } - - /** - * Determines whether to show a file popout message for the Safari browser - * @param win - The window context in which the check should be performed. - * @returns True if the extension is not in a popout; otherwise, false. - */ - showSafariFileWarning(win: Window): boolean { - return this.platformUtilsService.isSafari() && !BrowserPopupUtils.inPopout(win); - } - - /** - * Determines whether to show a file popout callout message for Chromium-based browsers in Linux and Mac OS X - * @param win - The window context in which the check should be performed. - * @returns True if the extension is not in a sidebar or popout; otherwise, false. - */ - showChromiumFileWarning(win: Window): boolean { - return ( - (this.isLinux(win) || this.isUnsupportedMac(win)) && - !this.platformUtilsService.isFirefox() && - !(BrowserPopupUtils.inSidebar(win) || BrowserPopupUtils.inPopout(win)) - ); - } - - private isLinux(win: Window): boolean { - return win?.navigator?.userAgent.indexOf("Linux") !== -1; - } - - private isUnsupportedMac(win: Window): boolean { - return this.platformUtilsService.isChrome() && win?.navigator?.appVersion.includes("Mac OS X"); - } -} diff --git a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html index 0fbe1c55b0a..1e9d63b709b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html @@ -4,14 +4,15 @@ type="button" (click)="openAttachments()" [disabled]="parentFormDisabled" + [title]="'popOutNewWindow' | i18n" >
{{ "attachments" | i18n }}
- - + {{ "popOutNewWindow" | i18n }} + diff --git a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.spec.ts index e9636e09873..b88b435c702 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.spec.ts @@ -20,9 +20,6 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { ToastService } from "@bitwarden/components"; import { CipherFormContainer } from "@bitwarden/vault"; -import BrowserPopupUtils from "../../../../../../platform/browser/browser-popup-utils"; -import { FilePopoutUtilsService } from "../../../../../../tools/popup/services/file-popout-utils.service"; - import { OpenAttachmentsComponent } from "./open-attachments.component"; describe("OpenAttachmentsComponent", () => { @@ -31,9 +28,6 @@ describe("OpenAttachmentsComponent", () => { let router: Router; const showToast = jest.fn(); const hasPremiumFromAnySource$ = new BehaviorSubject(true); - const openCurrentPagePopout = jest - .spyOn(BrowserPopupUtils, "openCurrentPagePopout") - .mockResolvedValue(null); const cipherView = { id: "5555-444-3333", type: CipherType.Login, @@ -55,7 +49,6 @@ describe("OpenAttachmentsComponent", () => { const getCipher = jest.fn().mockResolvedValue(cipherDomain); const organizations$ = jest.fn().mockReturnValue(of([org])); - const showFilePopoutMessage = jest.fn().mockReturnValue(false); const mockUserId = Utils.newGuid() as UserId; const accountService = { @@ -70,11 +63,9 @@ describe("OpenAttachmentsComponent", () => { const formStatusChange$ = new BehaviorSubject<"enabled" | "disabled">("enabled"); beforeEach(async () => { - openCurrentPagePopout.mockClear(); getCipher.mockClear(); showToast.mockClear(); organizations$.mockClear(); - showFilePopoutMessage.mockClear(); hasPremiumFromAnySource$.next(true); formStatusChange$.next("enabled"); @@ -103,10 +94,6 @@ describe("OpenAttachmentsComponent", () => { provide: OrganizationService, useValue: { organizations$ }, }, - { - provide: FilePopoutUtilsService, - useValue: { showFilePopoutMessage }, - }, { provide: AccountService, useValue: accountService, @@ -130,8 +117,7 @@ describe("OpenAttachmentsComponent", () => { fixture.detectChanges(); }); - it("opens attachments in new popout", async () => { - showFilePopoutMessage.mockReturnValue(true); + it("navigates to attachments route", async () => { component.canAccessAttachments = true; await component.ngOnInit(); @@ -140,20 +126,6 @@ describe("OpenAttachmentsComponent", () => { expect(router.navigate).toHaveBeenCalledWith(["/attachments"], { queryParams: { cipherId: "5555-444-3333" }, }); - expect(openCurrentPagePopout).toHaveBeenCalledWith(window); - }); - - it("opens attachments in same window", async () => { - showFilePopoutMessage.mockReturnValue(false); - component.canAccessAttachments = true; - await component.ngOnInit(); - - await component.openAttachments(); - - expect(openCurrentPagePopout).not.toHaveBeenCalled(); - expect(router.navigate).toHaveBeenCalledWith(["/attachments"], { - queryParams: { cipherId: "5555-444-3333" }, - }); }); it("routes the user to the premium page when they cannot access premium features", async () => { diff --git a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.ts b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.ts index a267e7999ab..1a1f767ca8c 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.ts @@ -23,9 +23,6 @@ import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstraction import { BadgeModule, ItemModule, ToastService, TypographyModule } from "@bitwarden/components"; import { CipherFormContainer } from "@bitwarden/vault"; -import BrowserPopupUtils from "../../../../../../platform/browser/browser-popup-utils"; -import { FilePopoutUtilsService } from "../../../../../../tools/popup/services/file-popout-utils.service"; - // FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ @@ -46,9 +43,6 @@ export class OpenAttachmentsComponent implements OnInit { // eslint-disable-next-line @angular-eslint/prefer-signals @Input({ required: true }) cipherId: CipherId; - /** True when the attachments window should be opened in a popout */ - openAttachmentsInPopout: boolean; - /** True when the user has access to premium or h */ canAccessAttachments: boolean; @@ -65,7 +59,6 @@ export class OpenAttachmentsComponent implements OnInit { private organizationService: OrganizationService, private toastService: ToastService, private i18nService: I18nService, - private filePopoutUtilsService: FilePopoutUtilsService, private accountService: AccountService, private cipherFormContainer: CipherFormContainer, private premiumUpgradeService: PremiumUpgradePromptService, @@ -87,8 +80,6 @@ export class OpenAttachmentsComponent implements OnInit { } async ngOnInit(): Promise { - this.openAttachmentsInPopout = this.filePopoutUtilsService.showFilePopoutMessage(window); - if (!this.cipherId) { return; } @@ -131,12 +122,5 @@ export class OpenAttachmentsComponent implements OnInit { } await this.router.navigate(["/attachments"], { queryParams: { cipherId: this.cipherId } }); - - // Open the attachments page in a popout - // This is done after the router navigation to ensure that the navigation - // is included in the `PopupRouterCacheService` history - if (this.openAttachmentsInPopout) { - await BrowserPopupUtils.openCurrentPagePopout(window); - } } } diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts index a322fbc53dd..a956b2fe68b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts @@ -421,29 +421,13 @@ describe("VaultV2Component", () => { expect(PremiumUpgradeDialogComponent.open).toHaveBeenCalledTimes(1); }); - it("navigateToImport navigates and opens popout if popup is open", fakeAsync(async () => { - (BrowserApi.isPopupOpen as jest.Mock).mockResolvedValueOnce(true); - + it("navigateToImport navigates to import route", fakeAsync(async () => { const ngRouter = TestBed.inject(Router); jest.spyOn(ngRouter, "navigate").mockResolvedValue(true as any); await component["navigateToImport"](); expect(ngRouter.navigate).toHaveBeenCalledWith(["/import"]); - - expect(BrowserPopupUtils.openCurrentPagePopout).toHaveBeenCalled(); - })); - - it("navigateToImport does not popout when popup is not open", fakeAsync(async () => { - (BrowserApi.isPopupOpen as jest.Mock).mockResolvedValueOnce(false); - - const ngRouter = TestBed.inject(Router); - jest.spyOn(ngRouter, "navigate").mockResolvedValue(true as any); - - await component["navigateToImport"](); - - expect(ngRouter.navigate).toHaveBeenCalledWith(["/import"]); - expect(BrowserPopupUtils.openCurrentPagePopout).not.toHaveBeenCalled(); })); it("ngOnInit dismisses intro carousel and opens decryption dialog for non-deleted failures", fakeAsync(() => { diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index fce084542a9..a5a74eb8ab8 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -56,8 +56,6 @@ import { } from "@bitwarden/vault"; import { CurrentAccountComponent } from "../../../../auth/popup/account-switching/current-account.component"; -import { BrowserApi } from "../../../../platform/browser/browser-api"; -import BrowserPopupUtils from "../../../../platform/browser/browser-popup-utils"; import { PopOutComponent } from "../../../../platform/popup/components/pop-out.component"; import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page.component"; @@ -370,9 +368,6 @@ export class VaultV2Component implements OnInit, OnDestroy { async navigateToImport() { await this.router.navigate(["/import"]); - if (await BrowserApi.isPopupOpen()) { - await BrowserPopupUtils.openCurrentPagePopout(window); - } } async dismissVaultNudgeSpotlight(type: NudgeType) { diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html index ad009c7a60b..c84188af863 100644 --- a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html @@ -13,7 +13,7 @@ - diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts index c1d90d678cb..c35345bd8ab 100644 --- a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts @@ -15,8 +15,6 @@ import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstraction import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { BadgeComponent, ItemModule, ToastOptions, ToastService } from "@bitwarden/components"; -import { BrowserApi } from "../../../platform/browser/browser-api"; -import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; @@ -90,9 +88,6 @@ export class VaultSettingsV2Component implements OnInit, OnDestroy { async import() { await this.router.navigate(["/import"]); - if (await BrowserApi.isPopupOpen()) { - await BrowserPopupUtils.openCurrentPagePopout(window); - } } async sync() { diff --git a/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html b/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html index fb9b82c44e5..7b966bb0345 100644 --- a/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html +++ b/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html @@ -7,11 +7,13 @@ {{ "sendTypeText" | i18n }} - +
{{ "sendTypeFile" | i18n }}
+ {{ "popOutNewWindow" | i18n }} +