diff --git a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts index c447ccffd78..44060f991ff 100644 --- a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts +++ b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts @@ -28,6 +28,7 @@ import { UserId } from "@bitwarden/common/types/guid"; import { ButtonModule, I18nMockService } from "@bitwarden/components"; import { RegistrationCheckEmailIcon } from "../../../../../../libs/auth/src/angular/icons/registration-check-email.icon"; +import { PopupRouterCacheService } from "../../../platform/popup/view-cache/popup-router-cache.service"; import { ExtensionAnonLayoutWrapperDataService } from "./extension-anon-layout-wrapper-data.service"; import { @@ -145,7 +146,15 @@ const decorators = (options: { ], }), applicationConfig({ - providers: [importProvidersFrom(RouterModule.forRoot(options.routes))], + providers: [ + importProvidersFrom(RouterModule.forRoot(options.routes)), + { + provide: PopupRouterCacheService, + useValue: { + back() {}, + } as Partial, + }, + ], }), ]; }; diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c738b6eed96..7d38682c200 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -236,6 +236,7 @@ import I18nService from "../platform/services/i18n.service"; import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service"; import { BackgroundPlatformUtilsService } from "../platform/services/platform-utils/background-platform-utils.service"; import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service"; +import { PopupViewCacheBackgroundService } from "../platform/services/popup-view-cache-background.service"; import { BackgroundTaskSchedulerService } from "../platform/services/task-scheduler/background-task-scheduler.service"; import { ForegroundTaskSchedulerService } from "../platform/services/task-scheduler/foreground-task-scheduler.service"; import { BackgroundMemoryStorageService } from "../platform/storage/background-memory-storage.service"; @@ -371,6 +372,8 @@ export default class MainBackground { private isSafari: boolean; private nativeMessagingBackground: NativeMessagingBackground; + private popupViewCacheBackgroundService: PopupViewCacheBackgroundService; + constructor(public popupOnlyContext: boolean = false) { // Services const lockedCallback = async (userId?: string) => { @@ -571,6 +574,10 @@ export default class MainBackground { logoutCallback, ); + this.popupViewCacheBackgroundService = new PopupViewCacheBackgroundService( + this.globalStateProvider, + ); + const migrationRunner = new MigrationRunner( this.storageService, this.logService, @@ -1201,6 +1208,8 @@ export default class MainBackground { await (this.i18nService as I18nService).init(); (this.eventUploadService as EventUploadService).init(true); + this.popupViewCacheBackgroundService.startObservingTabChanges(); + if (this.popupOnlyContext) { return; } @@ -1295,6 +1304,7 @@ export default class MainBackground { }), ), ); + await this.popupViewCacheBackgroundService.clearState(); await this.accountService.switchAccount(userId); await switchPromise; // Clear sequentialized caches @@ -1383,6 +1393,7 @@ export default class MainBackground { this.vaultTimeoutSettingsService.clear(userBeingLoggedOut), this.vaultFilterService.clear(), this.biometricStateService.logout(userBeingLoggedOut), + this.popupViewCacheBackgroundService.clearState(), /* We intentionally do not clear: * - autofillSettingsService * - badgeSettingsService diff --git a/apps/browser/src/platform/popup/layout/popup-header.component.ts b/apps/browser/src/platform/popup/layout/popup-header.component.ts index 1e41f7ccbe0..fcf7f57c89f 100644 --- a/apps/browser/src/platform/popup/layout/popup-header.component.ts +++ b/apps/browser/src/platform/popup/layout/popup-header.component.ts @@ -1,5 +1,5 @@ import { BooleanInput, coerceBooleanProperty } from "@angular/cdk/coercion"; -import { CommonModule, Location } from "@angular/common"; +import { CommonModule } from "@angular/common"; import { Component, Input, Signal, inject } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -10,6 +10,8 @@ import { TypographyModule, } from "@bitwarden/components"; +import { PopupRouterCacheService } from "../view-cache/popup-router-cache.service"; + import { PopupPageComponent } from "./popup-page.component"; @Component({ @@ -19,6 +21,7 @@ import { PopupPageComponent } from "./popup-page.component"; imports: [TypographyModule, CommonModule, IconButtonModule, JslibModule, AsyncActionsModule], }) export class PopupHeaderComponent { + private popupRouterCacheService = inject(PopupRouterCacheService); protected pageContentScrolled: Signal = inject(PopupPageComponent).isScrolled; /** Background color */ @@ -46,8 +49,6 @@ export class PopupHeaderComponent { **/ @Input() backAction: FunctionReturningAwaitable = async () => { - this.location.back(); + return this.popupRouterCacheService.back(); }; - - constructor(private location: Location) {} } diff --git a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts index 3d067b53056..affa804cc79 100644 --- a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts +++ b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts @@ -16,6 +16,8 @@ import { SectionComponent, } from "@bitwarden/components"; +import { PopupRouterCacheService } from "../view-cache/popup-router-cache.service"; + import { PopupFooterComponent } from "./popup-footer.component"; import { PopupHeaderComponent } from "./popup-header.component"; import { PopupPageComponent } from "./popup-page.component"; @@ -334,6 +336,12 @@ export default { { useHash: true }, ), ), + { + provide: PopupRouterCacheService, + useValue: { + back() {}, + } as Partial, + }, ], }), ], diff --git a/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts new file mode 100644 index 00000000000..52b08ac09ab --- /dev/null +++ b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts @@ -0,0 +1,131 @@ +import { Location } from "@angular/common"; +import { Injectable, inject } from "@angular/core"; +import { + ActivatedRouteSnapshot, + CanActivateFn, + NavigationEnd, + Router, + UrlSerializer, +} from "@angular/router"; +import { filter, first, firstValueFrom, map, Observable, of, switchMap } from "rxjs"; + +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { GlobalStateProvider } from "@bitwarden/common/platform/state"; + +import { POPUP_ROUTE_HISTORY_KEY } from "../../../platform/services/popup-view-cache-background.service"; +import BrowserPopupUtils from "../browser-popup-utils"; + +/** + * Preserves route history when opening and closing the popup + * + * Routes marked with `doNotSaveUrl` will not be stored + **/ +@Injectable({ + providedIn: "root", +}) +export class PopupRouterCacheService { + private router = inject(Router); + private state = inject(GlobalStateProvider).get(POPUP_ROUTE_HISTORY_KEY); + private location = inject(Location); + + constructor() { + // init history with existing state + this.history$() + .pipe(first()) + .subscribe((history) => history.forEach((location) => this.location.go(location))); + + // update state when route change occurs + this.router.events + .pipe( + filter((event) => event instanceof NavigationEnd), + filter((_event: NavigationEnd) => { + const state: ActivatedRouteSnapshot = this.router.routerState.snapshot.root; + + let child = state.firstChild; + while (child.firstChild) { + child = child.firstChild; + } + + return !child?.data?.doNotSaveUrl ?? true; + }), + switchMap((event) => this.push(event.url)), + ) + .subscribe(); + } + + history$(): Observable { + return this.state.state$; + } + + async setHistory(state: string[]): Promise { + return this.state.update(() => state); + } + + /** Get the last item from the history stack, or `null` if empty */ + last$(): Observable { + return this.history$().pipe( + map((history) => { + if (!history || history.length === 0) { + return null; + } + return history[history.length - 1]; + }), + ); + } + + /** + * If in browser popup, push new route onto history stack + */ + private async push(url: string): Promise { + if (!BrowserPopupUtils.inPopup(window) || url === (await firstValueFrom(this.last$()))) { + return; + } + await this.state.update((prevState) => (prevState == null ? [url] : prevState.concat(url))); + } + + /** + * Navigate back in history + */ + async back() { + await this.state.update((prevState) => prevState.slice(0, -1)); + + const url = this.router.url; + this.location.back(); + if (url !== this.router.url) { + return; + } + + // if no history is present, fallback to vault page + await this.router.navigate([""]); + } +} + +/** + * Redirect to the last visited route. Should be applied to root route. + * + * If `FeatureFlag.PersistPopupView` is disabled, do nothing. + **/ +export const popupRouterCacheGuard = (() => { + const configService = inject(ConfigService); + const popupHistoryService = inject(PopupRouterCacheService); + const urlSerializer = inject(UrlSerializer); + + return configService.getFeatureFlag$(FeatureFlag.PersistPopupView).pipe( + switchMap((featureEnabled) => { + if (!featureEnabled) { + return of(true); + } + + return popupHistoryService.last$().pipe( + map((url: string) => { + if (!url) { + return true; + } + + return urlSerializer.parse(url); + }), + ); + }), + ); +}) satisfies CanActivateFn; diff --git a/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts b/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts new file mode 100644 index 00000000000..dde7f10500b --- /dev/null +++ b/apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts @@ -0,0 +1,113 @@ +import { Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { Router, UrlSerializer, UrlTree } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { mock } from "jest-mock-extended"; +import { firstValueFrom, of } from "rxjs"; + +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { GlobalStateProvider } from "@bitwarden/common/platform/state"; +import { FakeGlobalStateProvider } from "@bitwarden/common/spec"; + +import { PopupRouterCacheService, popupRouterCacheGuard } from "./popup-router-cache.service"; + +const flushPromises = async () => await new Promise(process.nextTick); + +@Component({ template: "" }) +export class EmptyComponent {} + +describe("Popup router cache guard", () => { + const configServiceMock = mock(); + const fakeGlobalStateProvider = new FakeGlobalStateProvider(); + + let testBed: TestBed; + let serializer: UrlSerializer; + let router: Router; + + let service: PopupRouterCacheService; + + beforeEach(async () => { + jest.spyOn(configServiceMock, "getFeatureFlag$").mockReturnValue(of(true)); + + testBed = TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { path: "a", component: EmptyComponent }, + { path: "b", component: EmptyComponent }, + { + path: "c", + component: EmptyComponent, + data: { doNotSaveUrl: true }, + }, + ]), + ], + providers: [ + { provide: ConfigService, useValue: configServiceMock }, + { provide: GlobalStateProvider, useValue: fakeGlobalStateProvider }, + ], + }); + + await testBed.compileComponents(); + + router = testBed.inject(Router); + serializer = testBed.inject(UrlSerializer); + + service = testBed.inject(PopupRouterCacheService); + + await service.setHistory([]); + }); + + it("returns true if the history stack is empty", async () => { + const response = await firstValueFrom( + testBed.runInInjectionContext(() => popupRouterCacheGuard()), + ); + + expect(response).toBe(true); + }); + + it("redirects to the latest stored route", async () => { + await router.navigate(["a"]); + await router.navigate(["b"]); + + const response = (await firstValueFrom( + testBed.runInInjectionContext(() => popupRouterCacheGuard()), + )) as UrlTree; + + expect(serializer.serialize(response)).toBe("/b"); + }); + + it("back method redirects to the previous route", async () => { + await router.navigate(["a"]); + await router.navigate(["b"]); + + // wait for router events subscription + await flushPromises(); + + expect(await firstValueFrom(service.history$())).toEqual(["/a", "/b"]); + + await service.back(); + + expect(await firstValueFrom(service.history$())).toEqual(["/a"]); + }); + + it("does not save ignored routes", async () => { + await router.navigate(["a"]); + await router.navigate(["b"]); + await router.navigate(["c"]); + + const response = (await firstValueFrom( + testBed.runInInjectionContext(() => popupRouterCacheGuard()), + )) as UrlTree; + + expect(serializer.serialize(response)).toBe("/b"); + }); + + it("does not save duplicate routes", async () => { + await router.navigate(["a"]); + await router.navigate(["a"]); + + await flushPromises(); + + expect(await firstValueFrom(service.history$())).toEqual(["/a"]); + }); +}); diff --git a/apps/browser/src/platform/services/popup-view-cache-background.service.ts b/apps/browser/src/platform/services/popup-view-cache-background.service.ts new file mode 100644 index 00000000000..3427a02d3fa --- /dev/null +++ b/apps/browser/src/platform/services/popup-view-cache-background.service.ts @@ -0,0 +1,63 @@ +import { switchMap, merge, delay, filter, map } from "rxjs"; + +import { + POPUP_VIEW_MEMORY, + KeyDefinition, + GlobalStateProvider, +} from "@bitwarden/common/platform/state"; + +import { fromChromeEvent } from "../browser/from-chrome-event"; + +const popupClosedPortName = "new_popup"; + +export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition( + POPUP_VIEW_MEMORY, + "popup-route-history", + { + deserializer: (jsonValue) => jsonValue, + }, +); + +export class PopupViewCacheBackgroundService { + private popupRouteHistoryState = this.globalStateProvider.get(POPUP_ROUTE_HISTORY_KEY); + + constructor(private globalStateProvider: GlobalStateProvider) {} + + startObservingTabChanges() { + merge( + // on tab changed, excluding extension tabs + fromChromeEvent(chrome.tabs.onActivated).pipe( + switchMap(([tabInfo]) => chrome.tabs.get(tabInfo.tabId)), + map((tab) => tab.url || tab.pendingUrl), + filter((url) => !url.startsWith(chrome.runtime.getURL(""))), + ), + + // on popup closed, with 2 minute delay that is cancelled by re-opening the popup + fromChromeEvent(chrome.runtime.onConnect).pipe( + filter(([port]) => port.name === popupClosedPortName), + switchMap(([port]) => fromChromeEvent(port.onDisconnect).pipe(delay(1000 * 60 * 2))), + ), + ) + .pipe(switchMap(() => this.clearState())) + .subscribe(); + } + + async clearState() { + return Promise.all([ + this.popupRouteHistoryState.update(() => [], { shouldUpdate: this.objNotEmpty }), + ]); + } + + private objNotEmpty(obj: object): boolean { + return Object.keys(obj ?? {}).length !== 0; + } +} + +/** + * Communicates to {@link PopupViewCacheBackgroundService} that the extension popup has been closed. + * + * Call in the foreground. + **/ +export const initPopupClosedListener = () => { + chrome.runtime.connect({ name: popupClosedPortName }); +}; diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index b7b4e41778b..5c4c01bfc16 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -48,6 +48,7 @@ import { NotificationsSettingsV1Component } from "../autofill/popup/settings/not import { NotificationsSettingsComponent } from "../autofill/popup/settings/notifications.component"; import { PremiumComponent } from "../billing/popup/settings/premium.component"; import BrowserPopupUtils from "../platform/popup/browser-popup-utils"; +import { popupRouterCacheGuard } from "../platform/popup/view-cache/popup-router-cache.service"; import { GeneratorComponent } from "../tools/popup/generator/generator.component"; import { PasswordGeneratorHistoryComponent } from "../tools/popup/generator/password-generator-history.component"; import { SendAddEditComponent } from "../tools/popup/send/send-add-edit.component"; @@ -105,6 +106,7 @@ const routes: Routes = [ pathMatch: "full", children: [], // Children lets us have an empty component. canActivate: [ + popupRouterCacheGuard, redirectGuard({ loggedIn: "/tabs/current", loggedOut: "/home", locked: "/lock" }), ], }, diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 287e9096684..7ac3e02160d 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -20,6 +20,7 @@ import { } from "@bitwarden/components"; import { BrowserApi } from "../platform/browser/browser-api"; +import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service"; import { BrowserSendStateService } from "../tools/popup/services/browser-send-state.service"; import { VaultBrowserStateService } from "../vault/services/vault-browser-state.service"; @@ -59,6 +60,8 @@ export class AppComponent implements OnInit, OnDestroy { ) {} async ngOnInit() { + initPopupClosedListener(); + // Component states must not persist between closing and reopening the popup, otherwise they become dead objects // Clear them aggressively to make sure this doesn't occur await this.clearComponentStates(); diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts index 53a0441eecd..50e5531743a 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts @@ -34,6 +34,7 @@ import { CurrentAccountComponent } from "../../../auth/popup/account-switching/c 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"; +import { PopupRouterCacheService } from "../../../platform/popup/view-cache/popup-router-cache.service"; import { SendV2Component, SendState } from "./send-v2.component"; @@ -102,6 +103,7 @@ describe("SendV2Component", () => { { provide: SendItemsService, useValue: sendItemsService }, { provide: I18nService, useValue: { t: (key: string) => key } }, { provide: SendListFiltersService, useValue: sendListFiltersService }, + { provide: PopupRouterCacheService, useValue: mock() }, ], }).compileComponents(); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts index d25dfadf5bc..d37bc367110 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts @@ -7,6 +7,8 @@ import { mock, MockProxy } from "jest-mock-extended"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherFormGeneratorComponent } from "@bitwarden/vault"; +import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service"; + import { GeneratorDialogParams, GeneratorDialogResult, @@ -39,6 +41,7 @@ describe("VaultGeneratorDialogComponent", () => { { provide: I18nService, useValue: { t: (key: string) => key } }, { provide: DIALOG_DATA, useValue: dialogData }, { provide: DialogRef, useValue: mockDialogRef }, + { provide: PopupRouterCacheService, useValue: mock() }, ], }) .overrideComponent(VaultGeneratorDialogComponent, { diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 3b2849e0f54..87b372d3b13 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -13,6 +13,7 @@ export enum FeatureFlag { AC1795_UpdatedSubscriptionStatusSection = "AC-1795_updated-subscription-status-section", EnableDeleteProvider = "AC-1218-delete-provider", ExtensionRefresh = "extension-refresh", + PersistPopupView = "persist-popup-view", RestrictProviderAccess = "restrict-provider-access", PM4154_BulkEncryptionService = "PM-4154-bulk-encryption-service", UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection", @@ -54,6 +55,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.AC1795_UpdatedSubscriptionStatusSection]: FALSE, [FeatureFlag.EnableDeleteProvider]: FALSE, [FeatureFlag.ExtensionRefresh]: FALSE, + [FeatureFlag.PersistPopupView]: FALSE, [FeatureFlag.RestrictProviderAccess]: FALSE, [FeatureFlag.PM4154_BulkEncryptionService]: FALSE, [FeatureFlag.UseTreeWalkerApiForPageDetailsCollection]: FALSE, diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index cbf4cd28f2f..f3880633e06 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -111,6 +111,9 @@ export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); export const DESKTOP_SETTINGS_DISK = new StateDefinition("desktopSettings", "disk"); export const ENVIRONMENT_DISK = new StateDefinition("environment", "disk"); export const ENVIRONMENT_MEMORY = new StateDefinition("environment", "memory"); +export const POPUP_VIEW_MEMORY = new StateDefinition("popupView", "memory", { + browser: "memory-large-object", +}); export const SYNC_DISK = new StateDefinition("sync", "disk", { web: "memory" }); export const THEMING_DISK = new StateDefinition("theming", "disk", { web: "disk-local" }); export const TRANSLATION_DISK = new StateDefinition("translation", "disk", { web: "disk-local" }); diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 7900fddbe06..092b7dba396 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -13,10 +13,6 @@ import { CollectionView } from "@bitwarden/common/vault/models/view/collection.v import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { SearchModule } from "@bitwarden/components"; -import { PopupFooterComponent } from "../../../../apps/browser/src/platform/popup/layout/popup-footer.component"; -import { PopupHeaderComponent } from "../../../../apps/browser/src/platform/popup/layout/popup-header.component"; -import { PopupPageComponent } from "../../../../apps/browser/src/platform/popup/layout/popup-page.component"; - import { AdditionalOptionsComponent } from "./additional-options/additional-options.component"; import { AttachmentsV2ViewComponent } from "./attachments/attachments-v2-view.component"; import { AutofillOptionsViewComponent } from "./autofill-options/autofill-options-view.component"; @@ -35,9 +31,6 @@ import { ViewIdentitySectionsComponent } from "./view-identity-sections/view-ide CommonModule, SearchModule, JslibModule, - PopupPageComponent, - PopupHeaderComponent, - PopupFooterComponent, ItemDetailsV2Component, AdditionalOptionsComponent, AttachmentsV2ViewComponent,