1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-21 03:43:58 +00:00

initial refactor to consolidate logic using file-picker-popout.guard

This commit is contained in:
John Harrington
2025-12-11 10:00:53 -07:00
parent 20d53b8036
commit b4a1d9f437
20 changed files with 465 additions and 538 deletions

View File

@@ -3052,29 +3052,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."
},

View File

@@ -68,7 +68,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";
@@ -245,7 +245,7 @@ const routes: Routes = [
{
path: "attachments",
component: AttachmentsV2Component,
canActivate: [authGuard],
canActivate: [authGuard, filePickerPopoutGuard()],
data: { elevation: 4 } satisfies RouteDataProperties,
},
{
@@ -263,7 +263,7 @@ const routes: Routes = [
{
path: "import",
component: ImportBrowserV2Component,
canActivate: [authGuard, firefoxPopoutGuard()],
canActivate: [authGuard, filePickerPopoutGuard()],
data: { elevation: 1 } satisfies RouteDataProperties,
},
{
@@ -341,13 +341,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,
},
{

View File

@@ -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,

View File

@@ -204,7 +204,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 { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service";
import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service";
@@ -475,13 +474,6 @@ const safeProviders: SafeProvider[] = [
},
deps: [PlatformUtilsService],
}),
safeProvider({
provide: FilePopoutUtilsService,
useFactory: (platformUtilsService: PlatformUtilsService) => {
return new FilePopoutUtilsService(platformUtilsService);
},
deps: [PlatformUtilsService],
}),
safeProvider({
provide: DerivedStateProvider,
useClass: InlineDerivedStateProvider,

View File

@@ -1,11 +0,0 @@
<bit-callout
type="warning"
icon="bwi-external-link bwi-rotate-270 bwi-fw"
title="{{ 'sendFileCalloutHeader' | i18n }}"
(click)="popOutWindow()"
*ngIf="showFilePopoutMessage"
>
<div *ngIf="showChromiumFileWarning">{{ "sendLinuxChromiumFileWarning" | i18n }}</div>
<div *ngIf="showFirefoxFileWarning">{{ "sendFirefoxFileWarning" | i18n }}</div>
<div *ngIf="showSafariFileWarning">{{ "sendSafariFileWarning" | i18n }}</div>
</bit-callout>

View File

@@ -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);
}
}

View File

@@ -0,0 +1,380 @@
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("should open popout and block navigation for Chrome on Mac when not in popout or sidebar", async () => {
getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension);
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 not require popout for Edge on Mac (not Chrome)", async () => {
getDeviceSpy.mockReturnValue(DeviceType.EdgeExtension);
const guard = filePickerPopoutGuard();
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState));
// Edge/Opera/Vivaldi on Mac don't match the Chrome-specific Mac check
expect(openPopoutSpy).not.toHaveBeenCalled();
expect(closePopupSpy).not.toHaveBeenCalled();
expect(result).toBe(true);
});
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("should open popout for /import route", async () => {
const importState: RouterStateSnapshot = {
url: "/import",
} as RouterStateSnapshot;
const guard = filePickerPopoutGuard();
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 = filePickerPopoutGuard();
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 = filePickerPopoutGuard();
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, editSendState));
expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/edit-send");
expect(closePopupSpy).toHaveBeenCalledWith(window);
expect(result).toBe(false);
});
it("should open popout for /attachments route", async () => {
const attachmentsState: RouterStateSnapshot = {
url: "/attachments?cipherId=123",
} as RouterStateSnapshot;
const guard = filePickerPopoutGuard();
const result = await TestBed.runInInjectionContext(() => guard(mockRoute, attachmentsState));
expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/attachments?cipherId=123");
expect(closePopupSpy).toHaveBeenCalledWith(window);
expect(result).toBe(false);
});
it("should preserve query parameters on file picker routes", async () => {
const editSendWithParams: RouterStateSnapshot = {
url: "/edit-send?sendId=123&type=1",
} as RouterStateSnapshot;
const guard = filePickerPopoutGuard();
await TestBed.runInInjectionContext(() => guard(mockRoute, editSendWithParams));
expect(openPopoutSpy).toHaveBeenCalledWith("popup/index.html#/edit-send?sendId=123&type=1");
expect(closePopupSpy).toHaveBeenCalledWith(window);
});
});
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"));
});
});
});

View File

