From 1de8d928994528b13928adbd7cc7a637d62aa13d Mon Sep 17 00:00:00 2001 From: Jeffrey Holland Date: Mon, 10 Nov 2025 13:21:46 +0100 Subject: [PATCH 1/5] Fix stale data issue in new login popout --- .../add-edit/add-edit-v2.component.ts | 49 ++++++++++++++++++- .../popup/utils/vault-popout-window.spec.ts | 40 +++++++++++++++ .../vault/popup/utils/vault-popout-window.ts | 24 +++++++-- 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index 60e44cefbdf1..45288bb53ced 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, OnInit } from "@angular/core"; +import { Component, OnInit, OnDestroy } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormsModule } from "@angular/forms"; import { ActivatedRoute, Params, Router } from "@angular/router"; @@ -158,7 +158,7 @@ export type AddEditQueryParams = Partial>; IconButtonModule, ], }) -export class AddEditV2Component implements OnInit { +export class AddEditV2Component implements OnInit, OnDestroy { headerText: string; config: CipherFormConfig; canDeleteCipher$: Observable; @@ -200,12 +200,57 @@ export class AddEditV2Component implements OnInit { this.subscribeToParams(); } + private messageListener: (message: any) => void; + async ngOnInit() { this.fido2PopoutSessionData = await firstValueFrom(this.fido2PopoutSessionData$); if (BrowserPopupUtils.inPopout(window)) { this.popupCloseWarningService.enable(); } + + // Listen for messages to reload cipher data when already open + this.messageListener = async (message: any) => { + if (message?.command === "reloadAddEditCipherData") { + // Reload the cipher data with fresh values from the form + await this.reloadCipherData(); + } + }; + BrowserApi.addListener(chrome.runtime.onMessage, this.messageListener); + } + + ngOnDestroy() { + if (this.messageListener) { + chrome.runtime.onMessage.removeListener(this.messageListener); + } + } + + /** + * Reloads the cipher data when the popup is already open and new form data is submitted. + */ + private async reloadCipherData() { + if (!this.config) { + return; + } + + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + const latestCipherInfo = await firstValueFrom( + this.cipherService.addEditCipherInfo$(activeUserId), + ); + + if (latestCipherInfo != null) { + this.config = { + ...this.config, + initialValues: { + ...this.config.initialValues, + ...mapAddEditCipherInfoToInitialValues(latestCipherInfo), + }, + }; + + // Be sure to clear the "cached" cipher info, so it doesn't get used again + await this.cipherService.setAddEditCipherInfo(null, activeUserId); + } } /** diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts index 4597c0042901..3389228dda45 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts @@ -2,6 +2,7 @@ import { mock } from "jest-mock-extended"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { BrowserApi } from "../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; import { @@ -23,6 +24,19 @@ describe("VaultPopoutWindow", () => { .spyOn(BrowserPopupUtils, "closeSingleActionPopout") .mockImplementation(); + beforeEach(() => { + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([]); + jest.spyOn(BrowserApi, "updateWindowProperties").mockResolvedValue(); + global.chrome = { + ...global.chrome, + runtime: { + ...global.chrome?.runtime, + sendMessage: jest.fn().mockResolvedValue(undefined), + getURL: jest.fn((path) => `chrome-extension://extension-id/${path}`), + }, + }; + }); + afterEach(() => { jest.clearAllMocks(); }); @@ -123,6 +137,32 @@ describe("VaultPopoutWindow", () => { }, ); }); + + it("sends a message to refresh data when the popup is already open", async () => { + const existingPopupTab = { + id: 123, + windowId: 456, + url: `chrome-extension://extension-id/popup/index.html#/edit-cipher?singleActionPopout=${VaultPopoutType.addEditVaultItem}_${CipherType.Login}`, + } as chrome.tabs.Tab; + + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([existingPopupTab]); + const sendMessageSpy = jest.spyOn(chrome.runtime, "sendMessage"); + const updateWindowSpy = jest.spyOn(BrowserApi, "updateWindowProperties"); + + await openAddEditVaultItemPopout( + mock({ windowId: 1, url: "https://jest-testing-website.com" }), + { + cipherType: CipherType.Login, + }, + ); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledWith({ + command: "reloadAddEditCipherData", + data: { cipherId: undefined, cipherType: CipherType.Login }, + }); + expect(updateWindowSpy).toHaveBeenCalledWith(456, { focused: true }); + }); }); describe("closeAddEditVaultItemPopout", () => { diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.ts index 3dae96b6cc7f..3f25a2feab1c 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.ts @@ -97,6 +97,11 @@ async function openAddEditVaultItemPopout( let singleActionKey = VaultPopoutType.addEditVaultItem; let addEditCipherUrl = "popup/index.html#/edit-cipher"; let queryParamToken = "?"; + const extensionUrl = chrome.runtime.getURL("popup/index.html"); + const existingPopupTabs = await BrowserApi.tabsQuery({ url: `${extensionUrl}*` }); + const existingPopup = existingPopupTabs.find((tab) => + tab.url?.includes(`singleActionPopout=${singleActionKey}`), + ); const formatQueryString = (key: string, value: string) => { const queryString = `${queryParamToken}${key}=${value}`; queryParamToken = "&"; @@ -115,10 +120,21 @@ async function openAddEditVaultItemPopout( addEditCipherUrl += formatQueryString("uri", url); } - await BrowserPopupUtils.openPopout(addEditCipherUrl, { - singleActionKey, - senderWindowId: windowId, - }); + // Check if the an existing popup is already open + if (existingPopup) { + await chrome.runtime.sendMessage({ + command: "reloadAddEditCipherData", + data: { cipherId, cipherType }, + }); + await BrowserApi.updateWindowProperties(existingPopup.windowId, { + focused: true, + }); + } else { + await BrowserPopupUtils.openPopout(addEditCipherUrl, { + singleActionKey, + senderWindowId: windowId, + }); + } } /** From 0357842bee0e48e22761a0551837d7c0ac648ebc Mon Sep 17 00:00:00 2001 From: Jeffrey Holland Date: Mon, 10 Nov 2025 15:06:11 +0100 Subject: [PATCH 2/5] Update the comments --- .../components/vault-v2/add-edit/add-edit-v2.component.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index 45288bb53ced..ae1d6b6d1813 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -209,10 +209,9 @@ export class AddEditV2Component implements OnInit, OnDestroy { this.popupCloseWarningService.enable(); } - // Listen for messages to reload cipher data when already open + // Listen for messages to reload cipher data when the pop up is already open this.messageListener = async (message: any) => { if (message?.command === "reloadAddEditCipherData") { - // Reload the cipher data with fresh values from the form await this.reloadCipherData(); } }; From 43b3eb27964d2bf2e6b6ac05ad1e4ef92de7d3e1 Mon Sep 17 00:00:00 2001 From: Jeffrey Holland Date: Mon, 10 Nov 2025 15:21:40 +0100 Subject: [PATCH 3/5] Address critical claude code bot suggestions --- .../vault-v2/add-edit/add-edit-v2.component.ts | 2 +- .../src/vault/popup/utils/vault-popout-window.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index ae1d6b6d1813..f389ad0a8ca7 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -220,7 +220,7 @@ export class AddEditV2Component implements OnInit, OnDestroy { ngOnDestroy() { if (this.messageListener) { - chrome.runtime.onMessage.removeListener(this.messageListener); + BrowserApi.removeListener(chrome.runtime.onMessage, this.messageListener); } } diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.ts index 3f25a2feab1c..5425a2c2bb0f 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.ts @@ -97,11 +97,6 @@ async function openAddEditVaultItemPopout( let singleActionKey = VaultPopoutType.addEditVaultItem; let addEditCipherUrl = "popup/index.html#/edit-cipher"; let queryParamToken = "?"; - const extensionUrl = chrome.runtime.getURL("popup/index.html"); - const existingPopupTabs = await BrowserApi.tabsQuery({ url: `${extensionUrl}*` }); - const existingPopup = existingPopupTabs.find((tab) => - tab.url?.includes(`singleActionPopout=${singleActionKey}`), - ); const formatQueryString = (key: string, value: string) => { const queryString = `${queryParamToken}${key}=${value}`; queryParamToken = "&"; @@ -120,6 +115,11 @@ async function openAddEditVaultItemPopout( addEditCipherUrl += formatQueryString("uri", url); } + const extensionUrl = chrome.runtime.getURL("popup/index.html"); + const existingPopupTabs = await BrowserApi.tabsQuery({ url: `${extensionUrl}*` }); + const existingPopup = existingPopupTabs.find((tab) => + tab.url?.includes(`singleActionPopout=${singleActionKey}`), + ); // Check if the an existing popup is already open if (existingPopup) { await chrome.runtime.sendMessage({ From 2c43bb32094a7d9e859a12ac7d523d80d742ef17 Mon Sep 17 00:00:00 2001 From: Jeffrey Holland Date: Tue, 18 Nov 2025 13:41:08 +0100 Subject: [PATCH 4/5] Clean out all stale data from pop up --- .../add-edit/add-edit-v2.component.spec.ts | 84 +++++++++++++++++++ .../add-edit/add-edit-v2.component.ts | 6 +- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts index 1bffcd9ad51b..f2c9d4708166 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts @@ -381,4 +381,88 @@ describe("AddEditV2Component", () => { expect(navigate).toHaveBeenCalledWith(["/tabs/vault"]); }); }); + + describe("reloadAddEditCipherData", () => { + beforeEach(fakeAsync(() => { + addEditCipherInfo$.next({ + cipher: { + name: "InitialName", + type: CipherType.Login, + login: { + password: "initialPassword", + username: "initialUsername", + uris: [{ uri: "https://initial.com" }], + }, + }, + } as AddEditCipherInfo); + queryParams$.next({}); + tick(); + + cipherServiceMock.setAddEditCipherInfo.mockClear(); + })); + + it("replaces all initialValues with new data, clearing stale fields", fakeAsync(() => { + const newCipherInfo = { + cipher: { + name: "UpdatedName", + type: CipherType.Login, + login: { + password: "updatedPassword", + uris: [{ uri: "https://updated.com" }], + }, + }, + } as AddEditCipherInfo; + + addEditCipherInfo$.next(newCipherInfo); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "UpdatedName", + password: "updatedPassword", + loginUri: "https://updated.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).toHaveBeenCalledWith(null, "UserId"); + })); + + it("does not reload data if config is not set", fakeAsync(() => { + component.config = null; + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("does not reload data if latestCipherInfo is null", fakeAsync(() => { + addEditCipherInfo$.next(null); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "InitialName", + password: "initialPassword", + username: "initialUsername", + loginUri: "https://initial.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("ignores messages with different commands", fakeAsync(() => { + const initialValues = component.config.initialValues; + + const messageListener = component["messageListener"]; + messageListener({ command: "someOtherCommand" }); + tick(); + + expect(component.config.initialValues).toBe(initialValues); + })); + }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index f389ad0a8ca7..3c08f4a0f7ae 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -226,6 +226,7 @@ export class AddEditV2Component implements OnInit, OnDestroy { /** * Reloads the cipher data when the popup is already open and new form data is submitted. + * This completely replaces the initialValues to clear any stale data from the previous submission. */ private async reloadCipherData() { if (!this.config) { @@ -241,10 +242,7 @@ export class AddEditV2Component implements OnInit, OnDestroy { if (latestCipherInfo != null) { this.config = { ...this.config, - initialValues: { - ...this.config.initialValues, - ...mapAddEditCipherInfoToInitialValues(latestCipherInfo), - }, + initialValues: mapAddEditCipherInfoToInitialValues(latestCipherInfo), }; // Be sure to clear the "cached" cipher info, so it doesn't get used again From b109bfa16f3c52057b46971bccd55d9a5065c80d Mon Sep 17 00:00:00 2001 From: Jeffrey Holland Date: Thu, 20 Nov 2025 16:16:43 +0100 Subject: [PATCH 5/5] Fix cached cipher issue --- .../vault/src/cipher-form/components/cipher-form.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index 5e75ea5bc24a..32bbb8118e70 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -311,7 +311,9 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } // Use the cached cipher when it matches the cipher being edited - if (this.updatedCipherView.id === cachedCipher.id) { + // Don't use cached cipher for new ciphers (when updatedCipherView.id is null/undefined) + // This prevents stale data from previous "new cipher" sessions from being loaded + if (this.updatedCipherView.id && this.updatedCipherView.id === cachedCipher.id) { this.updatedCipherView = cachedCipher; } }