From 7418f678744fbd15d3ee93084319b7bfe5034742 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Wed, 22 Oct 2025 08:36:31 -0500 Subject: [PATCH] [PM-24929] Clear route history cache for add/edit cipher views on tab change (#16755) * clear route history cache for add/edit cipher views on tab change * remove duplicate words --- .../browser/src/background/main.background.ts | 6 ++ .../view-cache/popup-router-cache.service.ts | 50 +++++++++++------ .../view-cache/popup-router-cache.spec.ts | 33 ++++++++++- .../popup-router-cache-background.service.ts | 55 +++++++++++++++++++ .../popup-view-cache-background.service.ts | 20 ++++++- apps/browser/src/popup/app-routing.module.ts | 6 +- 6 files changed, 147 insertions(+), 23 deletions(-) create mode 100644 apps/browser/src/platform/services/popup-router-cache-background.service.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index bcceac6fb84..7a4ee64070f 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -319,6 +319,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 { PopupRouterCacheBackgroundService } from "../platform/services/popup-router-cache-background.service"; import { PopupViewCacheBackgroundService } from "../platform/services/popup-view-cache-background.service"; import { BrowserSdkLoadService } from "../platform/services/sdk/browser-sdk-load.service"; import { BackgroundTaskSchedulerService } from "../platform/services/task-scheduler/background-task-scheduler.service"; @@ -488,6 +489,7 @@ export default class MainBackground { private nativeMessagingBackground: NativeMessagingBackground; private popupViewCacheBackgroundService: PopupViewCacheBackgroundService; + private popupRouterCacheBackgroundService: PopupRouterCacheBackgroundService; constructor() { // Services @@ -684,6 +686,9 @@ export default class MainBackground { this.globalStateProvider, this.taskSchedulerService, ); + this.popupRouterCacheBackgroundService = new PopupRouterCacheBackgroundService( + this.globalStateProvider, + ); this.migrationRunner = new MigrationRunner( this.storageService, @@ -1514,6 +1519,7 @@ export default class MainBackground { (this.eventUploadService as EventUploadService).init(true); this.popupViewCacheBackgroundService.startObservingMessages(); + this.popupRouterCacheBackgroundService.init(); await this.vaultTimeoutService.init(true); this.fido2Background.init(); 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 index 2e9746642f4..abb7c6405c2 100644 --- 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 @@ -5,6 +5,7 @@ import { Injectable, inject } from "@angular/core"; import { ActivatedRouteSnapshot, CanActivateFn, + Data, NavigationEnd, Router, UrlSerializer, @@ -14,7 +15,10 @@ import { filter, first, firstValueFrom, map, Observable, of, switchMap, tap } fr import { GlobalStateProvider } from "@bitwarden/common/platform/state"; -import { POPUP_ROUTE_HISTORY_KEY } from "../../../platform/services/popup-view-cache-background.service"; +import { + POPUP_ROUTE_HISTORY_KEY, + RouteHistoryCacheState, +} from "../../../platform/services/popup-view-cache-background.service"; import BrowserPopupUtils from "../../browser/browser-popup-utils"; /** @@ -42,8 +46,7 @@ export class PopupRouterCacheService { this.history$() .pipe(first()) .subscribe( - (history) => - Array.isArray(history) && history.forEach((location) => this.location.go(location)), + (history) => Array.isArray(history) && history.forEach(({ url }) => this.location.go(url)), ); // update state when route change occurs @@ -54,31 +57,33 @@ export class PopupRouterCacheService { // `Location.back()` can now be called successfully this.hasNavigated = true; }), - filter((_event: NavigationEnd) => { + map((event) => { const state: ActivatedRouteSnapshot = this.router.routerState.snapshot.root; let child = state.firstChild; while (child.firstChild) { child = child.firstChild; } - - return !child?.data?.doNotSaveUrl; + return { event, data: child.data }; }), - switchMap((event) => this.push(event.url)), + filter(({ data }) => { + return !data?.doNotSaveUrl; + }), + switchMap(({ event, data }) => this.push(event.url, data)), ) .subscribe(); } - history$(): Observable { + history$(): Observable { return this.state.state$; } - async setHistory(state: string[]): Promise { + async setHistory(state: RouteHistoryCacheState[]): Promise { return this.state.update(() => state); } /** Get the last item from the history stack, or `null` if empty */ - last$(): Observable { + last$(): Observable { return this.history$().pipe( map((history) => { if (!history || history.length === 0) { @@ -92,11 +97,24 @@ export class PopupRouterCacheService { /** * If in browser popup, push new route onto history stack */ - private async push(url: string) { - if (!BrowserPopupUtils.inPopup(window) || url === (await firstValueFrom(this.last$()))) { + private async push(url: string, data: Data) { + if ( + !BrowserPopupUtils.inPopup(window) || + url === (await firstValueFrom(this.last$().pipe(map((h) => h?.url)))) + ) { return; } - await this.state.update((prevState) => (prevState == null ? [url] : prevState.concat(url))); + + const routeEntry: RouteHistoryCacheState = { + url, + options: { + resetRouterCacheOnTabChange: data?.resetRouterCacheOnTabChange ?? false, + }, + }; + + await this.state.update((prevState) => + prevState == null ? [routeEntry] : prevState.concat(routeEntry), + ); } /** @@ -142,13 +160,13 @@ export const popupRouterCacheGuard = ((): Observable => { } return popupHistoryService.last$().pipe( - map((url: string) => { - if (!url) { + map((entry) => { + if (!entry) { return true; } popupHistoryService.markCacheRestored(); - return urlSerializer.parse(url); + return urlSerializer.parse(entry.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 index 22fb7bf99b9..3304a99023e 100644 --- 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 @@ -96,11 +96,31 @@ describe("Popup router cache guard", () => { // wait for router events subscription await flushPromises(); - expect(await firstValueFrom(service.history$())).toEqual(["/a", "/b"]); + expect(await firstValueFrom(service.history$())).toEqual([ + { + options: { + resetRouterCacheOnTabChange: false, + }, + url: "/a", + }, + { + options: { + resetRouterCacheOnTabChange: false, + }, + url: "/b", + }, + ]); await service.back(); - expect(await firstValueFrom(service.history$())).toEqual(["/a"]); + expect(await firstValueFrom(service.history$())).toEqual([ + { + options: { + resetRouterCacheOnTabChange: false, + }, + url: "/a", + }, + ]); }); it("does not save ignored routes", async () => { @@ -121,6 +141,13 @@ describe("Popup router cache guard", () => { await flushPromises(); - expect(await firstValueFrom(service.history$())).toEqual(["/a"]); + expect(await firstValueFrom(service.history$())).toEqual([ + { + options: { + resetRouterCacheOnTabChange: false, + }, + url: "/a", + }, + ]); }); }); diff --git a/apps/browser/src/platform/services/popup-router-cache-background.service.ts b/apps/browser/src/platform/services/popup-router-cache-background.service.ts new file mode 100644 index 00000000000..37e9f7cd4a0 --- /dev/null +++ b/apps/browser/src/platform/services/popup-router-cache-background.service.ts @@ -0,0 +1,55 @@ +import { switchMap, filter, map, first, of } from "rxjs"; + +import { GlobalStateProvider } from "@bitwarden/common/platform/state"; + +import { BrowserApi } from "../browser/browser-api"; +import { fromChromeEvent } from "../browser/from-chrome-event"; + +import { POPUP_ROUTE_HISTORY_KEY } from "./popup-view-cache-background.service"; + +export class PopupRouterCacheBackgroundService { + private popupRouteHistoryState = this.globalStateProvider.get(POPUP_ROUTE_HISTORY_KEY); + + constructor(private globalStateProvider: GlobalStateProvider) {} + + init() { + fromChromeEvent(chrome.tabs.onActivated) + .pipe( + switchMap((tabs) => BrowserApi.getTab(tabs[0].tabId)!), + switchMap((tab) => { + // FireFox sets the `url` to "about:blank" and won't populate the `url` until the `onUpdated` event + if (tab.url !== "about:blank") { + return of(tab); + } + + return fromChromeEvent(chrome.tabs.onUpdated).pipe( + first(), + switchMap(([tabId]) => BrowserApi.getTab(tabId)!), + ); + }), + map((tab) => tab.url || tab.pendingUrl), + filter((url) => !url?.startsWith(chrome.runtime.getURL(""))), + switchMap(() => + this.popupRouteHistoryState.update((state) => { + if (!state || state.length === 0) { + return state; + } + + const lastRoute = state.at(-1); + if (!lastRoute) { + return state; + } + + // When the last route has resetRouterCacheOnTabChange set + // Reset the route history to empty to force the user to the default route + if (lastRoute.options?.resetRouterCacheOnTabChange) { + return []; + } + + return state; + }), + ), + ) + .subscribe(); + } +} 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 index 49eae15fbbd..6a0a72ceccd 100644 --- a/apps/browser/src/platform/services/popup-view-cache-background.service.ts +++ b/apps/browser/src/platform/services/popup-view-cache-background.service.ts @@ -42,6 +42,22 @@ export type ViewCacheState = { options?: ViewCacheOptions; }; +export type RouteCacheOptions = { + /** + * When true, the route history will be reset on tab change and respective route was the last visited route. + * i.e. Upon the user re-opening the extension the route history will be empty and the user will be taken to the default route. + */ + resetRouterCacheOnTabChange?: boolean; +}; + +export type RouteHistoryCacheState = { + /** Route URL */ + url: string; + + /** Options for managing the route history cache */ + options?: RouteCacheOptions; +}; + /** We cannot use `UserKeyDefinition` because we must be able to store state when there is no active user. */ export const POPUP_VIEW_CACHE_KEY = KeyDefinition.record( POPUP_VIEW_MEMORY, @@ -51,9 +67,9 @@ export const POPUP_VIEW_CACHE_KEY = KeyDefinition.record( }, ); -export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition( +export const POPUP_ROUTE_HISTORY_KEY = new KeyDefinition( POPUP_VIEW_MEMORY, - "popup-route-history", + "popup-route-history-details", { deserializer: (jsonValue) => jsonValue, }, diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index e3d63d20c17..e57625d382a 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -59,6 +59,7 @@ import { ProtectedByComponent } from "../dirt/phishing-detection/pages/protected import { RemovePasswordComponent } from "../key-management/key-connector/remove-password.component"; import BrowserPopupUtils from "../platform/browser/browser-popup-utils"; import { popupRouterCacheGuard } from "../platform/popup/view-cache/popup-router-cache.service"; +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 { SendAddEditComponent as SendAddEditV2Component } from "../tools/popup/send-v2/add-edit/send-add-edit.component"; @@ -98,7 +99,7 @@ import { TabsV2Component } from "./tabs-v2.component"; /** * Data properties acceptable for use in extension route objects */ -export interface RouteDataProperties { +export interface RouteDataProperties extends RouteCacheOptions { elevation: RouteElevation; /** @@ -204,7 +205,7 @@ const routes: Routes = [ path: "add-cipher", component: AddEditV2Component, canActivate: [authGuard, debounceNavigationGuard()], - data: { elevation: 1 } satisfies RouteDataProperties, + data: { elevation: 1, resetRouterCacheOnTabChange: true } satisfies RouteDataProperties, runGuardsAndResolvers: "always", }, { @@ -214,6 +215,7 @@ const routes: Routes = [ data: { // Above "trash" elevation: 3, + resetRouterCacheOnTabChange: true, } satisfies RouteDataProperties, runGuardsAndResolvers: "always", },