From 4d42e7b32e4be07ae8220cf977130c51b49446d2 Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Wed, 19 Feb 2025 12:56:11 +0100 Subject: [PATCH] Update form detection logic, update tests (#541) --- .../src/shared/formDetector/FormDetector.ts | 129 +++++++----------- .../__tests__/FormDetector.en.test.ts | 3 +- .../__tests__/FormDetector.generic.test.ts | 44 ++++++ .../__tests__/FormDetector.nl.test.ts | 6 +- .../formDetector/__tests__/TestUtils.ts | 90 ++++++------ .../test-forms/autocomplete-off.html | 15 ++ .../__tests__/test-forms/en-email-form1.html | 2 +- .../__tests__/test-forms/invalid-form1.html | 37 +++++ .../__tests__/test-forms/invalid-form2.html | 117 ++++++++++++++++ 9 files changed, 312 insertions(+), 131 deletions(-) create mode 100644 browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.generic.test.ts create mode 100644 browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/autocomplete-off.html create mode 100644 browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/invalid-form1.html create mode 100644 browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/invalid-form2.html diff --git a/browser-extensions/chrome/src/shared/formDetector/FormDetector.ts b/browser-extensions/chrome/src/shared/formDetector/FormDetector.ts index 918e46c79..7bb84d8de 100644 --- a/browser-extensions/chrome/src/shared/formDetector/FormDetector.ts +++ b/browser-extensions/chrome/src/shared/formDetector/FormDetector.ts @@ -18,54 +18,18 @@ export class FormDetector { } /** - * Detect login forms on the page, prioritizing the form containing the clicked element. + * Detect login forms on the page based on the clicked element. */ public detectForms(): LoginForm[] { - // If we have a clicked element, try to find its form first if (this.clickedElement) { - const formWrapper = this.clickedElement.closest('form'); + const formWrapper = this.clickedElement.closest('form') ?? this.document.body; - if (formWrapper) { + // Check if the wrapper contains a password or likely username field before processing. + if (this.containsPasswordField(formWrapper) || this.containsLikelyUsernameOrEmailField(formWrapper)) { this.createFormEntry(formWrapper); - - // If we found a valid form, return early - if (this.forms.length > 0) { - return this.forms; - } } } - // Find all input fields that could be part of a login form - const passwordFields = this.document.querySelectorAll('input[type="password"]'); - const emailFields = this.document.querySelectorAll('input[type="email"], input[type="text"]'); - const textFields = this.document.querySelectorAll('input[type="text"]'); - - // Process password fields first - passwordFields.forEach(passwordField => { - const form = passwordField.closest('form'); - this.createFormEntry(form); - }); - - // Process email fields that aren't already part of a processed form - emailFields.forEach(field => { - const form = field.closest('form'); - if (form && this.processedForms.has(form)) return; - - if (this.isLikelyEmailField(field)) { - this.createFormEntry(form); - } - }); - - // Process potential username fields that aren't already part of a processed form - textFields.forEach(field => { - const form = field.closest('form'); - if (form && this.processedForms.has(form)) return; - - if (this.isLikelyUsernameField(field)) { - this.createFormEntry(form); - } - }); - return this.forms; } @@ -318,38 +282,6 @@ export class FormDetector { }; } - /** - * Check if a field is likely an email field based on its attributes - */ - private isLikelyEmailField(input: HTMLInputElement): boolean { - const attributes = [ - input.type, - input.id, - input.name, - input.className, - input.placeholder - ].map(attr => attr?.toLowerCase() ?? ''); - - const patterns = ['email', 'e-mail', 'mail', 'address', '@']; - return patterns.some(pattern => attributes.some(attr => attr.includes(pattern))); - } - - /** - * Check if a field is likely a username field based on its attributes - */ - private isLikelyUsernameField(input: HTMLInputElement): boolean { - const attributes = [ - input.type, - input.id, - input.name, - input.className, - input.placeholder - ].map(attr => attr?.toLowerCase() ?? ''); - - const patterns = ['user', 'username', 'login', 'identifier']; - return patterns.some(pattern => attributes.some(attr => attr.includes(pattern))); - } - /** * Find the password field in a form. */ @@ -410,46 +342,77 @@ export class FormDetector { }; } + /** + * Check if a form contains a password field. + */ + private containsPasswordField(wrapper: HTMLElement): boolean { + const passwordFields = this.findPasswordField(wrapper as HTMLFormElement | null); + if (passwordFields.primary) { + return true; + } + + return false; + } + + /** + * Check if a form contains a likely username or email field. + */ + private containsLikelyUsernameOrEmailField(wrapper: HTMLElement): boolean { + // Check if the form contains an email field. + const emailFields = this.findEmailField(wrapper as HTMLFormElement | null); + if (emailFields.primary && emailFields.primary.getAttribute('autocomplete') !== 'off') { + return true; + } + + // Check if the form contains a username field. + const usernameField = this.findInputField(wrapper as HTMLFormElement | null, ['username', 'gebruikersnaam', 'gebruiker', 'login', 'identifier', 'user'], ['text'], []); + if (usernameField && usernameField.getAttribute('autocomplete') !== 'off') { + return true; + } + + return false; + } + /** * Create a form entry. */ - private createFormEntry(form: HTMLFormElement | null): void { + private createFormEntry(wrapper: HTMLElement | null): void { // Skip if we've already processed this form - if (form && this.processedForms.has(form)) return; - this.processedForms.add(form); + if (wrapper && this.processedForms.has(wrapper as HTMLFormElement)) return; + this.processedForms.add(wrapper as HTMLFormElement); // Keep track of detected fields to prevent overlap const detectedFields: HTMLInputElement[] = []; // Find fields in priority order (most specific to least specific). - const emailFields = this.findEmailField(form); + const emailFields = this.findEmailField(wrapper as HTMLFormElement | null); if (emailFields.primary) detectedFields.push(emailFields.primary); if (emailFields.confirm) detectedFields.push(emailFields.confirm); - const passwordFields = this.findPasswordField(form); + const passwordFields = this.findPasswordField(wrapper as HTMLFormElement | null); if (passwordFields.primary) detectedFields.push(passwordFields.primary); if (passwordFields.confirm) detectedFields.push(passwordFields.confirm); - const usernameField = this.findInputField(form, ['username', 'gebruikersnaam', 'gebruiker', 'login', 'identifier', 'user'],['text'], detectedFields); + const usernameField = this.findInputField(wrapper as HTMLFormElement | null, ['username', 'gebruikersnaam', 'gebruiker', 'login', 'identifier', 'user'],['text'], detectedFields); if (usernameField) detectedFields.push(usernameField); - const firstNameField = this.findInputField(form, ['firstname', 'first-name', 'fname', 'voornaam', 'name'], ['text'], detectedFields); + const firstNameField = this.findInputField(wrapper as HTMLFormElement | null, ['firstname', 'first-name', 'fname', 'voornaam', 'name'], ['text'], detectedFields); if (firstNameField) detectedFields.push(firstNameField); - const lastNameField = this.findInputField(form, ['lastname', 'last-name', 'lname', 'achternaam'], ['text'], detectedFields); + const lastNameField = this.findInputField(wrapper as HTMLFormElement | null, ['lastname', 'last-name', 'lname', 'achternaam'], ['text'], detectedFields); if (lastNameField) detectedFields.push(lastNameField); - const birthdateField = this.findBirthdateFields(form, detectedFields); + const birthdateField = this.findBirthdateFields(wrapper as HTMLFormElement | null, detectedFields); if (birthdateField.single) detectedFields.push(birthdateField.single); if (birthdateField.day) detectedFields.push(birthdateField.day); if (birthdateField.month) detectedFields.push(birthdateField.month); if (birthdateField.year) detectedFields.push(birthdateField.year); - const genderField = this.findGenderField(form, detectedFields); + const genderField = this.findGenderField(wrapper as HTMLFormElement | null, detectedFields); if (genderField.field) detectedFields.push(genderField.field as HTMLInputElement); this.forms.push({ - form, + form: wrapper as HTMLFormElement, emailField: emailFields.primary, emailConfirmField: emailFields.confirm, usernameField, diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.en.test.ts b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.en.test.ts index ee829ec6f..738c6cb7e 100644 --- a/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.en.test.ts +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.en.test.ts @@ -1,7 +1,7 @@ import { describe } from 'vitest'; import { FormField, testField } from './TestUtils'; -describe('FormDetector', () => { +describe('FormDetector English tests', () => { describe('English registration form 1 detection', () => { const htmlFile = 'en-registration-form1.html'; @@ -26,6 +26,7 @@ describe('FormDetector', () => { describe('English email form 1 detection', () => { const htmlFile = 'en-email-form1.html'; + // Assert that this test fails, because the autocomplete=off for the specified element. testField(FormField.Email, 'P0-0', htmlFile); }); }); diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.generic.test.ts b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.generic.test.ts new file mode 100644 index 000000000..4b84c7a6e --- /dev/null +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.generic.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { createTestDom } from './TestUtils'; +import { FormDetector } from '../FormDetector'; + +describe('FormDetector generic tests', () => { + describe('Invalid form not detected as login form 1', () => { + const htmlFile = 'invalid-form1.html'; + + it('should not detect any forms', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + const formDetector = new FormDetector(document); + const forms = formDetector.detectForms(); + expect(forms).toHaveLength(0); + }); + }); + + describe('Invalid form not detected as login form 2', () => { + const htmlFile = 'invalid-form2.html'; + + it('should not detect any forms even when clicking search input', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + + // Pass the search input as the clicked element to test if it's still not detected as a login form. + const searchInput = document.getElementById('js-issues-search'); + const formDetector = new FormDetector(document, searchInput as HTMLElement); + const forms = formDetector.detectForms(); + expect(forms).toHaveLength(0); + }); + }); + + describe('Form with autocomplete="off" not detected', () => { + const htmlFile = 'autocomplete-off.html'; + + it('should not detect form with autocomplete="off" on email field', () => { + const dom = createTestDom(htmlFile); + const document = dom.window.document; + const formDetector = new FormDetector(document); + const forms = formDetector.detectForms(); + expect(forms).toHaveLength(0); + }); + }); +}); diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.nl.test.ts b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.nl.test.ts index d90a43bfd..888647374 100644 --- a/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.nl.test.ts +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/FormDetector.nl.test.ts @@ -1,7 +1,7 @@ import { describe } from 'vitest'; import { FormField, testField, testBirthdateFormat } from './TestUtils'; -describe('FormDetector', () => { +describe('FormDetector Dutch tests', () => { describe('Dutch registration form detection', () => { const htmlFile = 'nl-registration-form1.html'; @@ -35,7 +35,7 @@ describe('FormDetector', () => { testField(FormField.Password, 'password', htmlFile); testField(FormField.BirthDate, 'date', htmlFile); - testBirthdateFormat('dd-mm-yyyy', htmlFile); + testBirthdateFormat('dd-mm-yyyy', htmlFile, 'date'); testField(FormField.GenderMale, 'gender1', htmlFile); testField(FormField.GenderFemale, 'gender2', htmlFile); testField(FormField.GenderOther, 'gender3', htmlFile); @@ -55,7 +55,7 @@ describe('FormDetector', () => { testField(FormField.FirstName, 'input_25_14', htmlFile); testField(FormField.LastName, 'input_25_15', htmlFile); testField(FormField.BirthDate, 'input_25_10', htmlFile); - testBirthdateFormat('dd/mm/yyyy', htmlFile); + testBirthdateFormat('dd/mm/yyyy', htmlFile, 'input_25_10'); }); describe('Dutch registration form 6 detection', () => { diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/TestUtils.ts b/browser-extensions/chrome/src/shared/formDetector/__tests__/TestUtils.ts index c8235ed22..3d19b6114 100644 --- a/browser-extensions/chrome/src/shared/formDetector/__tests__/TestUtils.ts +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/TestUtils.ts @@ -5,46 +5,6 @@ import { it, expect } from 'vitest'; import { JSDOM } from 'jsdom'; import { LoginForm } from '../types/LoginForm'; -/** - * Load a test HTML file. - */ -const loadTestHtml = (filename: string): string => { - return readFileSync(join(__dirname, 'test-forms', filename), 'utf-8'); -}; - -/** - * Setup a form detection test. - */ -const setupFormTest = (htmlFile: string, focusedElementId?: string) : { document: Document, result: LoginForm } => { - const html = loadTestHtml(htmlFile); - const dom = new JSDOM(html, { - url: 'http://localhost', - runScripts: 'dangerously', - resources: 'usable' - }); - const document = dom.window.document; - - // Set focus on specified element if provided - let focusedElement: HTMLElement | null = null; - if (focusedElementId) { - focusedElement = document.getElementById(focusedElementId); - if (!focusedElement) { - throw new Error(`Focus element with id "${focusedElementId}" not found in test HTML`); - } - focusedElement.focus(); - - // Create a new form detector with the focused element. - const formDetector = new FormDetector(document, focusedElement); - const result = formDetector.detectForms()[0]; - return { document, result }; - } - - // No focused element, so just detect the first form. - const formDetector = new FormDetector(document); - const result = formDetector.detectForms()[0]; - return { document, result }; -}; - export enum FormField { Username = 'username', FirstName = 'firstName', @@ -64,6 +24,19 @@ export enum FormField { GenderOther = 'genderOther' } +/** + * Create a JSDOM instance for a test HTML file that can be used to provide as + * input to the form detector logic. + */ +export const createTestDom = (htmlFile: string) : JSDOM => { + const html = loadTestHtml(htmlFile); + return new JSDOM(html, { + url: 'http://localhost', + runScripts: 'dangerously', + resources: 'usable' + }); +}; + /** * Helper function to test field detection */ @@ -109,9 +82,40 @@ export const testField = (fieldName: FormField, elementId: string, htmlFile: str /** * Test the birthdate format. */ -export const testBirthdateFormat = (expectedFormat: string, htmlFile: string) : void => { +export const testBirthdateFormat = (expectedFormat: string, htmlFile: string, focusedElementId: string) : void => { it('should detect correct birthdate format', () => { - const { result } = setupFormTest(htmlFile); + const { result } = setupFormTest(htmlFile, focusedElementId); expect(result.birthdateField.format).toBe(expectedFormat); }); -}; \ No newline at end of file +}; + +/** + * Load a test HTML file. + */ +const loadTestHtml = (filename: string): string => { + return readFileSync(join(__dirname, 'test-forms', filename), 'utf-8'); +}; + +/** + * Set up a form detection test. + */ +const setupFormTest = (htmlFile: string, focusedElementId: string) : { document: Document, result: LoginForm } => { + const html = loadTestHtml(htmlFile); + const dom = new JSDOM(html, { + url: 'http://localhost', + runScripts: 'dangerously', + resources: 'usable' + }); + const document = dom.window.document; + + // Set focus on specified element if provided + let focusedElement = document.getElementById(focusedElementId); + if (!focusedElement) { + throw new Error(`Focus element with id "${focusedElementId}" not found in test HTML`); + } + + // Create a new form detector with the focused element. + const formDetector = new FormDetector(document, focusedElement); + const result = formDetector.detectForms()[0]; + return { document, result }; +}; diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/autocomplete-off.html b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/autocomplete-off.html new file mode 100644 index 000000000..231e598da --- /dev/null +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/autocomplete-off.html @@ -0,0 +1,15 @@ +

Create your account

+ +
Newsletters may contain information about Guardian products, services and chosen charities or online advertisements.
This service is protected by reCAPTCHA and the Google privacy policy and terms of service apply.

Already have an account? Sign in

\ No newline at end of file diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/en-email-form1.html b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/en-email-form1.html index 231e598da..8814522c8 100644 --- a/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/en-email-form1.html +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/en-email-form1.html @@ -1,4 +1,4 @@ -

Create your account

Create your account

+ + +
+ + +
+
+ +
+
\ No newline at end of file diff --git a/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/invalid-form2.html b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/invalid-form2.html new file mode 100644 index 000000000..856c2bacc --- /dev/null +++ b/browser-extensions/chrome/src/shared/formDetector/__tests__/test-forms/invalid-form2.html @@ -0,0 +1,117 @@ + +
+ +
+ + + + + + + New pull request + New + + +
+
\ No newline at end of file