Update browser extension formDetector logic and add new tests (#1756)

apps/browser-extension/src/utils/formDetector/FormDetector.ts

@@ -568,9 +568,65 @@ export class FormDetector {
       ['text', 'email']
     );
 
-    // Filter out parent-child relationships
+    /*
+     * Filter out parent-child relationships
+     */
     const filteredEmailFields = this.filterOutNestedDuplicates(emailFields);
-    const primaryEmail = filteredEmailFields[0] ?? null;
+
+    /*
+     * Filter out fields that are more likely to be username fields.
+     * Some forms have labels like "Username / Email" or "Gebruikersnaam / e-mailadres"
+     * which can match both patterns. We need to check if the label contains BOTH
+     * username and email keywords to determine if this is a dual-purpose field.
+     */
+    const emailFieldsWithoutUsernamePriority = filteredEmailFields.filter(field => {
+      const fieldName = (field.getAttribute('name') || '').toLowerCase();
+      const fieldId = (field.id || '').toLowerCase();
+      const fieldAttributes = `${fieldName} ${fieldId}`;
+
+      /*
+       * Get the label text for this field
+       */
+      let labelText = '';
+      if (field.id || fieldName) {
+        const label = this.document.querySelector(`label[for="${field.id || fieldName}"]`);
+        if (label) {
+          labelText = (label.textContent || '').toLowerCase();
+        }
+      }
+
+      /*
+       * Check if label contains BOTH username and email patterns (dual-purpose field)
+       */
+      const labelHasUsername = CombinedFieldPatterns.username.some(pattern =>
+        labelText.includes(pattern)
+      );
+      const labelHasEmail = CombinedFieldPatterns.email.some(pattern =>
+        labelText.includes(pattern)
+      );
+
+      /*
+       * Only filter out if:
+       * 1. Label contains BOTH username and email keywords (dual-purpose label)
+       * 2. AND the field's name/id contains username pattern but NOT email pattern
+       */
+      if (labelHasUsername && labelHasEmail) {
+        const hasUsernameInNameOrId = CombinedFieldPatterns.username.some(pattern =>
+          fieldAttributes.includes(pattern)
+        );
+        const hasEmailInNameOrId = CombinedFieldPatterns.email.some(pattern =>
+          fieldAttributes.includes(pattern)
+        );
+
+        if (hasUsernameInNameOrId && !hasEmailInNameOrId) {
+          return false;
+        }
+      }
+
+      return true;
+    });
+
+    const primaryEmail = emailFieldsWithoutUsernamePriority[0] ?? null;
 
     /*
      * Find confirmation email field if primary exists
@@ -1072,7 +1128,17 @@ export class FormDetector {
       detectedFields.push(lastNameField);
     }
 
-    const firstNameField = this.findInputField(wrapper as HTMLFormElement | null, CombinedFieldPatterns.firstName, ['text'], detectedFields);
+    /*
+     * For login forms (username + password WITHOUT email or confirmation fields),
+     * skip firstName detection to avoid matching session fields or other inputs.
+     * If there's an email field alongside username, it's likely a registration form.
+     */
+    const isLikelyLoginForm = usernameField && passwordFields.primary &&
+                               !emailFields.primary &&
+                               !emailFields.confirm && !passwordFields.confirm;
+
+    const firstNameField = !isLikelyLoginForm ?
+      this.findInputField(wrapper as HTMLFormElement | null, CombinedFieldPatterns.firstName, ['text'], detectedFields) : null;
     if (firstNameField) {
       detectedFields.push(firstNameField);
     }

apps/browser-extension/src/utils/formDetector/__tests__/FormDetectorFiller.nl-login-form1.test.ts

@@ -0,0 +1,266 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+import type { Credential } from '@/utils/dist/core/models/vault';
+
+import { FormDetector } from '../FormDetector';
+import { FormFiller } from '../FormFiller';
+
+import { createTestDom, createMockCredential, wasTriggerCalledFor } from './TestUtils';
+
+/**
+ * Tests for Dutch login form with multiple hidden fields and session options.
+ */
+describe('Dutch login form 1 detection and filling', () => {
+  const htmlFile = 'nl-login-form1.html';
+
+  describe('Field detection', () => {
+    it('should detect username field despite autocomplete="off"', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      // Get the username input field
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      expect(usernameInput).not.toBeNull();
+      expect(usernameInput.getAttribute('autocomplete')).toBe('off');
+
+      // Create form detector with username field focused
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      // Verify username field is detected despite autocomplete="off"
+      expect(detectedFields?.usernameField).toBe(usernameInput);
+      expect(detectedFields?.usernameField?.id).toBe('login_form_user');
+    });
+
+    it('should detect password field with autocomplete="current-password"', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      // Get the password input field
+      const passwordInput = doc.getElementById('login_form_password') as HTMLInputElement;
+      expect(passwordInput).not.toBeNull();
+      expect(passwordInput.getAttribute('autocomplete')).toBe('current-password');
+
+      // Create form detector with password field focused
+      const formDetector = new FormDetector(doc, passwordInput);
+      const detectedFields = formDetector.getForm();
+
+      // Verify password field is detected
+      expect(detectedFields?.passwordField).toBe(passwordInput);
+      expect(detectedFields?.passwordField?.id).toBe('login_form_password');
+    });
+
+    it('should not detect hidden fields as login fields', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      // Verify hidden fields are not detected
+      const hiddenLocation = doc.getElementById('login_form_location') as HTMLInputElement;
+      const hiddenToken = doc.getElementById('login_form__token') as HTMLInputElement;
+
+      expect(detectedFields?.usernameField).not.toBe(hiddenLocation);
+      expect(detectedFields?.usernameField).not.toBe(hiddenToken);
+      expect(detectedFields?.passwordField).not.toBe(hiddenLocation);
+      expect(detectedFields?.passwordField).not.toBe(hiddenToken);
+    });
+
+    it('should not detect session option fields as login fields', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      // Verify session option fields are not detected as login fields
+      const sessionNameInput = doc.getElementById('login_form_sessionName') as HTMLInputElement;
+      const durationSelect = doc.getElementById('login_form_duration') as HTMLSelectElement;
+
+      expect(detectedFields?.usernameField).not.toBe(sessionNameInput);
+      expect(detectedFields?.usernameField).not.toBe(durationSelect);
+      expect(detectedFields?.passwordField).not.toBe(sessionNameInput);
+    });
+
+    it('should detect both username and password fields from the same form', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const passwordInput = doc.getElementById('login_form_password') as HTMLInputElement;
+
+      // Detect from username field focus
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      // Both fields should be detected
+      expect(detectedFields?.usernameField).toBe(usernameInput);
+      expect(detectedFields?.passwordField).toBe(passwordInput);
+    });
+  });
+
+  describe('Field filling', () => {
+    let mockCredential: Credential;
+
+    beforeEach(() => {
+      mockCredential = createMockCredential();
+    });
+
+    it('should fill username field successfully', async () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      expect(detectedFields).not.toBeNull();
+
+      if (detectedFields) {
+        const triggerMock = vi.fn();
+        const filler = new FormFiller(detectedFields, triggerMock);
+        await filler.fillFields(mockCredential);
+
+        // Verify username is filled
+        expect(usernameInput.value).toBe('testuser');
+        expect(wasTriggerCalledFor(triggerMock, usernameInput)).toBe(true);
+      }
+    });
+
+    it('should fill password field successfully', async () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const passwordInput = doc.getElementById('login_form_password') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, passwordInput);
+      const detectedFields = formDetector.getForm();
+
+      expect(detectedFields).not.toBeNull();
+
+      if (detectedFields) {
+        const triggerMock = vi.fn();
+        const filler = new FormFiller(detectedFields, triggerMock);
+        await filler.fillFields(mockCredential);
+
+        // Delay for password filling (character-by-character)
+        await new Promise(resolve => setTimeout(resolve, 150));
+
+        // Verify password is filled
+        expect(passwordInput.value).toBe('testpass');
+        expect(wasTriggerCalledFor(triggerMock, passwordInput)).toBe(true);
+      }
+    });
+
+    it('should fill both username and password fields without filling other fields', async () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const passwordInput = doc.getElementById('login_form_password') as HTMLInputElement;
+      const sessionNameInput = doc.getElementById('login_form_sessionName') as HTMLInputElement;
+      const hiddenLocation = doc.getElementById('login_form_location') as HTMLInputElement;
+
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      expect(detectedFields).not.toBeNull();
+
+      if (detectedFields) {
+        const triggerMock = vi.fn();
+        const filler = new FormFiller(detectedFields, triggerMock);
+        await filler.fillFields(mockCredential);
+
+        // Delay for password filling
+        await new Promise(resolve => setTimeout(resolve, 150));
+
+        // Verify correct fields are filled
+        expect(usernameInput.value).toBe('testuser');
+        expect(passwordInput.value).toBe('testpass');
+
+        // Verify other fields are NOT filled
+        expect(sessionNameInput.value).toBe('');
+        expect(hiddenLocation.value).toBe('https://example.com/');
+
+        // Verify trigger events were called for the correct fields only
+        expect(wasTriggerCalledFor(triggerMock, usernameInput)).toBe(true);
+        expect(wasTriggerCalledFor(triggerMock, passwordInput)).toBe(true);
+        expect(wasTriggerCalledFor(triggerMock, sessionNameInput)).toBe(false);
+        expect(wasTriggerCalledFor(triggerMock, hiddenLocation)).toBe(false);
+      }
+    });
+
+    it('should handle form with credential containing email', async () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      expect(detectedFields).not.toBeNull();
+
+      if (detectedFields) {
+        const triggerMock = vi.fn();
+        const filler = new FormFiller(detectedFields, triggerMock);
+
+        // Credential with email should use username if username field exists
+        await filler.fillFields(mockCredential);
+
+        // Username field should be filled with username, not email
+        expect(usernameInput.value).toBe('testuser');
+        expect(usernameInput.value).not.toBe('test@example.com');
+      }
+    });
+  });
+
+  describe('Edge cases and regressions', () => {
+    it('should only fill the actual username field, not hidden fields that might match patterns', async () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+      const detectedFields = formDetector.getForm();
+
+      expect(detectedFields).not.toBeNull();
+
+      if (detectedFields) {
+        const triggerMock = vi.fn();
+        const filler = new FormFiller(detectedFields, triggerMock);
+        await filler.fillFields(createMockCredential());
+
+        // Check that only the visible username field was filled
+        const allInputs = Array.from(doc.querySelectorAll('input[type="text"]')) as HTMLInputElement[];
+        const filledInputs = allInputs.filter(input => input.value !== '');
+
+        // Should have exactly one filled text input (the username field)
+        // Session name field should remain empty
+        expect(filledInputs.length).toBe(1);
+        expect(filledInputs[0]).toBe(usernameInput);
+      }
+    });
+
+    it('should handle form detection when clicking on username field with multiple inputs nearby', () => {
+      const dom = createTestDom(htmlFile);
+      const doc = dom.window.document;
+
+      // Simulate clicking the username field
+      const usernameInput = doc.getElementById('login_form_user') as HTMLInputElement;
+      const formDetector = new FormDetector(doc, usernameInput);
+
+      // Form should be detected as a login form
+      expect(formDetector.containsLoginForm()).toBe(true);
+
+      const detectedFields = formDetector.getForm();
+
+      // Should detect exactly the right fields, not extras
+      expect(detectedFields?.usernameField).toBeTruthy();
+      expect(detectedFields?.passwordField).toBeTruthy();
+      expect(detectedFields?.emailField).toBeFalsy(); // No separate email field
+      expect(detectedFields?.passwordConfirmField).toBeFalsy(); // No password confirm
+    });
+  });
+});

