From 09cfee2888fdbd2cd3f860ea816e8bb4311ea001 Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Fri, 19 Sep 2025 12:28:55 +0200 Subject: [PATCH] Add test case for nested form elements, refactor logic (#1252) --- .../src/utils/formDetector/FormDetector.ts | 82 +++++++++++++++++-- .../__tests__/FormDetector.generic.test.ts | 71 ++++++++++++++++ .../nested-custom-elements-confirm.html | 25 ++++++ .../test-forms/nested-custom-elements.html | 26 ++++++ 4 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements-confirm.html create mode 100644 apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements.html diff --git a/apps/browser-extension/src/utils/formDetector/FormDetector.ts b/apps/browser-extension/src/utils/formDetector/FormDetector.ts index cf3328bd1..3367a4ccc 100644 --- a/apps/browser-extension/src/utils/formDetector/FormDetector.ts +++ b/apps/browser-extension/src/utils/formDetector/FormDetector.ts @@ -479,14 +479,18 @@ export class FormDetector { excludeElements: HTMLInputElement[] = [] ): HTMLInputElement | null { const all = this.findAllInputFields(form, patterns, types, excludeElements); + + // Filter out parent-child duplicates + const filtered = this.filterOutNestedDuplicates(all); + // if email type explicitly requested, prefer actual if (types.includes('email')) { - const emailMatch = all.find(i => (i.type || '').toLowerCase() === 'email'); + const emailMatch = filtered.find(i => (i.type || '').toLowerCase() === 'email'); if (emailMatch) { return emailMatch; } } - return all.length > 0 ? all[0] : null; + return filtered.length > 0 ? filtered[0] : null; } /** @@ -496,25 +500,32 @@ export class FormDetector { primary: HTMLInputElement | null, confirm: HTMLInputElement | null } { - // Find primary email field - const primaryEmail = this.findInputField( + // Find all email fields first + const emailFields = this.findAllInputFields( form, CombinedFieldPatterns.email, ['text', 'email'] ); + // Filter out parent-child relationships + const filteredEmailFields = this.filterOutNestedDuplicates(emailFields); + const primaryEmail = filteredEmailFields[0] ?? null; + /* * Find confirmation email field if primary exists * and ensure it's not the same as the primary email field. */ - const confirmEmail = primaryEmail - ? this.findInputField( + const confirmEmailFields = primaryEmail + ? this.findAllInputFields( form, CombinedFieldPatterns.emailConfirm, ['text', 'email'], [primaryEmail] ) - : null; + : []; + + const filteredConfirmFields = this.filterOutNestedDuplicates(confirmEmailFields); + const confirmEmail = filteredConfirmFields[0] ?? null; return { primary: primaryEmail, @@ -667,6 +678,56 @@ export class FormDetector { }; } + /** + * Filter out nested duplicates where a parent element and its child are both detected. + * This happens with custom elements that contain actual input elements. + * We prefer the innermost actual input element over the parent custom element. + */ + private filterOutNestedDuplicates(fields: HTMLInputElement[]): HTMLInputElement[] { + if (fields.length <= 1) { + return fields; + } + + const filtered: HTMLInputElement[] = []; + + for (const field of fields) { + let shouldInclude = true; + + // Check if this field is a parent of any other field in the list + for (const otherField of fields) { + if (field !== otherField) { + // Check if field contains otherField (field is parent) + if (field.contains(otherField)) { + shouldInclude = false; + break; + } + + // Check if field's shadow DOM contains otherField + const fieldWithShadow = field as HTMLElement & { shadowRoot?: ShadowRoot }; + if (fieldWithShadow.shadowRoot && fieldWithShadow.shadowRoot.contains(otherField)) { + shouldInclude = false; + break; + } + } + } + + if (shouldInclude) { + // Also check if this field is not already represented by its actual input + const actualInput = this.getActualInputElement(field); + if (actualInput !== field) { + // If the actual input is also in the list, skip the parent + if (fields.includes(actualInput as HTMLInputElement)) { + continue; + } + } + + filtered.push(field); + } + } + + return filtered; + } + /** * Find the password field in a form. */ @@ -676,9 +737,12 @@ export class FormDetector { } { const passwordFields = this.findAllInputFields(form, CombinedFieldPatterns.password, ['password']); + // Filter out parent-child relationships to avoid detecting the same field twice + const filteredFields = this.filterOutNestedDuplicates(passwordFields); + return { - primary: passwordFields[0] ?? null, - confirm: passwordFields[1] ?? null + primary: filteredFields[0] ?? null, + confirm: filteredFields[1] ?? null }; } diff --git a/apps/browser-extension/src/utils/formDetector/__tests__/FormDetector.generic.test.ts b/apps/browser-extension/src/utils/formDetector/__tests__/FormDetector.generic.test.ts index a31ae7bca..870ee07bf 100644 --- a/apps/browser-extension/src/utils/formDetector/__tests__/FormDetector.generic.test.ts +++ b/apps/browser-extension/src/utils/formDetector/__tests__/FormDetector.generic.test.ts @@ -79,4 +79,75 @@ describe('FormDetector generic tests', () => { expect(form).toBe(false); }); }); + + describe('Nested custom elements (parent-child duplicate prevention)', () => { + describe('TrueNAS-style nested custom elements', () => { + const htmlFile = 'nested-custom-elements.html'; + + it('should not detect both parent custom element and child input as separate password fields', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + + // Click on the actual password input element + const passwordInput = document.getElementById('password-field'); + const formDetector = new FormDetector(document, passwordInput as HTMLElement); + + // Get the detected form + const form = formDetector.getForm(); + expect(form).toBeTruthy(); + + // Should detect only ONE password field + expect(form?.passwordField).toBeTruthy(); + expect(form?.passwordConfirmField).toBeFalsy(); + + // The detected password field should be the actual input element + expect(form?.passwordField?.tagName.toLowerCase()).toBe('input'); + expect(form?.passwordField?.type).toBe('password'); + expect(form?.passwordField?.id).toBe('password-field'); + }); + + it('should detect username field correctly without duplication', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + + const usernameInput = document.getElementById('username-field'); + const formDetector = new FormDetector(document, usernameInput as HTMLElement); + + const form = formDetector.getForm(); + expect(form).toBeTruthy(); + + // Should detect the username field + expect(form?.usernameField).toBeTruthy(); + expect(form?.usernameField?.tagName.toLowerCase()).toBe('input'); + expect(form?.usernameField?.id).toBe('username-field'); + }); + }); + + describe('Nested custom elements with actual password confirm field', () => { + const htmlFile = 'nested-custom-elements-confirm.html'; + + it('should correctly identify actual password confirm fields vs parent-child duplicates', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + + const passwordElement = document.getElementById('password-field'); + const formDetector = new FormDetector(document, passwordElement as HTMLElement); + + const form = formDetector.getForm(); + expect(form).toBeTruthy(); + + // Should correctly detect both password and confirm as separate fields + expect(form?.passwordField).toBeTruthy(); + expect(form?.passwordConfirmField).toBeTruthy(); + + // Both should be actual input elements + expect(form?.passwordField?.tagName.toLowerCase()).toBe('input'); + expect(form?.passwordConfirmField?.tagName.toLowerCase()).toBe('input'); + + // They should be different elements + expect(form?.passwordField?.id).toBe('password-field'); + expect(form?.passwordConfirmField?.id).toBe('password-confirm-field'); + }); + }); + }); }); diff --git a/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements-confirm.html b/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements-confirm.html new file mode 100644 index 000000000..750a32ff2 --- /dev/null +++ b/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements-confirm.html @@ -0,0 +1,25 @@ + + + + Registration Form - Nested Custom Elements with Confirm + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + \ No newline at end of file diff --git a/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements.html b/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements.html new file mode 100644 index 000000000..b541e54a3 --- /dev/null +++ b/apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nested-custom-elements.html @@ -0,0 +1,26 @@ + + + + TrueNAS Login - Nested Custom Elements + + +
+
+ + +
+ +
+
+
+
+ + +
+ +
+
+
+
+ + \ No newline at end of file