[PM-10138] Inline menu not properly validating as the top-most element within mutation observer (#10292)

* [PM-10138] Inline menu not properly validating as the top-most element within mutation observer * [PM-10138] Identity inline menu appearing for invalid field types * [PM-10138] Identity inline menu appearing for invalid field types * [PM-10138] Fixing an issue present with references to card and identity ciphers * [PM-10138] Fixing issues with initialization of the inline menu * [PM-10138] Fixing issues with initialization of the inline menu * [PM-10138] Removing inclusion of file protocol when injecting scripts
2024-07-31 12:51:41 -05:00 · 2024-07-31 12:51:41 -05:00 · 85c8ff04a1
parent ea362a9ed9
commit 85c8ff04a1
10 changed files with 106 additions and 28 deletions
--- a/apps/browser/src/autofill/background/abstractions/overlay.background.ts
+++ b/apps/browser/src/autofill/background/abstractions/overlay.background.ts
@ -190,7 +190,6 @@ export type OverlayBackgroundExtensionMessageHandlers = {
  }: BackgroundOnMessageHandlerParams) => void;
  collectPageDetailsResponse: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
  unlockCompleted: ({ message }: BackgroundMessageParam) => void;
-  doFullSync: () => void;
  addedCipher: () => void;
  addEditCipherSubmitted: () => void;
  editedCipher: () => void;