apps/browser-extension/src/utils/formDetector/__tests__/test-forms/nl-login-form1.html

@@ -0,0 +1,88 @@
+<!--
+Dutch login form with multiple hidden fields and session options.
+This form has a username/email field with autocomplete="off"
+and a password field with autocomplete="current-password".
+The form contains multiple hidden fields and session configuration options
+that should not interfere with field detection.
+Tests for edge case where username field should still be detected despite autocomplete="off".
+-->
+<div id="layout" class="content-grid ">
+    <div id="contentArea">
+            <div id="header">
+
+                            <h1>Inloggen</h1>
+
+        </div>
+
+
+    <div id="mytnet" class="small">
+
+        <form name="login_form" method="post" action="https://example.com/login/?location=https%3A%2F%2Fexample.com%2F" id="loginForm">
+
+        <table class="profileLabels formTable" width="100%" cellspacing="0">
+            <tbody><tr>
+                <td class="label"><label for="login_form_user" class="required">Gebruikersnaam / e-mailadres</label></td>
+                <td>
+                    <input type="text" id="login_form_user" name="login_form[user]" required="required" autofocus="autofocus" class="text" autocomplete="off">
+                    <div class="light-account-notice" style="display:none">
+                        <p>Er is nog geen account actief op dit e-mailadres, maar het adres is al wel in gebruik bij een of meerdere prijsalerts. Om het e-mailadres van je prijsalert te wijzigen heb je een account nodig. Met een account kun je:</p>
+                        <ul>
+                            <li>op artikelen reageren en meepraten op het forum</li>
+                            <li>wenslijsten en inventarissen bijhouden</li>
+                            <li>productreviews schrijven en advertenties plaatsen</li>
+                        </ul>
+                        <p><a href="https://example.com/register/?email=__EMAIL__&amp;location=https://example.com/" class="ctaButton">Maak account aan</a></p>
+                    </div>
+                </td>
+            </tr>
+
+            <tr>
+                <td class="label"><label for="login_form_password" class="required">Wachtwoord</label></td>
+                <td>
+                    <input type="password" id="login_form_password" name="login_form[password]" required="required" class="text" autocomplete="current-password"><br>
+
+                    <details>
+                        <summary class="simple-accordion">Inlogopties</summary>
+                        <div id="showMoreOptions">
+                            <div class="row">
+                                                                <input type="checkbox" id="login_form_lock" name="login_form[lock]" autocomplete="off" value="1">
+
+
+                                <label class="sublabel" for="login_form_lock">Vergrendel deze sessie aan mijn IP (xxx.xxx.xxx.xxx)</label>
+                                <br><br>
+                                                                <input type="checkbox" id="login_form_terminate" name="login_form[terminate]" autocomplete="off" value="1">
+
+
+                                <label class="sublabel" for="login_form_terminate">Alle voorgaande sessies beëindigen</label>
+                            </div>
+
+                            <div class="row">
+                                                                <div class="label"><label class="sublabel required" for="login_form_duration">Sessie levensduur</label></div>
+                                <select id="login_form_duration" name="login_form[duration]" class="text" autocomplete="off"><option value="five_minutes">5 min</option><option value="one_hour">1 uur</option><option value="six_hours">6 uur</option><option value="one_day">1 dag</option><option value="one_week">1 week</option><option value="thirty_days">30 dagen</option><option value="one_year" selected="selected">1 jaar</option></select>
+                            </div>
+
+                            <div class="row">
+                                                                <div class="label"><label for="login_form_sessionName">Sessie naam</label></div>
+                                <input type="text" id="login_form_sessionName" name="login_form[sessionName]" class="text" autocomplete="off">
+                            </div>
+                        </div>
+                    </details>
+                </td>
+            </tr>
+
+                    </tbody></table>
+
+        <p class="submit">
+            <a href="https://example.com/passmailer/">Wachtwoord vergeten</a><br><br>
+            <input type="submit" value="Inloggen" class="ctaButton" accesskey="s">
+        </p>
+        <p class="subtext">
+            Heb je nog geen account? <a href="https://example.com/register/">Registreren</a>!
+        </p>
+
+        <input type="hidden" id="login_form_location" name="login_form[location]" value="https://example.com/"><input type="hidden" id="login_form__token" name="login_form[_token]" data-controller="csrf-protection" value="aoA+NBvZaZhw1I8p9IFVaRfwny3psttvL0hTKlTq6iyKV0BSzxOuTCFw">
+        </form>
+    </div>
+<div class="clear"></div>
+    </div>
+</div>