From daa6e7ba41d205c06979ce6aa515841c1be4eab8 Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Tue, 24 Mar 2026 08:45:59 -0400 Subject: [PATCH] [PM-29524]Remove ts strict ignore in collect autofill content service (#19525) * initialize observer fields as nullable lifecycle properties * [PM-29524]Remove @ts-strict-ignore in collect-autofill-content.service * formatting * address feedback * update jsdoc * prettier * dead code path --- .../src/autofill/models/autofill-field.ts | 4 +- .../collect-autofill-content.service.spec.ts | 7 +- .../collect-autofill-content.service.ts | 192 +++++++++++------- 3 files changed, 127 insertions(+), 76 deletions(-) diff --git a/apps/browser/src/autofill/models/autofill-field.ts b/apps/browser/src/autofill/models/autofill-field.ts index 9d2cf3773d4..592f56d0531 100644 --- a/apps/browser/src/autofill/models/autofill-field.ts +++ b/apps/browser/src/autofill/models/autofill-field.ts @@ -96,9 +96,9 @@ export default class AutofillField { */ readonly?: boolean; /** - * The `opid` attribute value of the form that contains the field + * The `opid` attribute value of the form that contains the field, null if no form or opid is absent */ - form?: string; + form?: string | null; /** * The `x-autocompletetype`, `autocompletetype`, or `autocomplete` attribute for the field */ diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts index b47092ec27a..79889545bb5 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts @@ -3007,13 +3007,12 @@ describe("CollectAutofillContentService", () => { describe("destroy", () => { it("clears the updateAfterMutationIdleCallback", () => { jest.spyOn(window, "clearTimeout"); - collectAutofillContentService["updateAfterMutationIdleCallback"] = setTimeout(jest.fn, 100); + const callbackId = setTimeout(jest.fn, 100); + collectAutofillContentService["updateAfterMutationIdleCallback"] = callbackId; collectAutofillContentService.destroy(); - expect(clearTimeout).toHaveBeenCalledWith( - collectAutofillContentService["updateAfterMutationIdleCallback"], - ); + expect(clearTimeout).toHaveBeenCalledWith(callbackId); }); it("clears all pending overlay setup timeouts", () => { diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index e3d565a1ef1..436b0fed919 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { AUTOFILL_ATTRIBUTES } from "@bitwarden/common/autofill/constants"; import AutofillField from "../models/autofill-field"; @@ -48,11 +46,11 @@ export class CollectAutofillContentService implements CollectAutofillContentServ private autofillFieldElements: AutofillFieldElements = new Map(); private autofillFieldsByOpid: Map = new Map(); private currentLocationHref = ""; - private intersectionObserver: IntersectionObserver; + private intersectionObserver: IntersectionObserver | null = null; private elementInitializingIntersectionObserver: Set = new Set(); - private mutationObserver: MutationObserver; + private mutationObserver: MutationObserver | null = null; private mutationsQueue: MutationRecord[][] = []; - private updateAfterMutationIdleCallback: NodeJS.Timeout | number; + private updateAfterMutationIdleCallback: number | NodeJS.Timeout | null = null; private pendingOverlaySetup: Map = new Map(); private readonly overlaySetupDelayMs = 100; private shadowDomCheckTimeout: NodeJS.Timeout | number | null = null; @@ -115,11 +113,11 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.setupInitialTopLayerListeners(); } - if (!this.mutationObserver) { + if (this.mutationObserver === null) { this.setupMutationObserver(); } - if (!this.intersectionObserver) { + if (this.intersectionObserver === null) { this.setupIntersectionObserver(); } @@ -139,9 +137,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ const { formElements, formFieldElements } = this.queryAutofillFormAndFieldElements(); const autofillFormsData: Record = this.buildAutofillFormsData(formElements); - const autofillFieldsData: AutofillField[] = ( - await this.buildAutofillFieldsData(formFieldElements as FormFieldElement[]) - ).filter((field) => !!field); + const autofillFieldsData: AutofillField[] = await this.buildAutofillFieldsData( + formFieldElements as FormFieldElement[], + ); this.sortAutofillFieldElementsMap(); if (!autofillFieldsData.length) { @@ -274,7 +272,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ htmlClass: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.CLASS), htmlID: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.ID), htmlMethod: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.METHOD), - }); + } as AutofillForm); } return this.getFormattedAutofillFormsData(); @@ -284,14 +282,15 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * Returns the action attribute of the form element. If the action attribute * is a relative path, it will be converted to an absolute path. * @param {ElementWithOpId} element - * @returns {string} + * @returns {string | null} * @private */ - private getFormActionAttribute(element: ElementWithOpId): string { - return new URL( - this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ACTION), - globalThis.location.href, - ).href; + private getFormActionAttribute(element: ElementWithOpId): string | null { + const action = this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ACTION); + if (action === null) { + return null; + } + return new URL(action, globalThis.location.href).href; } /** @@ -326,9 +325,16 @@ export class CollectAutofillContentService implements CollectAutofillContentServ autofillFieldsLimit, formFieldElements, ); - const autofillFieldDataPromises = autofillFieldElements.map(this.buildAutofillFieldItem); + const autofillFieldDataPromises = autofillFieldElements.map( + (element: FormFieldElement, i: number) => + this.buildAutofillFieldItem(element as ElementWithOpId, i), + ); - return Promise.all(autofillFieldDataPromises); + const candidates = await Promise.all(autofillFieldDataPromises); + const autofillFields: AutofillField[] = candidates.filter( + (field): field is AutofillField => field !== null, + ); + return autofillFields; } /** @@ -350,7 +356,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ globalThis.document.documentElement, this.formFieldQueryString, (node: Node) => this.isNodeFormFieldElement(node), - this.mutationObserver, + this.mutationObserver ?? undefined, ); } @@ -370,7 +376,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ element, AUTOFILL_ATTRIBUTES.TYPE, )?.toLowerCase(); - if (unimportantFieldTypesSet.has(fieldType)) { + if (fieldType !== undefined && unimportantFieldTypesSet.has(fieldType)) { unimportantFormFields.push(element); continue; } @@ -413,7 +419,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ return existingAutofillField; } - const autofillFieldBase = { + const autofillFieldBase: AutofillField = { opid: element.opid, elementNumber: index, maxLength: this.getAutofillFieldMaxLength(element), @@ -429,7 +435,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ if (!autofillFieldBase.viewable) { this.elementInitializingIntersectionObserver.add(element); - this.intersectionObserver?.observe(element); + if (this.intersectionObserver !== null) { + this.intersectionObserver.observe(element); + } } if (elementIsSpanElement(element)) { @@ -452,7 +460,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } const fieldFormElement = (element as ElementWithOpId).form; - const autofillField = { + const autofillField: AutofillField = { ...autofillFieldBase, ...autofillFieldLabels, rel: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.REL), @@ -505,10 +513,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * Identifies the autocomplete attribute associated with an element and returns * the value of the attribute if it is not set to "off". * @param {ElementWithOpId} element - * @returns {string} + * @returns {string | null} * @private */ - private getAutoCompleteAttribute(element: ElementWithOpId): string { + private getAutoCompleteAttribute(element: ElementWithOpId): string | null { return ( this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.AUTOCOMPLETE) || this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.X_AUTOCOMPLETE_TYPE) || @@ -520,13 +528,13 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * Returns the attribute of an element as a lowercase value. * @param {ElementWithOpId} element * @param {string} attributeName - * @returns {string} + * @returns {string | undefined} * @private */ private getAttributeLowerCase( element: ElementWithOpId, attributeName: string, - ): string { + ): string | undefined { return this.getPropertyOrAttribute(element, attributeName)?.toLowerCase(); } @@ -555,26 +563,31 @@ export class CollectAutofillContentService implements CollectAutofillContentServ return this.createLabelElementsTag(labelElementsSet); } - const labelElements: NodeListOf | null = this.queryElementLabels(element); - for (let labelIndex = 0; labelIndex < labelElements?.length; labelIndex++) { - labelElementsSet.add(labelElements[labelIndex]); + const labelElements = this.queryElementLabels(element); + if (labelElements?.length) { + for (const label of labelElements) { + labelElementsSet.add(label); + } } let currentElement: HTMLElement | null = element; - while (currentElement && currentElement !== document.documentElement) { + while (currentElement !== null && currentElement !== document.documentElement) { if (elementIsLabelElement(currentElement)) { labelElementsSet.add(currentElement); } - - currentElement = currentElement.parentElement?.closest("label"); + currentElement = currentElement.parentElement?.closest("label") ?? null; } + const parentElement = element.parentElement; if ( !labelElementsSet.size && - elementIsDescriptionDetailsElement(element.parentElement) && - elementIsDescriptionTermElement(element.parentElement.previousElementSibling) + parentElement !== null && + elementIsDescriptionDetailsElement(parentElement) ) { - labelElementsSet.add(element.parentElement.previousElementSibling); + const prevSibling = parentElement.previousElementSibling; + if (prevSibling instanceof HTMLElement && elementIsDescriptionTermElement(prevSibling)) { + labelElementsSet.add(prevSibling); + } } return this.createLabelElementsTag(labelElementsSet); @@ -769,14 +782,20 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * @returns {string} * @private */ - private getTextContentFromElement(element: Node | HTMLElement): string { + private getTextContentFromElement(element: Node | HTMLElement): string | null { if (element.nodeType === Node.TEXT_NODE) { - return this.trimAndRemoveNonPrintableText(element.nodeValue); + const nodeValue = element.nodeValue; + if (nodeValue === null) { + return null; + } + return this.trimAndRemoveNonPrintableText(nodeValue); } - return this.trimAndRemoveNonPrintableText( - element.textContent || (element as HTMLElement).innerText, - ); + const textContentOrInnerText = element.textContent || (element as HTMLElement).innerText; + if (textContentOrInnerText === null) { + return null; + } + return this.trimAndRemoveNonPrintableText(textContentOrInnerText); } /** @@ -802,8 +821,8 @@ export class CollectAutofillContentService implements CollectAutofillContentServ */ private recursivelyGetTextFromPreviousSiblings(element: Node | HTMLElement): string[] { const textContentItems: string[] = []; - let currentElement = element; - while (currentElement && currentElement.previousSibling) { + let currentElement: Node | HTMLElement | null = element; + while (currentElement !== null && currentElement.previousSibling !== null) { // Ensure we are capturing text content from nodes and elements. currentElement = currentElement.previousSibling; @@ -821,34 +840,43 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } } - if (!currentElement || textContentItems.length) { + if (currentElement === null || textContentItems.length > 0) { return textContentItems; } // Prioritize capturing text content from elements rather than nodes. - currentElement = currentElement.parentElement || currentElement.parentNode; - if (!currentElement) { + const parent = + currentElement.parentElement !== null + ? currentElement.parentElement + : currentElement.parentNode; + if (parent === null) { return textContentItems; } + currentElement = parent; - let siblingElement = nodeIsElement(currentElement) + let siblingElement: ChildNode | Element | null = nodeIsElement(currentElement) ? currentElement.previousElementSibling : currentElement.previousSibling; while ( - siblingElement?.lastChild && - !this.isNewSectionElement(siblingElement) && + siblingElement !== null && + siblingElement.lastChild !== null && + !this.isNewSectionElement(siblingElement as Node) && !this.containsChildField(siblingElement) ) { siblingElement = siblingElement.lastChild; } - if (this.isNewSectionElement(siblingElement) || this.containsChildField(siblingElement)) { + if ( + siblingElement === null || + this.isNewSectionElement(siblingElement) || + this.containsChildField(siblingElement) + ) { return textContentItems; } - const textContent = this.getTextContentFromElement(siblingElement); - if (textContent) { - textContentItems.push(textContent); + const siblingTextContent = this.getTextContentFromElement(siblingElement); + if (siblingTextContent) { + textContentItems.push(siblingTextContent); return textContentItems; } @@ -951,7 +979,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ return false; }, - this.mutationObserver, + this.mutationObserver ?? undefined, ); if (formElements.length || formFieldElements.length) { @@ -1186,20 +1214,24 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } private setupTopLayerCandidateListener = (element: Element) => { - if (this.autofillOverlayContentService) { - const ownedTags = this.autofillOverlayContentService.getOwnedInlineMenuTagNames() || []; + const overlayService = this.autofillOverlayContentService; + if (overlayService !== undefined) { + const ownedTags = overlayService.getOwnedInlineMenuTagNames() || []; this.ownedExperienceTagNames = ownedTags; if (!ownedTags.includes(element.tagName)) { - element.addEventListener("toggle", (event: ToggleEvent) => { - if (event.newState === "open") { + const toggleListener = (event: Event) => { + if ((event as ToggleEvent).newState === "open") { // Add a slight delay (but faster than a user's reaction), to ensure the layer // positioning happens after any triggered toggle has completed. - setTimeout(this.autofillOverlayContentService.refreshMenuLayerPosition, 100); + setTimeout(() => { + overlayService.refreshMenuLayerPosition(); + }, 100); } - }); + }; + element.addEventListener("toggle", toggleListener); - this.autofillOverlayContentService.refreshMenuLayerPosition(); + overlayService.refreshMenuLayerPosition(); } } }; @@ -1275,7 +1307,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ `form, ${this.formFieldQueryString}`, (walkerNode: Node) => nodeIsFormElement(walkerNode) || this.isNodeFormFieldElement(walkerNode), - this.mutationObserver, + this.mutationObserver ?? undefined, true, ); @@ -1377,8 +1409,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * @private */ private updateAutofillElementsAfterMutation() { - if (this.updateAfterMutationIdleCallback) { + if (this.updateAfterMutationIdleCallback !== null) { cancelIdleCallbackPolyfill(this.updateAfterMutationIdleCallback); + this.updateAfterMutationIdleCallback = null; } const now = Date.now(); @@ -1422,6 +1455,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } const attributeName = mutation.attributeName?.toLowerCase(); + if (attributeName === undefined) { + return; + } const autofillForm = this._autofillFormElements.get( targetElement as ElementWithOpId, ); @@ -1466,7 +1502,12 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.updateAutofillDataAttribute({ element, attributeName, dataTarget, dataTargetKey }); }; const updateActions: Record = { - action: () => (dataTarget.htmlAction = this.getFormActionAttribute(element)), + action: () => { + const actionUrl = this.getFormActionAttribute(element); + if (actionUrl !== null) { + dataTarget.htmlAction = actionUrl; + } + }, name: () => updateAttribute("htmlName"), id: () => updateAttribute("htmlID"), class: () => updateAttribute("htmlClass"), @@ -1606,7 +1647,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement); if (!cachedAutofillFieldElement) { - this.intersectionObserver.unobserve(entry.target); + if (this.intersectionObserver !== null) { + this.intersectionObserver.unobserve(entry.target); + } continue; } @@ -1618,7 +1661,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ cachedAutofillFieldElement.viewable = true; this.setupOverlayOnField(formFieldElement, cachedAutofillFieldElement); - this.intersectionObserver?.unobserve(entry.target); + if (this.intersectionObserver !== null) { + this.intersectionObserver.unobserve(entry.target); + } } }; @@ -1728,15 +1773,22 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * timeouts and disconnects the mutation observer. */ destroy() { - if (this.updateAfterMutationIdleCallback) { + if (this.updateAfterMutationIdleCallback !== null) { cancelIdleCallbackPolyfill(this.updateAfterMutationIdleCallback); + this.updateAfterMutationIdleCallback = null; } if (this.shadowDomCheckTimeout) { clearTimeout(this.shadowDomCheckTimeout); } this.pendingOverlaySetup.forEach((timeout) => globalThis.clearTimeout(timeout)); this.pendingOverlaySetup.clear(); - this.mutationObserver?.disconnect(); - this.intersectionObserver?.disconnect(); + if (this.mutationObserver !== null) { + this.mutationObserver.disconnect(); + this.mutationObserver = null; + } + if (this.intersectionObserver !== null) { + this.intersectionObserver.disconnect(); + this.intersectionObserver = null; + } } }