bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-15 07:54:55 +00:00
Code Issues Releases Wiki Activity

enforce popout on Safari with test coverage

Browse Source
This commit is contained in:
John Harrington
2025-12-12 08:28:53 -07:00
parent 6619d4486a
commit 541d216df1
2 changed files with 54 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

75
apps/browser/src/tools/popup/guards/file-picker-popout.guard.spec.ts
View File

@@ -84,15 +84,40 @@ describe("filePickerPopoutGuard", () => {
inSidebarSpy.mockReturnValue(false);
});
it("should allow navigation without popout requirement", async () => {
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", () => {
@@ -167,28 +192,24 @@ describe("filePickerPopoutGuard", () => {
});
});
it("should open popout and block navigation for Chrome on Mac when not in popout or sidebar", async () => {
getDeviceSpy.mockReturnValue(DeviceType.ChromeExtension);
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));
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);
});
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);
@@ -299,18 +320,6 @@ describe("filePickerPopoutGuard", () => {
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", () => {

26
apps/browser/src/tools/popup/guards/file-picker-popout.guard.ts
View File

@@ -11,9 +11,9 @@ import { DeviceType } from "@bitwarden/common/enums";
*
* Browser-specific requirements:
* - Firefox: Requires sidebar OR popout (crashes with file picker in popup: https://bugzilla.mozilla.org/show_bug.cgi?id=1292701)
* - Chromium on Linux/Mac: Requires sidebar OR popout
* - Safari: Requires popout only
* - All Chromium browsers (Chrome, Edge, Opera, Vivaldi) on Linux/Mac: Requires sidebar OR popout
* - Chromium on Windows: No special requirement
* - Safari: No special requirement
*
* @returns CanActivateFn that opens popout and blocks navigation when file picker access is needed
*/
@@ -33,8 +33,14 @@ export function filePickerPopoutGuard(): CanActivateFn {
needsPopout = true;
}
// Chromium on Linux: needs sidebar OR popout for file picker access
// All Chromium-based browsers (Chrome, Edge, Opera, Vivaldi) on Linux
// 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,
@@ -43,17 +49,9 @@ export function filePickerPopoutGuard(): CanActivateFn {
].includes(deviceType);
const isLinux = window?.navigator?.userAgent?.indexOf("Linux") !== -1;
const isMac = window?.navigator?.appVersion?.includes("Mac OS X");
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) {
if (isChromiumBased && (isLinux || isMac) && !inPopout && !inSidebar) {
needsPopout = true;
}