--- a/apps/browser/src/autofill/background/overlay.background.spec.ts
+++ b/apps/browser/src/autofill/background/overlay.background.spec.ts
@ -2014,7 +2014,6 @@ describe("OverlayBackground", () => {

    describe("extension messages that trigger an update of the inline menu ciphers", () => {
      const extensionMessages = [
-        "doFullSync",
        "addedCipher",
        "addEditCipherSubmitted",
        "editedCipher",
--- a/apps/browser/src/autofill/background/overlay.background.ts
+++ b/apps/browser/src/autofill/background/overlay.background.ts
@ -120,7 +120,6 @@ export class OverlayBackground implements OverlayBackgroundInterface {
      this.triggerDestroyInlineMenuListeners(sender.tab, message.subFrameData.frameId),
    collectPageDetailsResponse: ({ message, sender }) => this.storePageDetails(message, sender),
    unlockCompleted: ({ message }) => this.unlockCompleted(message),
-    doFullSync: () => this.updateOverlayCiphers(),
    addedCipher: () => this.updateOverlayCiphers(),
    addEditCipherSubmitted: () => this.updateOverlayCiphers(),
    editedCipher: () => this.updateOverlayCiphers(),
@ -273,7 +272,9 @@ export class OverlayBackground implements OverlayBackgroundInterface {
      await this.cipherService.getAllDecryptedForUrl(currentTab?.url || "")
    ).sort((a, b) => this.cipherService.sortCiphersByLastUsedThenName(a, b));

-    return cipherViews.concat(...this.cardAndIdentityCiphers);
+    return this.cardAndIdentityCiphers
+      ? cipherViews.concat(...this.cardAndIdentityCiphers)
+      : cipherViews;
  }

  /**
--- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts
+++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts
@ -399,6 +399,11 @@ describe("AutofillInlineMenuContentService", () => {
      });

      it("sets the z-index of to a lower value", async () => {
+        autofillInlineMenuContentService["handlePersistentLastChildOverrideTimeout"] = setTimeout(
+          jest.fn(),
+          1000,
+        );
+
        await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"]();
        await waitForIdleCallback();

@ -411,8 +416,9 @@ describe("AutofillInlineMenuContentService", () => {
        });
        globalThis.document.elementFromPoint = jest.fn(() => persistentLastChild);

-        await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"]();
-        await waitForIdleCallback();
+        await autofillInlineMenuContentService["verifyInlineMenuIsNotObscured"](
+          persistentLastChild,
+        );

        expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", {
          overlayElement: AutofillOverlayElement.Button,
@ -425,8 +431,9 @@ describe("AutofillInlineMenuContentService", () => {
        });
        globalThis.document.elementFromPoint = jest.fn(() => persistentLastChild);

-        await autofillInlineMenuContentService["handleBodyElementMutationObserverUpdate"]();
-        await waitForIdleCallback();
+        await autofillInlineMenuContentService["verifyInlineMenuIsNotObscured"](
+          persistentLastChild,
+        );

        expect(sendExtensionMessageSpy).toHaveBeenCalledWith("autofillOverlayElementClosed", {
          overlayElement: AutofillOverlayElement.List,
--- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts
+++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts
@ -33,6 +33,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
  private bodyElementMutationObserver: MutationObserver;
  private mutationObserverIterations = 0;
  private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout;
+  private handlePersistentLastChildOverrideTimeout: number | NodeJS.Timeout;
  private lastElementOverrides: WeakMap<Element, number> = new WeakMap();
  private readonly customElementDefaultStyles: Partial<CSSStyleDeclaration> = {
    all: "initial",
@ -405,7 +406,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
    }

    if (this.lastElementOverrides.get(lastChild) >= 3) {
-      await this.handlePersistentLastChildOverride(lastChild);
+      this.handlePersistentLastChildOverride(lastChild);

      return;
    }
@ -430,6 +431,26 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
    globalThis.document.body.insertBefore(lastChild, this.buttonElement);
  };

+  /**
+   * Handles the behavior of a persistent child element that is forcing itself to
+   * the bottom of the body element. This method will ensure that the inline menu
+   * elements are not obscured by the persistent child element.
+   *
+   * @param lastChild - The last child of the body element.
+   */
+  private handlePersistentLastChildOverride(lastChild: Element) {
+    const lastChildZIndex = parseInt((lastChild as HTMLElement).style.zIndex);
+    if (lastChildZIndex >= 2147483647) {
+      (lastChild as HTMLElement).style.zIndex = "2147483646";
+    }
+
+    this.clearPersistentLastChildOverrideTimeout();
+    this.handlePersistentLastChildOverrideTimeout = globalThis.setTimeout(
+      () => this.verifyInlineMenuIsNotObscured(lastChild),
+      500,
+    );
+  }
+
  /**
   * Verifies if the last child of the body element is overlaying the inline menu elements.
   * This is triggered when the last child of the body is being forced by some script to
@ -437,12 +458,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
   *
   * @param lastChild - The last child of the body element.
   */
-  private async handlePersistentLastChildOverride(lastChild: Element) {
-    const lastChildZIndex = parseInt((lastChild as HTMLElement).style.zIndex);
-    if (lastChildZIndex >= 2147483647) {
-      (lastChild as HTMLElement).style.zIndex = "2147483646";
-    }
-
+  private verifyInlineMenuIsNotObscured = async (lastChild: Element) => {
    const inlineMenuPosition: InlineMenuPosition = await this.sendExtensionMessage(
      "getAutofillInlineMenuPosition",
    );
@ -456,7 +472,7 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
    if (!!list && this.elementAtCenterOfInlineMenuPosition(list) === lastChild) {
      this.closeInlineMenu();
    }
-  }
+  };

  /**
   * Returns the element present at the center of the inline menu position.
@ -470,6 +486,16 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
    );
  }

+  /**
+   * Clears the timeout that is used to verify that the last child of the body element
+   * is not overlaying the inline menu elements.
+   */
+  private clearPersistentLastChildOverrideTimeout() {
+    if (this.handlePersistentLastChildOverrideTimeout) {
+      globalThis.clearTimeout(this.handlePersistentLastChildOverrideTimeout);
+    }
+  }
+
  /**
   * Identifies if the mutation observer is triggering excessive iterations.
   * Will trigger a blur of the most recently focused field and remove the
@ -503,5 +529,6 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte
   */
  destroy() {
    this.closeInlineMenu();
+    this.clearPersistentLastChildOverrideTimeout();
  }
 }
--- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts
+++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts
@ -377,6 +377,21 @@ describe("AutofillInlineMenuIframeService", () => {
            autofillInlineMenuIframeService["ariaAlertElement"],
          );
        });
+
+        it("resets the fade in timeout if it is set", () => {
+          autofillInlineMenuIframeService["fadeInTimeout"] = setTimeout(jest.fn, 100);
+          const styles = { top: "100px", left: "100px" };
+          jest.spyOn(autofillInlineMenuIframeService as any, "handleFadeInInlineMenuIframe");
+
+          sendPortMessage(portSpy, {
+            command: "updateAutofillInlineMenuPosition",
+            styles,
+          });
+
+          expect(
+            autofillInlineMenuIframeService["handleFadeInInlineMenuIframe"],
+          ).toHaveBeenCalled();
+        });
      });

      it("updates the visibility of the iframe", () => {
--- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts
+++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts
@ -260,10 +260,13 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe
      return;
    }

+    const styles = this.fadeInTimeout ? Object.assign(position, { opacity: "0" }) : position;
+    this.updateElementStyles(this.iframe, styles);
+
    if (this.fadeInTimeout) {
      this.handleFadeInInlineMenuIframe();
    }
-    this.updateElementStyles(this.iframe, position);
+
    this.announceAriaAlert();
  }

@ -320,10 +323,10 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe
   */
  private handleFadeInInlineMenuIframe() {
    this.clearFadeInTimeout();
-    this.fadeInTimeout = globalThis.setTimeout(
-      () => this.updateElementStyles(this.iframe, { display: "block", opacity: "1" }),
-      10,
-    );
+    this.fadeInTimeout = globalThis.setTimeout(() => {
+      this.updateElementStyles(this.iframe, { display: "block", opacity: "1" });
+      this.clearFadeInTimeout();
+    }, 10);
  }

  /**
--- a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts
+++ b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts
@ -16,7 +16,7 @@ export class InlineMenuFieldQualificationService
  implements InlineMenuFieldQualificationServiceInterface
 {
  private searchFieldNamesSet = new Set(AutoFillConstants.SearchFieldNames);
-  private excludedAutofillLoginTypesSet = new Set(AutoFillConstants.ExcludedAutofillLoginTypes);
+  private excludedAutofillFieldTypesSet = new Set(AutoFillConstants.ExcludedAutofillLoginTypes);
  private usernameFieldTypes = new Set(["text", "email", "number", "tel"]);
  private usernameAutocompleteValue = "username";
  private emailAutocompleteValue = "email";
@ -244,6 +244,10 @@ export class InlineMenuFieldQualificationService
   * @param pageDetails - The details of the page that the field is on.
   */
  isFieldForAccountCreationForm(field: AutofillField, pageDetails: AutofillPageDetails): boolean {
+    if (this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet)) {
+      return false;
+    }
+
    if (!this.isUsernameField(field) && !this.isPasswordField(field)) {
      return false;
    }
@ -286,6 +290,10 @@ export class InlineMenuFieldQualificationService
   * @param _pageDetails - Currently unused, will likely be required in the future
   */
  isFieldForIdentityForm(field: AutofillField, _pageDetails: AutofillPageDetails): boolean {
+    if (this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet)) {
+      return false;
+    }
+
    if (this.fieldContainsAutocompleteValues(field, this.identityAutocompleteValues)) {
      return true;
    }
@ -833,7 +841,7 @@ export class InlineMenuFieldQualificationService
  isUsernameField = (field: AutofillField): boolean => {
    if (
      !this.usernameFieldTypes.has(field.type) ||
-      this.isExcludedFieldType(field, this.excludedAutofillLoginTypesSet)
+      this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet)
    ) {
      return false;
    }
@ -852,7 +860,7 @@ export class InlineMenuFieldQualificationService
    }

    return (
-      !this.isExcludedFieldType(field, this.excludedAutofillLoginTypesSet) &&
+      !this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet) &&
      this.keywordsFoundInFieldData(field, AutoFillConstants.EmailFieldNames)
    );
  };
