1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 22:03:36 +00:00

[PM-5682] Chrome's extension API for retrieving closed ShadowRoots from elements causes performance issues when passed a Node with nested children (#7542)

* [PM-5682] Chrome's extension API for retrieving closed ShadowRoots from elements causes performance issues when passed a Node with nested children

* [PM-5682] Updating jest test to reflect logic changes

* [PM-5682] Removing instances of instanceof check to facilitate better performance

* [PM-5682] Fixing jest test to ensure code coverage

* [PM-5682] Fixing merge conflict
This commit is contained in:
Cesar Gonzalez
2024-01-23 15:53:24 -06:00
committed by GitHub
parent e23bcb50e8
commit 219bad0e42
2 changed files with 85 additions and 34 deletions

View File

@@ -1967,6 +1967,14 @@ describe("CollectAutofillContentService", () => {
}); });
describe("getShadowRoot", () => { describe("getShadowRoot", () => {
beforeEach(() => {
// eslint-disable-next-line
// @ts-ignore
globalThis.chrome.dom = {
openOrClosedShadowRoot: jest.fn(),
};
});
it("returns null if the passed node is not an HTMLElement instance", () => { it("returns null if the passed node is not an HTMLElement instance", () => {
const textNode = document.createTextNode("Hello, world!"); const textNode = document.createTextNode("Hello, world!");
const shadowRoot = collectAutofillContentService["getShadowRoot"](textNode); const shadowRoot = collectAutofillContentService["getShadowRoot"](textNode);
@@ -1974,12 +1982,27 @@ describe("CollectAutofillContentService", () => {
expect(shadowRoot).toEqual(null); expect(shadowRoot).toEqual(null);
}); });
it("returns a value provided by Chrome's openOrClosedShadowRoot API", () => { it("returns null if the passed node contains children elements", () => {
const element = document.createElement("div");
element.innerHTML = "<p>Hello, world!</p>";
const shadowRoot = collectAutofillContentService["getShadowRoot"](element);
// eslint-disable-next-line // eslint-disable-next-line
// @ts-ignore // @ts-ignore
globalThis.chrome.dom = { expect(chrome.dom.openOrClosedShadowRoot).not.toBeCalled();
openOrClosedShadowRoot: jest.fn(), expect(shadowRoot).toEqual(null);
}; });
it("returns an open shadow root if the passed node has a shadowDOM element", () => {
const element = document.createElement("div");
element.attachShadow({ mode: "open" });
const shadowRoot = collectAutofillContentService["getShadowRoot"](element);
expect(shadowRoot).toBeInstanceOf(ShadowRoot);
});
it("returns a value provided by Chrome's openOrClosedShadowRoot API", () => {
const element = document.createElement("div"); const element = document.createElement("div");
collectAutofillContentService["getShadowRoot"](element); collectAutofillContentService["getShadowRoot"](element);

View File

@@ -28,6 +28,14 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
private mutationObserver: MutationObserver; private mutationObserver: MutationObserver;
private updateAutofillElementsAfterMutationTimeout: NodeJS.Timeout; private updateAutofillElementsAfterMutationTimeout: NodeJS.Timeout;
private readonly updateAfterMutationTimeoutDelay = 1000; private readonly updateAfterMutationTimeoutDelay = 1000;
private readonly ignoredInputTypes = new Set([
"hidden",
"submit",
"reset",
"button",
"image",
"file",
]);
constructor( constructor(
domElementVisibilityService: DomElementVisibilityService, domElementVisibilityService: DomElementVisibilityService,
@@ -339,7 +347,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
tagName: this.getAttributeLowerCase(element, "tagName"), tagName: this.getAttributeLowerCase(element, "tagName"),
}; };
if (element instanceof HTMLSpanElement) { if (element.tagName.toLowerCase() === "span") {
this.cacheAutofillFieldElement(index, element, autofillFieldBase); this.cacheAutofillFieldElement(index, element, autofillFieldBase);
this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField( this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField(
element, element,
@@ -352,7 +360,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
const elementType = this.getAttributeLowerCase(element, "type"); const elementType = this.getAttributeLowerCase(element, "type");
if (elementType !== "hidden") { if (elementType !== "hidden") {
autofillFieldLabels = { autofillFieldLabels = {
"label-tag": this.createAutofillFieldLabelTag(element), "label-tag": this.createAutofillFieldLabelTag(element as FillableFormFieldElement),
"label-data": this.getPropertyOrAttribute(element, "data-label"), "label-data": this.getPropertyOrAttribute(element, "data-label"),
"label-aria": this.getPropertyOrAttribute(element, "aria-label"), "label-aria": this.getPropertyOrAttribute(element, "aria-label"),
"label-top": this.createAutofillFieldTopLabel(element), "label-top": this.createAutofillFieldTopLabel(element),
@@ -362,6 +370,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
}; };
} }
const fieldFormElement = (element as ElementWithOpId<FillableFormFieldElement>).form;
const autofillField = { const autofillField = {
...autofillFieldBase, ...autofillFieldBase,
...autofillFieldLabels, ...autofillFieldLabels,
@@ -373,8 +382,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
disabled: this.getAttributeBoolean(element, "disabled"), disabled: this.getAttributeBoolean(element, "disabled"),
readonly: this.getAttributeBoolean(element, "readonly"), readonly: this.getAttributeBoolean(element, "readonly"),
selectInfo: selectInfo:
element instanceof HTMLSelectElement ? this.getSelectElementOptions(element) : null, element.tagName.toLowerCase() === "select"
form: element.form ? this.getPropertyOrAttribute(element.form, "opid") : null, ? this.getSelectElementOptions(element as HTMLSelectElement)
: null,
form: fieldFormElement ? this.getPropertyOrAttribute(fieldFormElement, "opid") : null,
"aria-hidden": this.getAttributeBoolean(element, "aria-hidden", true), "aria-hidden": this.getAttributeBoolean(element, "aria-hidden", true),
"aria-disabled": this.getAttributeBoolean(element, "aria-disabled", true), "aria-disabled": this.getAttributeBoolean(element, "aria-disabled", true),
"aria-haspopup": this.getAttributeBoolean(element, "aria-haspopup", true), "aria-haspopup": this.getAttributeBoolean(element, "aria-haspopup", true),
@@ -487,7 +498,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
let currentElement: HTMLElement | null = element; let currentElement: HTMLElement | null = element;
while (currentElement && currentElement !== document.documentElement) { while (currentElement && currentElement !== document.documentElement) {
if (currentElement instanceof HTMLLabelElement) { if (currentElement.tagName.toLowerCase() === "label") {
labelElementsSet.add(currentElement); labelElementsSet.add(currentElement);
} }
@@ -562,10 +573,13 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
* @private * @private
*/ */
private getAutofillFieldMaxLength(element: FormFieldElement): number | null { private getAutofillFieldMaxLength(element: FormFieldElement): number | null {
const elementHasMaxLengthProperty = const elementTagName = element.tagName.toLowerCase();
element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement; const elementHasMaxLengthProperty = elementTagName === "input" || elementTagName === "textarea";
const elementMaxLength = const elementMaxLength =
elementHasMaxLengthProperty && element.maxLength > -1 ? element.maxLength : 999; elementHasMaxLengthProperty &&
(element as HTMLInputElement | HTMLTextAreaElement).maxLength > -1
? (element as HTMLInputElement | HTMLTextAreaElement).maxLength
: 999;
return elementHasMaxLengthProperty ? Math.min(elementMaxLength, 999) : null; return elementHasMaxLengthProperty ? Math.min(elementMaxLength, 999) : null;
} }
@@ -775,13 +789,13 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
* @private * @private
*/ */
private getElementValue(element: FormFieldElement): string { private getElementValue(element: FormFieldElement): string {
if (element instanceof HTMLSpanElement) { if (element.tagName.toLowerCase() === "span") {
const spanTextContent = element.textContent || element.innerText; const spanTextContent = element.textContent || element.innerText;
return spanTextContent || ""; return spanTextContent || "";
} }
const elementValue = element.value || ""; const elementValue = (element as FillableFormFieldElement).value || "";
const elementType = String(element.type).toLowerCase(); const elementType = String((element as FillableFormFieldElement).type).toLowerCase();
if ("checked" in element && elementType === "checkbox") { if ("checked" in element && elementType === "checkbox") {
return element.checked ? "✓" : ""; return element.checked ? "✓" : "";
} }
@@ -832,7 +846,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
const formElements: Node[] = []; const formElements: Node[] = [];
const formFieldElements: Node[] = []; const formFieldElements: Node[] = [];
this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => { this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => {
if (node instanceof HTMLFormElement) { if ((node as HTMLFormElement).tagName?.toLowerCase() === "form") {
formElements.push(node); formElements.push(node);
return true; return true;
} }
@@ -855,40 +869,49 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
* @private * @private
*/ */
private isNodeFormFieldElement(node: Node): boolean { private isNodeFormFieldElement(node: Node): boolean {
if (!(node instanceof HTMLElement)) {
return false;
}
const nodeTagName = node.tagName.toLowerCase();
const nodeIsSpanElementWithAutofillAttribute = const nodeIsSpanElementWithAutofillAttribute =
node instanceof HTMLSpanElement && node.hasAttribute("data-bwautofill"); nodeTagName === "span" && node.hasAttribute("data-bwautofill");
if (nodeIsSpanElementWithAutofillAttribute) {
return true;
}
const ignoredInputTypes = new Set(["hidden", "submit", "reset", "button", "image", "file"]); const nodeHasBwIgnoreAttribute = node.hasAttribute("data-bwignore");
const nodeIsValidInputElement = const nodeIsValidInputElement =
node instanceof HTMLInputElement && !ignoredInputTypes.has(node.type); nodeTagName === "input" && !this.ignoredInputTypes.has((node as HTMLInputElement).type);
if (nodeIsValidInputElement && !nodeHasBwIgnoreAttribute) {
return true;
}
const nodeIsTextAreaOrSelectElement = return ["textarea", "select"].includes(nodeTagName) && !nodeHasBwIgnoreAttribute;
node instanceof HTMLTextAreaElement || node instanceof HTMLSelectElement;
const nodeIsNonIgnoredFillableControlElement =
(nodeIsTextAreaOrSelectElement || nodeIsValidInputElement) &&
!node.hasAttribute("data-bwignore");
return nodeIsSpanElementWithAutofillAttribute || nodeIsNonIgnoredFillableControlElement;
} }
/** /**
* Attempts to get the ShadowRoot of the passed node. If support for the * Attempts to get the ShadowRoot of the passed node. If support for the
* extension based openOrClosedShadowRoot API is available, it will be used. * extension based openOrClosedShadowRoot API is available, it will be used.
* Will return null if the node is not an HTMLElement or if the node has
* child nodes.
*
* @param {Node} node * @param {Node} node
* @returns {ShadowRoot | null}
* @private
*/ */
private getShadowRoot(node: Node): ShadowRoot | null { private getShadowRoot(node: Node): ShadowRoot | null {
if (!(node instanceof HTMLElement)) { if (!(node instanceof HTMLElement) || node.childNodes.length !== 0) {
return null; return null;
} }
if (node.shadowRoot) {
return node.shadowRoot;
}
if ((chrome as any).dom?.openOrClosedShadowRoot) { if ((chrome as any).dom?.openOrClosedShadowRoot) {
return (chrome as any).dom.openOrClosedShadowRoot(node); return (chrome as any).dom.openOrClosedShadowRoot(node);
} }
return (node as any).openOrClosedShadowRoot || node.shadowRoot; return (node as any).openOrClosedShadowRoot;
} }
/** /**
@@ -1031,7 +1054,9 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
const autofillElementNodes = this.queryAllTreeWalkerNodes( const autofillElementNodes = this.queryAllTreeWalkerNodes(
node, node,
(node: Node) => node instanceof HTMLFormElement || this.isNodeFormFieldElement(node), (walkerNode: Node) =>
(walkerNode as HTMLElement).tagName?.toLowerCase() === "form" ||
this.isNodeFormFieldElement(walkerNode),
) as HTMLElement[]; ) as HTMLElement[];
if (autofillElementNodes.length) { if (autofillElementNodes.length) {
@@ -1084,8 +1109,11 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte
private deleteCachedAutofillElement( private deleteCachedAutofillElement(
element: ElementWithOpId<HTMLFormElement> | ElementWithOpId<FormFieldElement>, element: ElementWithOpId<HTMLFormElement> | ElementWithOpId<FormFieldElement>,
) { ) {
if (element instanceof HTMLFormElement && this.autofillFormElements.has(element)) { if (
this.autofillFormElements.delete(element); element.tagName.toLowerCase() === "form" &&
this.autofillFormElements.has(element as ElementWithOpId<HTMLFormElement>)
) {
this.autofillFormElements.delete(element as ElementWithOpId<HTMLFormElement>);
return; return;
} }