Merge branch 'autofill/pm-6546-blurring-of-autofilled-elements-causes-problems-in-blur-event-listeners' into autofill/pm-5189-fix-issues-present-with-inline-menu-rendering-in-iframes

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-button-iframe.spec.ts

@@ -1,8 +1,15 @@
 import AutofillOverlayButtonIframe from "./autofill-overlay-button-iframe";
-import AutofillOverlayIframeElement from "./autofill-overlay-iframe-element";
 
 describe("AutofillOverlayButtonIframe", () => {
-  window.customElements.define("autofill-overlay-button-iframe", AutofillOverlayButtonIframe);
+  window.customElements.define(
+    "autofill-overlay-button-iframe",
+    class extends HTMLElement {
+      constructor() {
+        super();
+        new AutofillOverlayButtonIframe(this);
+      }
+    },
+  );
 
   afterAll(() => {
     jest.clearAllMocks();
@@ -13,7 +20,7 @@ describe("AutofillOverlayButtonIframe", () => {
 
     const iframe = document.querySelector("autofill-overlay-button-iframe");
 
-    expect(iframe).toBeInstanceOf(AutofillOverlayButtonIframe);
+    expect(iframe).toBeInstanceOf(HTMLElement);
-    expect(iframe).toBeInstanceOf(AutofillOverlayIframeElement);
+    expect(iframe.shadowRoot).toBeDefined();
   });
 });

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-button-iframe.ts

@@ -3,8 +3,9 @@ import { AutofillOverlayPort } from "../../utils/autofill-overlay.enum";
 import AutofillOverlayIframeElement from "./autofill-overlay-iframe-element";
 
 class AutofillOverlayButtonIframe extends AutofillOverlayIframeElement {
-  constructor() {
+  constructor(element: HTMLElement) {
     super(
+      element,
       "overlay/button.html",
       AutofillOverlayPort.Button,
       {

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe-element.spec.ts

@@ -4,7 +4,21 @@ import AutofillOverlayIframeService from "./autofill-overlay-iframe.service";
 jest.mock("./autofill-overlay-iframe.service");
 
 describe("AutofillOverlayIframeElement", () => {
-  window.customElements.define("autofill-overlay-iframe", AutofillOverlayIframeElement);
+  window.customElements.define(
+    "autofill-overlay-iframe",
+    class extends HTMLElement {
+      constructor() {
+        super();
+        new AutofillOverlayIframeElement(
+          this,
+          "overlay/button.html",
+          "overlay/button",
+          { background: "transparent", border: "none" },
+          "bitwardenOverlayButton",
+        );
+      }
+    },
+  );
 
   afterAll(() => {
     jest.clearAllMocks();

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe-element.ts

@@ -1,16 +1,15 @@
 import AutofillOverlayIframeService from "./autofill-overlay-iframe.service";
 
-class AutofillOverlayIframeElement extends HTMLElement {
+class AutofillOverlayIframeElement {
   constructor(
+    element: HTMLElement,
     iframePath: string,
     portName: string,
     initStyles: Partial<CSSStyleDeclaration>,
     iframeTitle: string,
     ariaAlert?: string,
   ) {
-    super();
+    const shadow: ShadowRoot = element.attachShadow({ mode: "closed" });
-
-    const shadow: ShadowRoot = this.attachShadow({ mode: "closed" });
     const autofillOverlayIframeService = new AutofillOverlayIframeService(
       iframePath,
       portName,

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-list-iframe.spec.ts

@@ -1,8 +1,15 @@
-import AutofillOverlayIframeElement from "./autofill-overlay-iframe-element";
 import AutofillOverlayListIframe from "./autofill-overlay-list-iframe";
 
 describe("AutofillOverlayListIframe", () => {
-  window.customElements.define("autofill-overlay-list-iframe", AutofillOverlayListIframe);
+  window.customElements.define(
+    "autofill-overlay-list-iframe",
+    class extends HTMLElement {
+      constructor() {
+        super();
+        new AutofillOverlayListIframe(this);
+      }
+    },
+  );
 
   afterAll(() => {
     jest.clearAllMocks();
@@ -13,7 +20,7 @@ describe("AutofillOverlayListIframe", () => {
 
     const iframe = document.querySelector("autofill-overlay-list-iframe");
 
-    expect(iframe).toBeInstanceOf(AutofillOverlayListIframe);
+    expect(iframe).toBeInstanceOf(HTMLElement);
-    expect(iframe).toBeInstanceOf(AutofillOverlayIframeElement);
+    expect(iframe.shadowRoot).toBeDefined();
   });
 });

apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-list-iframe.ts

@@ -3,8 +3,9 @@ import { AutofillOverlayPort } from "../../utils/autofill-overlay.enum";
 import AutofillOverlayIframeElement from "./autofill-overlay-iframe-element";
 
 class AutofillOverlayListIframe extends AutofillOverlayIframeElement {
-  constructor() {
+  constructor(element: HTMLElement) {
     super(
+      element,
       "overlay/list.html",
       AutofillOverlayPort.List,
       {

apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts

@@ -877,6 +877,44 @@ describe("AutofillOverlayContentService", () => {
         sender: "autofillOverlayContentService",
       });
     });
+
+    it("builds the overlay elements as custom web components if the user's browser is not Firefox", () => {
+      let namesIndex = 0;
+      const customNames = ["op-autofill-overlay-button", "op-autofill-overlay-list"];
+
+      jest
+        .spyOn(autofillOverlayContentService as any, "generateRandomCustomElementName")
+        .mockImplementation(() => {
+          if (namesIndex > 1) {
+            return "";
+          }
+          const customName = customNames[namesIndex];
+          namesIndex++;
+
+          return customName;
+        });
+      autofillOverlayContentService["isFirefoxBrowser"] = false;
+
+      autofillOverlayContentService.openAutofillOverlay();
+
+      expect(autofillOverlayContentService["overlayButtonElement"]).toBeInstanceOf(HTMLElement);
+      expect(autofillOverlayContentService["overlayButtonElement"].tagName).toEqual(
+        customNames[0].toUpperCase(),
+      );
+      expect(autofillOverlayContentService["overlayListElement"]).toBeInstanceOf(HTMLElement);
+      expect(autofillOverlayContentService["overlayListElement"].tagName).toEqual(
+        customNames[1].toUpperCase(),
+      );
+    });
+
+    it("builds the overlay elements as `div` elements if the user's browser is Firefox", () => {
+      autofillOverlayContentService["isFirefoxBrowser"] = true;
+
+      autofillOverlayContentService.openAutofillOverlay();
+
+      expect(autofillOverlayContentService["overlayButtonElement"]).toBeInstanceOf(HTMLDivElement);
+      expect(autofillOverlayContentService["overlayListElement"]).toBeInstanceOf(HTMLDivElement);
+    });
   });
 
   describe("focusMostRecentOverlayField", () => {

apps/browser/src/autofill/services/autofill-overlay-content.service.ts

@@ -30,6 +30,10 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
   isOverlayCiphersPopulated = false;
   pageDetailsUpdateRequired = false;
   autofillOverlayVisibility: number;
+  private isFirefoxBrowser =
+    globalThis.navigator.userAgent.indexOf(" Firefox/") !== -1 ||
+    globalThis.navigator.userAgent.indexOf(" Gecko/") !== -1;
+  private readonly generateRandomCustomElementName = generateRandomCustomElementName;
   private readonly findTabs = tabbable;
   private readonly sendExtensionMessage = sendExtensionMessage;
   private formFieldElements: Set<ElementWithOpId<FormFieldElement>> = new Set([]);
@@ -593,6 +597,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
   private updateOverlayButtonPosition() {
     if (!this.overlayButtonElement) {
       this.createAutofillOverlayButton();
+      this.updateCustomElementDefaultStyles(this.overlayButtonElement);
     }
 
     if (!this.isOverlayButtonVisible) {
@@ -613,6 +618,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
   private updateOverlayListPosition() {
     if (!this.overlayListElement) {
       this.createAutofillOverlayList();
+      this.updateCustomElementDefaultStyles(this.overlayListElement);
     }
 
     if (!this.isOverlayListVisible) {
@@ -765,11 +771,24 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
       return;
     }
 
-    const customElementName = generateRandomCustomElementName();
+    if (this.isFirefoxBrowser) {
-    globalThis.customElements?.define(customElementName, AutofillOverlayButtonIframe);
+      this.overlayButtonElement = globalThis.document.createElement("div");
-    this.overlayButtonElement = globalThis.document.createElement(customElementName);
+      new AutofillOverlayButtonIframe(this.overlayButtonElement);
 
-    this.updateCustomElementDefaultStyles(this.overlayButtonElement);
+      return;
+    }
+
+    const customElementName = this.generateRandomCustomElementName();
+    globalThis.customElements?.define(
+      customElementName,
+      class extends HTMLElement {
+        constructor() {
+          super();
+          new AutofillOverlayButtonIframe(this);
+        }
+      },
+    );
+    this.overlayButtonElement = globalThis.document.createElement(customElementName);
   }
 
   /**
@@ -781,11 +800,24 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
       return;
     }
 
-    const customElementName = generateRandomCustomElementName();
+    if (this.isFirefoxBrowser) {
-    globalThis.customElements?.define(customElementName, AutofillOverlayListIframe);
+      this.overlayListElement = globalThis.document.createElement("div");
-    this.overlayListElement = globalThis.document.createElement(customElementName);
+      new AutofillOverlayListIframe(this.overlayListElement);
 
-    this.updateCustomElementDefaultStyles(this.overlayListElement);
+      return;
+    }
+
+    const customElementName = this.generateRandomCustomElementName();
+    globalThis.customElements?.define(
+      customElementName,
+      class extends HTMLElement {
+        constructor() {
+          super();
+          new AutofillOverlayListIframe(this);
+        }
+      },
+    );
+    this.overlayListElement = globalThis.document.createElement(customElementName);
   }
 
   /**

apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts

@@ -553,17 +553,30 @@ describe("InsertAutofillContentService", () => {
         insertAutofillContentService as any,
         "simulateUserMouseClickAndFocusEventInteractions",
       );
+      jest.spyOn(targetInput, "blur");
 
       insertAutofillContentService["handleFocusOnFieldByOpidAction"]("__0");
 
       expect(
         insertAutofillContentService["collectAutofillContentService"].getAutofillFieldElementByOpid,
       ).toBeCalledWith("__0");
+      expect(targetInput.blur).not.toHaveBeenCalled();
       expect(
         insertAutofillContentService["simulateUserMouseClickAndFocusEventInteractions"],
       ).toHaveBeenCalledWith(targetInput, true);
       expect(elementEventCount).toEqual(expectedElementEventCount);
     });
+
+    it("blurs the element if it is currently the active element before simulating click and focus events", () => {
+      const targetInput = document.querySelector('input[type="text"]') as FormElementWithAttribute;
+      targetInput.opid = "__0";
+      targetInput.focus();
+      jest.spyOn(targetInput, "blur");
+
+      insertAutofillContentService["handleFocusOnFieldByOpidAction"]("__0");
+
+      expect(targetInput.blur).toHaveBeenCalled();
+    });
   });
 
   describe("insertValueIntoField", () => {
@@ -710,7 +723,7 @@ describe("InsertAutofillContentService", () => {
   });
 
   describe("triggerPostInsertEventsOnElement", () => {
-    it("triggers simulated event interactions and blurs the element after", () => {
+    it("triggers simulated event interactions", () => {
       const elementValue = "test";
       document.body.innerHTML = `<input type="text" id="username" value="${elementValue}"/>`;
       const element = document.getElementById("username") as FillableFormFieldElement;
@@ -726,7 +739,6 @@ describe("InsertAutofillContentService", () => {
       expect(insertAutofillContentService["simulateInputElementChangedEvent"]).toHaveBeenCalledWith(
         element,
       );
-      expect(element.blur).toHaveBeenCalled();
       expect(element.value).toBe(elementValue);
     });
   });

apps/browser/src/autofill/services/insert-autofill-content.service.ts

@@ -185,11 +185,18 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf
 
   /**
    * Handles finding an element by opid and triggering click and focus events on the element.
-   * @param {string} opid
+   * To ensure that we trigger a blur event correctly on a filled field, we first check if the
-   * @private
+   * element is already focused. If it is, we blur the element before focusing on it again.
+   *
+   * @param {string} opid - The opid of the element to focus on.
    */
   private handleFocusOnFieldByOpidAction(opid: string) {
     const element = this.collectAutofillContentService.getAutofillFieldElementByOpid(opid);
+
+    if (document.activeElement === element) {
+      element.blur();
+    }
+
     this.simulateUserMouseClickAndFocusEventInteractions(element, true);
   }
 
@@ -282,7 +289,6 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf
     }
 
     this.simulateInputElementChangedEvent(element);
-    element.blur();
   }
 
   /**
@@ -379,10 +385,6 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf
       element.dispatchEvent(new Event(simulatedInputEvents[index], { bubbles: true }));
     }
   }
-
-  private nodeIsElement(node: Node): node is HTMLElement {
-    return node.nodeType === Node.ELEMENT_NODE;
-  }
 }
 
 export default InsertAutofillContentService;

libs/importer/src/components/import.component.html

@@ -37,7 +37,7 @@
         <bit-option [value]="null" label="-- {{ 'selectImportFolder' | i18n }} --" />
         <bit-option
           *ngFor="let f of folders$ | async"
-          [value]="f.id"
+          [value]="f"
           [label]="f.name"
           icon="bwi-folder"
         />
@@ -46,7 +46,7 @@
         <bit-option [value]="null" label="-- {{ 'selectImportCollection' | i18n }} --" />
         <bit-option
           *ngFor="let c of collections$ | async"
-          [value]="c.id"
+          [value]="c"
           [label]="c.name"
           icon="bwi-collection"
         />

libs/importer/src/services/import.service.abstraction.ts

@@ -1,3 +1,6 @@
+import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
+import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
+
 import { Importer } from "../importers/importer";
 import { ImportOption, ImportType } from "../models/import-options";
 import { ImportResult } from "../models/import-result";
@@ -10,7 +13,7 @@ export abstract class ImportServiceAbstraction {
     importer: Importer,
     fileContents: string,
     organizationId?: string,
-    selectedImportTarget?: string,
+    selectedImportTarget?: FolderView | CollectionView,
     canAccessImportExport?: boolean,
   ) => Promise<ImportResult>;
   getImporter: (

libs/importer/src/services/import.service.spec.ts

@@ -86,7 +86,7 @@ describe("ImportService", () => {
     });
 
     it("empty importTarget does nothing", async () => {
-      await importService["setImportTarget"](importResult, null, "");
+      await importService["setImportTarget"](importResult, null, null);
       expect(importResult.folders.length).toBe(0);
     });
 
@@ -99,9 +99,9 @@ describe("ImportService", () => {
         Promise.resolve([mockImportTargetFolder]),
       );
 
-      await importService["setImportTarget"](importResult, null, "myImportTarget");
+      await importService["setImportTarget"](importResult, null, mockImportTargetFolder);
       expect(importResult.folders.length).toBe(1);
-      expect(importResult.folders[0].name).toBe("myImportTarget");
+      expect(importResult.folders[0]).toBe(mockImportTargetFolder);
     });
 
     const mockFolder1 = new FolderView();
@@ -119,16 +119,18 @@ describe("ImportService", () => {
         mockFolder2,
       ]);
 
-      const myImportTarget = "myImportTarget";
-
       importResult.folders.push(mockFolder1);
       importResult.folders.push(mockFolder2);
 
-      await importService["setImportTarget"](importResult, null, myImportTarget);
+      await importService["setImportTarget"](importResult, null, mockImportTargetFolder);
       expect(importResult.folders.length).toBe(3);
-      expect(importResult.folders[0].name).toBe(myImportTarget);
+      expect(importResult.folders[0]).toBe(mockImportTargetFolder);
-      expect(importResult.folders[1].name).toBe(`${myImportTarget}/${mockFolder1.name}`);
+      expect(importResult.folders[1].name).toBe(
-      expect(importResult.folders[2].name).toBe(`${myImportTarget}/${mockFolder2.name}`);
+        `${mockImportTargetFolder.name}/${mockFolder1.name}`,
+      );
+      expect(importResult.folders[2].name).toBe(
+        `${mockImportTargetFolder.name}/${mockFolder2.name}`,
+      );
     });
 
     const mockImportTargetCollection = new CollectionView();
@@ -152,9 +154,13 @@ describe("ImportService", () => {
         mockCollection1,
       ]);
 
-      await importService["setImportTarget"](importResult, organizationId, "myImportTarget");
+      await importService["setImportTarget"](
+        importResult,
+        organizationId,
+        mockImportTargetCollection,
+      );
       expect(importResult.collections.length).toBe(1);
-      expect(importResult.collections[0].name).toBe("myImportTarget");
+      expect(importResult.collections[0]).toBe(mockImportTargetCollection);
     });
 
     it("passing importTarget sets it as new root for all existing collections", async () => {
@@ -164,16 +170,42 @@ describe("ImportService", () => {
         mockCollection2,
       ]);
 
-      const myImportTarget = "myImportTarget";
-
       importResult.collections.push(mockCollection1);
       importResult.collections.push(mockCollection2);
 
-      await importService["setImportTarget"](importResult, organizationId, myImportTarget);
+      await importService["setImportTarget"](
+        importResult,
+        organizationId,
+        mockImportTargetCollection,
+      );
       expect(importResult.collections.length).toBe(3);
-      expect(importResult.collections[0].name).toBe(myImportTarget);
+      expect(importResult.collections[0]).toBe(mockImportTargetCollection);
-      expect(importResult.collections[1].name).toBe(`${myImportTarget}/${mockCollection1.name}`);
+      expect(importResult.collections[1].name).toBe(
-      expect(importResult.collections[2].name).toBe(`${myImportTarget}/${mockCollection2.name}`);
+        `${mockImportTargetCollection.name}/${mockCollection1.name}`,
+      );
+      expect(importResult.collections[2].name).toBe(
+        `${mockImportTargetCollection.name}/${mockCollection2.name}`,
+      );
+    });
+
+    it("passing importTarget as null on setImportTarget with organizationId throws error", async () => {
+      const setImportTargetMethod = importService["setImportTarget"](
+        null,
+        organizationId,
+        new Object() as FolderView,
+      );
+
+      await expect(setImportTargetMethod).rejects.toThrow("Error assigning target collection");
+    });
+
+    it("passing importTarget as null on setImportTarget throws error", async () => {
+      const setImportTargetMethod = importService["setImportTarget"](
+        null,
+        "",
+        new Object() as CollectionView,
+      );
+
+      await expect(setImportTargetMethod).rejects.toThrow("Error assigning target folder");
     });
   });
 });

libs/importer/src/services/import.service.ts

@@ -110,7 +110,7 @@ export class ImportService implements ImportServiceAbstraction {
     importer: Importer,
     fileContents: string,
     organizationId: string = null,
-    selectedImportTarget: string = null,
+    selectedImportTarget: FolderView | CollectionView = null,
     canAccessImportExport: boolean,
   ): Promise<ImportResult> {
     let importResult: ImportResult;
@@ -147,11 +147,7 @@ export class ImportService implements ImportServiceAbstraction {
       }
     }
 
-    if (
+    if (organizationId && !selectedImportTarget && !canAccessImportExport) {
-      organizationId &&
-      Utils.isNullOrWhitespace(selectedImportTarget) &&
-      !canAccessImportExport
-    ) {
       const hasUnassignedCollections =
         importResult.collectionRelationships.length < importResult.ciphers.length;
       if (hasUnassignedCollections) {
@@ -428,29 +424,30 @@ export class ImportService implements ImportServiceAbstraction {
   private async setImportTarget(
     importResult: ImportResult,
     organizationId: string,
-    importTarget: string,
+    importTarget: FolderView | CollectionView,
   ) {
-    if (Utils.isNullOrWhitespace(importTarget)) {
+    if (!importTarget) {
       return;
     }
 
     if (organizationId) {
-      const collectionViews: CollectionView[] = await this.collectionService.getAllDecrypted();
+      if (!(importTarget instanceof CollectionView)) {
-      const targetCollection = collectionViews.find((c) => c.id === importTarget);
+        throw new Error("Error assigning target collection");
+      }
 
       const noCollectionRelationShips: [number, number][] = [];
       importResult.ciphers.forEach((c, index) => {
         if (!Array.isArray(c.collectionIds) || c.collectionIds.length == 0) {
-          c.collectionIds = [targetCollection.id];
+          c.collectionIds = [importTarget.id];
           noCollectionRelationShips.push([index, 0]);
         }
       });
 
       const collections: CollectionView[] = [...importResult.collections];
-      importResult.collections = [targetCollection];
+      importResult.collections = [importTarget as CollectionView];
       collections.map((x) => {
         const f = new CollectionView();
-        f.name = `${targetCollection.name}/${x.name}`;
+        f.name = `${importTarget.name}/${x.name}`;
         importResult.collections.push(f);
       });
 
@@ -463,21 +460,22 @@ export class ImportService implements ImportServiceAbstraction {
       return;
     }
 
-    const folderViews = await this.folderService.getAllDecryptedFromState();
+    if (!(importTarget instanceof FolderView)) {
-    const targetFolder = folderViews.find((f) => f.id === importTarget);
+      throw new Error("Error assigning target folder");
+    }
 
     const noFolderRelationShips: [number, number][] = [];
     importResult.ciphers.forEach((c, index) => {
       if (Utils.isNullOrEmpty(c.folderId)) {
-        c.folderId = targetFolder.id;
+        c.folderId = importTarget.id;
         noFolderRelationShips.push([index, 0]);
       }
     });
 
     const folders: FolderView[] = [...importResult.folders];
-    importResult.folders = [targetFolder];
+    importResult.folders = [importTarget as FolderView];
     folders.map((x) => {
-      const newFolderName = `${targetFolder.name}/${x.name}`;
+      const newFolderName = `${importTarget.name}/${x.name}`;
       const f = new FolderView();
       f.name = newFolderName;
       importResult.folders.push(f);