@ -898,7 +906,7 @@ export class InlineMenuFieldQualificationService
    const isInputPasswordType = field.type === "password";
    if (
      (!isInputPasswordType &&
-        this.isExcludedFieldType(field, this.excludedAutofillLoginTypesSet)) ||
+        this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet)) ||
      this.fieldHasDisqualifyingAttributeValue(field)
    ) {
      return false;
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@ -1245,6 +1245,13 @@ export default class MainBackground {
    }
  }

+  async updateOverlayCiphers() {
+    // overlayBackground null in popup only contexts
+    if (this.overlayBackground) {
+      await this.overlayBackground.updateOverlayCiphers();
+    }
+  }
+
  /**
   * Switch accounts to indicated userId -- null is no active user
   */
@ -1273,7 +1280,7 @@ export default class MainBackground {
      if (userId == null) {
        await this.refreshBadge();
        await this.refreshMenu();
-        await this.overlayBackground?.updateOverlayCiphers(); // null in popup only contexts
+        await this.updateOverlayCiphers();
        this.messagingService.send("goHome");
        return;
      }
@ -1296,7 +1303,7 @@ export default class MainBackground {
        this.messagingService.send("unlocked", { userId: userId });
        await this.refreshBadge();
        await this.refreshMenu();
-        await this.overlayBackground?.updateOverlayCiphers(); // null in popup only contexts
+        await this.updateOverlayCiphers();
        await this.syncService.fullSync(false);
      }
    } finally {
@ -1480,7 +1487,17 @@ export default class MainBackground {
   * Temporary solution to handle initialization of the overlay background behind a feature flag.
   * Will be reverted to instantiation within the constructor once the feature flag is removed.
   */
-  private async initOverlayAndTabsBackground() {
+  async initOverlayAndTabsBackground() {
+    if (
+      this.popupOnlyContext ||
+      this.overlayBackground ||
+      this.tabsBackground ||
+      (await firstValueFrom(this.authService.activeAccountStatus$)) ===
+        AuthenticationStatus.LoggedOut
+    ) {
+      return;
+    }
+
    const inlineMenuPositioningImprovementsEnabled = await this.configService.getFeatureFlag(
      FeatureFlag.InlineMenuPositioningImprovements,
    );
--- a/apps/browser/src/background/runtime.background.ts
+++ b/apps/browser/src/background/runtime.background.ts
@ -200,6 +200,7 @@ export default class RuntimeBackground {
        let item: LockedVaultPendingNotificationsData;

        if (msg.command === "loggedIn") {
+          await this.main.initOverlayAndTabsBackground();
          await this.sendBwInstalledMessageToVault();
          await this.autofillService.reloadAutofillScripts();
        }
@ -246,6 +247,7 @@ export default class RuntimeBackground {
            await this.main.refreshMenu();
          }, 2000);
          await this.configService.ensureConfigFetched();
+          await this.main.updateOverlayCiphers();
        }
        break;
      case "openPopup":