@@ -0,0 +1,78 @@
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";
/**
* 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
* - Chromium on Linux/Mac: Requires sidebar OR popout
* - Chromium on Windows: No special requirement
*
* @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 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: needs sidebar OR popout for file picker access
// All Chromium-based browsers (Chrome, Edge, Opera, Vivaldi) on Linux
const isChromiumBased = [
DeviceType.ChromeExtension,
DeviceType.EdgeExtension,
DeviceType.OperaExtension,
DeviceType.VivaldiExtension,
].includes(deviceType);
const isLinux = window?.navigator?.userAgent?.indexOf("Linux") !== -1;
if (isChromiumBased && isLinux && !inPopout && !inSidebar) {
needsPopout = true;
}
// Chrome (specifically) on Mac: needs sidebar OR popout for file picker access
const isMac =
deviceType === DeviceType.ChromeExtension &&
window?.navigator?.appVersion.includes("Mac OS X");
if (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
};
}

View File

@@ -1,208 +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);
});
it("should preserve query parameters on file picker routes", async () => {
const editSendWithParams: RouterStateSnapshot = {
url: "/edit-send?sendId=123&mode=edit",
} as RouterStateSnapshot;
const guard = firefoxPopoutGuard();
await TestBed.runInInjectionContext(() => guard(mockRoute, editSendWithParams));
expect(openPopoutSpy).toHaveBeenCalledWith(
"popup/index.html#/edit-send?sendId=123&mode=edit",
);
expect(closePopupSpy).toHaveBeenCalledWith(window);
});
});
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);
});
});
});

View File

@@ -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
};
}

View File

@@ -10,8 +10,6 @@
>
</tools-send-form>
<send-file-popout-dialog-container [config]="config"></send-file-popout-dialog-container>
<popup-footer slot="footer">
<button bitButton type="submit" form="sendForm" buttonType="primary" #submitBtn>
{{ "save" | i18n }}

View File

@@ -33,7 +33,6 @@ import { PopupBackBrowserDirective } from "../../../../platform/popup/layout/pop
import { PopupFooterComponent } from "../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page.component";
import { SendFilePopoutDialogContainerComponent } from "../send-file-popout-dialog/send-file-popout-dialog-container.component";
/**
* Helper class to parse query parameters for the AddEdit route.
@@ -81,7 +80,6 @@ export type AddEditQueryParams = Partial<Record<keyof QueryParams, string>>;
PopupPageComponent,
PopupHeaderComponent,
PopupFooterComponent,
SendFilePopoutDialogContainerComponent,
SendFormModule,
AsyncActionsModule,
PopupBackBrowserDirective,

View File

@@ -1,41 +0,0 @@
import { CommonModule } from "@angular/common";
import { Component, input, OnInit } from "@angular/core";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { SendType } from "@bitwarden/common/tools/send/enums/send-type";
import { CenterPositionStrategy, DialogService } from "@bitwarden/components";
import { SendFormConfig } from "@bitwarden/send-ui";
import { FilePopoutUtilsService } from "../../services/file-popout-utils.service";
import { SendFilePopoutDialogComponent } from "./send-file-popout-dialog.component";
// 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-container",
templateUrl: "./send-file-popout-dialog-container.component.html",
imports: [JslibModule, CommonModule],
})
export class SendFilePopoutDialogContainerComponent implements OnInit {
// FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
// eslint-disable-next-line @angular-eslint/prefer-signals
config = input.required<SendFormConfig>();
constructor(
private dialogService: DialogService,
private filePopoutUtilsService: FilePopoutUtilsService,
) {}
ngOnInit() {
if (
this.config().sendType === SendType.File &&
this.config().mode === "add" &&
this.filePopoutUtilsService.showFilePopoutMessage(window)
) {
this.dialogService.open(SendFilePopoutDialogComponent, {
positionStrategy: new CenterPositionStrategy(),
});
}
}
}

View File

@@ -1,20 +0,0 @@
<bit-simple-dialog dialogSize="default">
<div bitDialogIcon>
<i class="bwi bwi-info-circle bwi-2x tw-text-info" aria-hidden="true"></i>
</div>
<ng-container bitDialogContent>
<div bitTypography="h3">
{{ "sendFilePopoutDialogText" | i18n }}
</div>
<div bitTypography="body1">{{ "sendFilePopoutDialogDesc" | i18n }}</div>
</ng-container>
<ng-container bitDialogFooter>
<button buttonType="primary" bitButton type="button" (click)="popOutWindow()">
{{ "popOut" | i18n }}
<i class="bwi bwi-popout tw-ml-1" aria-hidden="true"></i>
</button>
<button bitButton buttonType="secondary" type="button" (click)="close()">
{{ "cancel" | i18n }}
</button>
</ng-container>
</bit-simple-dialog>

View File

@@ -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();
}
}

View File

@@ -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");
}
}

View File

@@ -10,8 +10,7 @@
<app-premium-badge></app-premium-badge>
</div>
<ng-container slot="end">
<i class="bwi bwi-popout" aria-hidden="true" *ngIf="openAttachmentsInPopout"></i>
<i class="bwi bwi-angle-right" aria-hidden="true" *ngIf="!openAttachmentsInPopout"></i>
<i class="bwi bwi-angle-right" aria-hidden="true"></i>
</ng-container>
</button>
</bit-item>

View File

@@ -19,9 +19,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", () => {
@@ -30,9 +27,6 @@ describe("OpenAttachmentsComponent", () => {
let router: Router;
const showToast = jest.fn();
const hasPremiumFromAnySource$ = new BehaviorSubject<boolean>(true);
const openCurrentPagePopout = jest
.spyOn(BrowserPopupUtils, "openCurrentPagePopout")
.mockResolvedValue(null);
const cipherView = {
id: "5555-444-3333",
type: CipherType.Login,
@@ -54,7 +48,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 = {
@@ -68,11 +61,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");
@@ -101,10 +92,6 @@ describe("OpenAttachmentsComponent", () => {
provide: OrganizationService,
useValue: { organizations$ },
},
{
provide: FilePopoutUtilsService,
useValue: { showFilePopoutMessage },
},
{
provide: AccountService,
useValue: accountService,
@@ -128,8 +115,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();
@@ -138,20 +124,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 () => {

View File

@@ -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<void> {
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);
}
}
}