1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-19 15:57:42 +01:00

[PM-7128] Fix cached form fields not showing the inline menu after their visibility is changed using CSS (#8509)

This commit is contained in:
Cesar Gonzalez 2024-03-29 14:08:46 -05:00 committed by GitHub
parent 670f33daa8
commit 77cfa8a5ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 161 additions and 44 deletions

View File

@ -24,6 +24,7 @@ describe("AutofillInit", () => {
}, },
}); });
autofillInit = new AutofillInit(autofillOverlayContentService); autofillInit = new AutofillInit(autofillOverlayContentService);
window.IntersectionObserver = jest.fn(() => mock<IntersectionObserver>());
}); });
afterEach(() => { afterEach(() => {

View File

@ -173,12 +173,10 @@ describe("AutofillOverlayContentService", () => {
autofillFieldData = mock<AutofillField>(); autofillFieldData = mock<AutofillField>();
}); });
it("ignores fields that are readonly", () => { it("ignores fields that are readonly", async () => {
autofillFieldData.readonly = true; autofillFieldData.readonly = true;
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -186,12 +184,10 @@ describe("AutofillOverlayContentService", () => {
expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled(); expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled();
}); });
it("ignores fields that contain a disabled attribute", () => { it("ignores fields that contain a disabled attribute", async () => {
autofillFieldData.disabled = true; autofillFieldData.disabled = true;
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -199,12 +195,10 @@ describe("AutofillOverlayContentService", () => {
expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled(); expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled();
}); });
it("ignores fields that are not viewable", () => { it("ignores fields that are not viewable", async () => {
autofillFieldData.viewable = false; autofillFieldData.viewable = false;
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -213,12 +207,10 @@ describe("AutofillOverlayContentService", () => {
}); });
it("ignores fields that are part of the ExcludedOverlayTypes", () => { it("ignores fields that are part of the ExcludedOverlayTypes", () => {
AutoFillConstants.ExcludedOverlayTypes.forEach((excludedType) => { AutoFillConstants.ExcludedOverlayTypes.forEach(async (excludedType) => {
autofillFieldData.type = excludedType; autofillFieldData.type = excludedType;
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -227,12 +219,10 @@ describe("AutofillOverlayContentService", () => {
}); });
}); });
it("ignores fields that contain the keyword `search`", () => { it("ignores fields that contain the keyword `search`", async () => {
autofillFieldData.placeholder = "search"; autofillFieldData.placeholder = "search";
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -240,12 +230,10 @@ describe("AutofillOverlayContentService", () => {
expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled(); expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled();
}); });
it("ignores fields that contain the keyword `captcha` ", () => { it("ignores fields that contain the keyword `captcha` ", async () => {
autofillFieldData.placeholder = "captcha"; autofillFieldData.placeholder = "captcha";
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -253,12 +241,10 @@ describe("AutofillOverlayContentService", () => {
expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled(); expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled();
}); });
it("ignores fields that do not appear as a login field", () => { it("ignores fields that do not appear as a login field", async () => {
autofillFieldData.placeholder = "not-a-login-field"; autofillFieldData.placeholder = "not-a-login-field";
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement, autofillFieldElement,
autofillFieldData, autofillFieldData,
); );
@ -267,6 +253,17 @@ describe("AutofillOverlayContentService", () => {
}); });
}); });
it("skips setup on fields that have been previously set up", async () => {
autofillOverlayContentService["formFieldElements"].add(autofillFieldElement);
await autofillOverlayContentService.setupAutofillOverlayListenerOnField(
autofillFieldElement,
autofillFieldData,
);
expect(autofillFieldElement.addEventListener).not.toHaveBeenCalled();
});
describe("identifies the overlay visibility setting", () => { describe("identifies the overlay visibility setting", () => {
it("defaults the overlay visibility setting to `OnFieldFocus` if a value is not set", async () => { it("defaults the overlay visibility setting to `OnFieldFocus` if a value is not set", async () => {
sendExtensionMessageSpy.mockResolvedValueOnce(undefined); sendExtensionMessageSpy.mockResolvedValueOnce(undefined);

View File

@ -86,7 +86,7 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte
formFieldElement: ElementWithOpId<FormFieldElement>, formFieldElement: ElementWithOpId<FormFieldElement>,
autofillFieldData: AutofillField, autofillFieldData: AutofillField,
) { ) {
if (this.isIgnoredField(autofillFieldData)) { if (this.isIgnoredField(autofillFieldData) || this.formFieldElements.has(formFieldElement)) {
return; return;
} }

View File

@ -27,6 +27,7 @@ describe("CollectAutofillContentService", () => {
const domElementVisibilityService = new DomElementVisibilityService(); const domElementVisibilityService = new DomElementVisibilityService();
const autofillOverlayContentService = new AutofillOverlayContentService(); const autofillOverlayContentService = new AutofillOverlayContentService();
let collectAutofillContentService: CollectAutofillContentService; let collectAutofillContentService: CollectAutofillContentService;
const mockIntersectionObserver = mock<IntersectionObserver>();
beforeEach(() => { beforeEach(() => {
document.body.innerHTML = mockLoginForm; document.body.innerHTML = mockLoginForm;
@ -34,6 +35,7 @@ describe("CollectAutofillContentService", () => {
domElementVisibilityService, domElementVisibilityService,
autofillOverlayContentService, autofillOverlayContentService,
); );
window.IntersectionObserver = jest.fn(() => mockIntersectionObserver);
}); });
afterEach(() => { afterEach(() => {
@ -2527,10 +2529,10 @@ describe("CollectAutofillContentService", () => {
}); });
updatedAttributes.forEach((attribute) => { updatedAttributes.forEach((attribute) => {
it(`will update the ${attribute} value for the field element`, async () => { it(`will update the ${attribute} value for the field element`, () => {
jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set"); jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set");
await collectAutofillContentService["updateAutofillFieldElementData"]( collectAutofillContentService["updateAutofillFieldElementData"](
attribute, attribute,
fieldElement, fieldElement,
autofillField, autofillField,
@ -2543,10 +2545,10 @@ describe("CollectAutofillContentService", () => {
}); });
}); });
it("will not update an attribute value if it is not present in the updateActions object", async () => { it("will not update an attribute value if it is not present in the updateActions object", () => {
jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set"); jest.spyOn(collectAutofillContentService["autofillFieldElements"], "set");
await collectAutofillContentService["updateAutofillFieldElementData"]( collectAutofillContentService["updateAutofillFieldElementData"](
"random-attribute", "random-attribute",
fieldElement, fieldElement,
autofillField, autofillField,
@ -2555,4 +2557,67 @@ describe("CollectAutofillContentService", () => {
expect(collectAutofillContentService["autofillFieldElements"].set).not.toBeCalled(); expect(collectAutofillContentService["autofillFieldElements"].set).not.toBeCalled();
}); });
}); });
describe("handleFormElementIntersection", () => {
let isFormFieldViewableSpy: jest.SpyInstance;
let setupAutofillOverlayListenerOnFieldSpy: jest.SpyInstance;
beforeEach(() => {
isFormFieldViewableSpy = jest.spyOn(
collectAutofillContentService["domElementVisibilityService"],
"isFormFieldViewable",
);
setupAutofillOverlayListenerOnFieldSpy = jest.spyOn(
collectAutofillContentService["autofillOverlayContentService"],
"setupAutofillOverlayListenerOnField",
);
});
it("skips the initial intersection event for an observed element", async () => {
const formFieldElement = document.createElement("input") as ElementWithOpId<FormFieldElement>;
collectAutofillContentService["elementInitializingIntersectionObserver"].add(
formFieldElement,
);
const entries = [
{ target: formFieldElement, isIntersecting: true },
] as unknown as IntersectionObserverEntry[];
await collectAutofillContentService["handleFormElementIntersection"](entries);
expect(isFormFieldViewableSpy).not.toHaveBeenCalled();
expect(setupAutofillOverlayListenerOnFieldSpy).not.toHaveBeenCalled();
});
it("skips setting up the overlay listeners on a field that is not viewable", async () => {
const formFieldElement = document.createElement("input") as ElementWithOpId<FormFieldElement>;
const entries = [
{ target: formFieldElement, isIntersecting: true },
] as unknown as IntersectionObserverEntry[];
isFormFieldViewableSpy.mockReturnValueOnce(false);
await collectAutofillContentService["handleFormElementIntersection"](entries);
expect(isFormFieldViewableSpy).toHaveBeenCalledWith(formFieldElement);
expect(setupAutofillOverlayListenerOnFieldSpy).not.toHaveBeenCalled();
});
it("sets up the overlay listeners on a viewable field", async () => {
const formFieldElement = document.createElement("input") as ElementWithOpId<FormFieldElement>;
const autofillField = mock<AutofillField>();
const entries = [
{ target: formFieldElement, isIntersecting: true },
] as unknown as IntersectionObserverEntry[];
isFormFieldViewableSpy.mockReturnValueOnce(true);
collectAutofillContentService["autofillFieldElements"].set(formFieldElement, autofillField);
collectAutofillContentService["intersectionObserver"] = mockIntersectionObserver;
await collectAutofillContentService["handleFormElementIntersection"](entries);
expect(isFormFieldViewableSpy).toHaveBeenCalledWith(formFieldElement);
expect(setupAutofillOverlayListenerOnFieldSpy).toHaveBeenCalledWith(
formFieldElement,
autofillField,
);
});
});
}); });

View File

@ -38,6 +38,8 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
private autofillFormElements: AutofillFormElements = new Map(); private autofillFormElements: AutofillFormElements = new Map();
private autofillFieldElements: AutofillFieldElements = new Map(); private autofillFieldElements: AutofillFieldElements = new Map();
private currentLocationHref = ""; private currentLocationHref = "";
private intersectionObserver: IntersectionObserver;
private elementInitializingIntersectionObserver: Set<Element> = new Set();
private mutationObserver: MutationObserver; private mutationObserver: MutationObserver;
private updateAutofillElementsAfterMutationTimeout: number | NodeJS.Timeout; private updateAutofillElementsAfterMutationTimeout: number | NodeJS.Timeout;
private readonly updateAfterMutationTimeoutDelay = 1000; private readonly updateAfterMutationTimeoutDelay = 1000;
@ -70,6 +72,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
this.setupMutationObserver(); this.setupMutationObserver();
} }
if (!this.intersectionObserver) {
this.setupIntersectionObserver();
}
if (!this.domRecentlyMutated && this.noFieldsFound) { if (!this.domRecentlyMutated && this.noFieldsFound) {
return this.getFormattedPageDetails({}, []); return this.getFormattedPageDetails({}, []);
} }
@ -360,11 +366,14 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
tagName: this.getAttributeLowerCase(element, "tagName"), tagName: this.getAttributeLowerCase(element, "tagName"),
}; };
if (!autofillFieldBase.viewable) {
this.elementInitializingIntersectionObserver.add(element);
this.intersectionObserver.observe(element);
}
if (elementIsSpanElement(element)) { if (elementIsSpanElement(element)) {
this.cacheAutofillFieldElement(index, element, autofillFieldBase); this.cacheAutofillFieldElement(index, element, autofillFieldBase);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(
element, element,
autofillFieldBase, autofillFieldBase,
); );
@ -407,9 +416,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
}; };
this.cacheAutofillFieldElement(index, element, autofillField); this.cacheAutofillFieldElement(index, element, autofillField);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(
// eslint-disable-next-line @typescript-eslint/no-floating-promises element,
this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(element, autofillField); autofillField,
);
return autofillField; return autofillField;
}; };
@ -1189,8 +1199,6 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
return; return;
} }
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.updateAutofillFieldElementData( this.updateAutofillFieldElementData(
attributeName, attributeName,
targetElement as ElementWithOpId<FormFieldElement>, targetElement as ElementWithOpId<FormFieldElement>,
@ -1232,13 +1240,12 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
/** /**
* Updates the autofill field element data based on the passed attribute name. * Updates the autofill field element data based on the passed attribute name.
*
* @param {string} attributeName * @param {string} attributeName
* @param {ElementWithOpId<FormFieldElement>} element * @param {ElementWithOpId<FormFieldElement>} element
* @param {AutofillField} dataTarget * @param {AutofillField} dataTarget
* @returns {Promise<void>}
* @private
*/ */
private async updateAutofillFieldElementData( private updateAutofillFieldElementData(
attributeName: string, attributeName: string,
element: ElementWithOpId<FormFieldElement>, element: ElementWithOpId<FormFieldElement>,
dataTarget: AutofillField, dataTarget: AutofillField,
@ -1304,6 +1311,52 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
return attributeValue; return attributeValue;
} }
/**
* Sets up an IntersectionObserver to observe found form
* field elements that are not viewable in the viewport.
*/
private setupIntersectionObserver() {
this.intersectionObserver = new IntersectionObserver(this.handleFormElementIntersection, {
root: null,
rootMargin: "0px",
threshold: 1.0,
});
}
/**
* Handles observed form field elements that are not viewable in the viewport.
* Will re-evaluate the visibility of the element and set up the autofill
* overlay listeners on the field if it is viewable.
*
* @param entries - The entries observed by the IntersectionObserver
*/
private handleFormElementIntersection = async (entries: IntersectionObserverEntry[]) => {
for (let entryIndex = 0; entryIndex < entries.length; entryIndex++) {
const entry = entries[entryIndex];
const formFieldElement = entry.target as ElementWithOpId<FormFieldElement>;
if (this.elementInitializingIntersectionObserver.has(formFieldElement)) {
this.elementInitializingIntersectionObserver.delete(formFieldElement);
continue;
}
const isViewable =
await this.domElementVisibilityService.isFormFieldViewable(formFieldElement);
if (!isViewable) {
continue;
}
const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement);
cachedAutofillFieldElement.viewable = true;
void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(
formFieldElement,
cachedAutofillFieldElement,
);
this.intersectionObserver.unobserve(entry.target);
}
};
/** /**
* Destroys the CollectAutofillContentService. Clears all * Destroys the CollectAutofillContentService. Clears all
* timeouts and disconnects the mutation observer. * timeouts and disconnects the mutation observer.
@ -1313,6 +1366,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
clearTimeout(this.updateAutofillElementsAfterMutationTimeout); clearTimeout(this.updateAutofillElementsAfterMutationTimeout);
} }
this.mutationObserver?.disconnect(); this.mutationObserver?.disconnect();
this.intersectionObserver?.disconnect();
} }
} }