Bug 2081620 - Keep page dialogs open until accept_dialog or dismiss_d…

src/firefox/core.ts

@@ -157,6 +157,9 @@ const BINARY_NOT_FOUND_HINT =
   'Unable to detect Firefox binary automatically, please provide the full path via ' +
   '--firefox-path';
 
+/** Leave prompts open until accept_dialog or dismiss_dialog handles them. */
+const UNHANDLED_PROMPT_BEHAVIOR = 'ignore';
+
 export class FirefoxCore {
   private currentContextId: string | null = null;
   private detectedBinaryPath: string | undefined;
@@ -218,6 +221,7 @@ export class FirefoxCore {
 
       const caps = new Capabilities();
       caps.set('webSocketUrl', true);
+      caps.set('unhandledPromptBehavior', UNHANDLED_PROMPT_BEHAVIOR);
       caps.set('moz:firefoxOptions', mozOptions);
       if (this.options.acceptInsecureCerts) {
         caps.set('acceptInsecureCerts', true);
@@ -275,6 +279,7 @@ export class FirefoxCore {
       // session in a locked state that blocks reconnection.
       const caps = new Capabilities();
       caps.set('webSocketUrl', true);
+      caps.set('unhandledPromptBehavior', UNHANDLED_PROMPT_BEHAVIOR);
 
       // createSession() returns synchronously; the session is established async under the hood.
       // Passing geckodriverPath to ServiceBuilder prevents getBinaryPaths() from running,
@@ -317,6 +322,7 @@ export class FirefoxCore {
       // Standard path: launch a new Firefox via selenium-webdriver
       const firefoxOptions = new firefox.Options();
       firefoxOptions.enableBidi();
+      firefoxOptions.setAlertBehavior(UNHANDLED_PROMPT_BEHAVIOR);
 
       if (this.options.headless) {
         firefoxOptions.addArguments('-headless');

src/firefox/pages.ts

@@ -36,6 +36,18 @@ export function isReadinessState(value: unknown): value is BrowsingContext.Readi
   return READINESS_STATES.includes(value as BrowsingContext.ReadinessState);
 }
 
+/**
+ * Keep NoSuchAlertError as is so callers can tell it apart: Marionette sends it with an empty message.
+ */
+function dialogError(action: 'accept' | 'dismiss', error: unknown): Error {
+  if (error instanceof Error && error.name === 'NoSuchAlertError') {
+    return error;
+  }
+  return new Error(
+    `Failed to ${action} dialog: ${error instanceof Error ? error.message : String(error)}`
+  );
+}
+
 export class PageManagement {
   constructor(
     private driver: WebDriver,
@@ -108,9 +120,7 @@ export class PageManagement {
       }
       await alert.accept();
     } catch (error) {
-      throw new Error(
-        `Failed to accept dialog: ${error instanceof Error ? error.message : String(error)}`
-      );
+      throw dialogError('accept', error);
     }
   }
 
@@ -122,9 +132,7 @@ export class PageManagement {
       const alert = await this.driver.switchTo().alert();
       await alert.dismiss();
     } catch (error) {
-      throw new Error(
-        `Failed to dismiss dialog: ${error instanceof Error ? error.message : String(error)}`
-      );
+      throw dialogError('dismiss', error);
     }
   }
 

src/tools/utilities.ts

@@ -92,10 +92,8 @@ export const handleAcceptDialog = defineToolHandler(async function handleAcceptD
     await firefox.acceptDialog(promptText);
     return successResponse(promptText ? `Accepted: "${promptText}"` : 'Accepted');
   } catch (error) {
-    const errorMsg = (error as Error).message;
-
     // Concise error for no active dialog
-    if (errorMsg.includes('no such alert') || errorMsg.includes('No dialog')) {
+    if ((error as Error).name === 'NoSuchAlertError') {
       throw new Error('No active dialog');
     }
 
@@ -113,10 +111,8 @@ export const handleDismissDialog = defineToolHandler(async function handleDismis
     await firefox.dismissDialog();
     return successResponse('Dismissed');
   } catch (error) {
-    const errorMsg = (error as Error).message;
-
     // Concise error for no active dialog
-    if (errorMsg.includes('no such alert') || errorMsg.includes('No dialog')) {
+    if ((error as Error).name === 'NoSuchAlertError') {
       throw new Error('No active dialog');
     }
 

tests/firefox/connect-existing.test.ts

@@ -136,6 +136,7 @@ describe('FirefoxCore connect() BiDi endpoint check', () => {
 
   // Mocks for the launch path (used by the launch-mode test only)
   const mockEnableBidi = vi.fn();
+  const mockSetAlertBehavior = vi.fn();
   const mockOptionsAddArguments = vi.fn();
 
   // Fake socket for the Marionette port probe: emits `event` on the next tick.
@@ -183,6 +184,7 @@ describe('FirefoxCore connect() BiDi endpoint check', () => {
       default: {
         Options: class {
           enableBidi = mockEnableBidi;
+          setAlertBehavior = mockSetAlertBehavior;
           addArguments = mockOptionsAddArguments;
           windowSize = vi.fn();
           setBinary = vi.fn();
@@ -373,4 +375,34 @@ describe('FirefoxCore connect() BiDi endpoint check', () => {
     expect(mockCreateSession).not.toHaveBeenCalled();
     expect(core.getCurrentContextId()).toBe('mock-context-id');
   });
+
+  describe('unhandled prompt behavior', () => {
+    it('should keep prompts open in launch mode', async () => {
+      const { FirefoxCore } = await import('@/firefox/core.js');
+
+      await new FirefoxCore({ headless: true }).connect();
+
+      expect(mockSetAlertBehavior).toHaveBeenCalledWith('ignore');
+    });
+
+    it('should keep prompts open in connect-existing mode', async () => {
+      mockCreateSession.mockReturnValue(
+        makeDriver({ browserVersion: '142.0', webSocketUrl: 'ws://127.0.0.1:9222/session/abc' })
+      );
+      const { FirefoxCore } = await import('@/firefox/core.js');
+
+      await new FirefoxCore({ connectExisting: true, marionettePort: 2828 }).connect();
+
+      expect(mockCapabilitiesSet).toHaveBeenCalledWith('unhandledPromptBehavior', 'ignore');
+    });
+
+    it('should keep prompts open in Android mode', async () => {
+      mockCreateSession.mockReturnValue(makeDriver({ browserVersion: '142.0' }));
+      const { FirefoxCore } = await import('@/firefox/core.js');
+
+      await new FirefoxCore({ androidDevice: 'auto', androidWipeAppData: true }).connect();
+
+      expect(mockCapabilitiesSet).toHaveBeenCalledWith('unhandledPromptBehavior', 'ignore');
+    });
+  });
 });

tests/firefox/core-prefs.test.ts

@@ -27,6 +27,7 @@ describe('FirefoxCore prefs via firefoxOptions', () => {
       default: {
         Options: class {
           enableBidi = vi.fn();
+          setAlertBehavior = vi.fn();
           addArguments = vi.fn();
           setBinary = vi.fn();
           setAcceptInsecureCerts = vi.fn();

tests/firefox/core.test.ts

@@ -313,6 +313,7 @@ describe('FirefoxCore connect() profile handling', () => {
   const mockAddArguments = vi.fn();
   const mockSetProfile = vi.fn();
   const mockEnableBidi = vi.fn();
+  const mockSetAlertBehavior = vi.fn();
   const mockSetBinary = vi.fn();
   const mockWindowSize = vi.fn();
   const mockSetAcceptInsecureCerts = vi.fn();
@@ -327,6 +328,7 @@ describe('FirefoxCore connect() profile handling', () => {
       default: {
         Options: class {
           enableBidi = mockEnableBidi;
+          setAlertBehavior = mockSetAlertBehavior;
           addArguments = mockAddArguments;
           setProfile = mockSetProfile;
           setBinary = mockSetBinary;
@@ -453,6 +455,7 @@ describe('FirefoxCore connect() binary lookup failure', () => {
       default: {
         Options: class {
           enableBidi = vi.fn();
+          setAlertBehavior = vi.fn();
           addArguments = vi.fn();
           setBinary = vi.fn();
           windowSize = vi.fn();

tests/firefox/pages.test.ts

@@ -464,4 +464,54 @@ describe('PageManagement', () => {
       expect(driver.close).not.toHaveBeenCalled();
     });
   });
+
+  describe('dialogs', () => {
+    function createDialogMocks(alert: Record<string, unknown>) {
+      const driver = {
+        switchTo: vi.fn(() => ({ alert: vi.fn().mockResolvedValue(alert) })),
+      } as any;
+      const pages = new PageManagement(driver, vi.fn(), vi.fn(), vi.fn());
+      return { pages, driver };
+    }
+
+    function noSuchAlertError(): Error {
+      const error = new Error('');
+      error.name = 'NoSuchAlertError';
+      return error;
+    }
+
+    it('types the prompt text before accepting', async () => {
+      const alert = { sendKeys: vi.fn(), accept: vi.fn(), dismiss: vi.fn() };
+      const { pages } = createDialogMocks(alert);
+
+      await pages.acceptDialog('hello');
+
+      expect(alert.sendKeys).toHaveBeenCalledWith('hello');
+      expect(alert.accept).toHaveBeenCalledOnce();
+    });
+
+    it('keeps NoSuchAlertError unchanged when there is no dialog', async () => {
+      const error = noSuchAlertError();
+      const alert = {
+        sendKeys: vi.fn(),
+        accept: vi.fn().mockRejectedValue(error),
+        dismiss: vi.fn().mockRejectedValue(error),
+      };
+      const { pages } = createDialogMocks(alert);
+
+      await expect(pages.acceptDialog()).rejects.toBe(error);
+      await expect(pages.dismissDialog()).rejects.toBe(error);
+    });
+
+    it('wraps other errors with the action that failed', async () => {
+      const alert = {
+        accept: vi.fn().mockRejectedValue(new Error('boom')),
+        dismiss: vi.fn().mockRejectedValue(new Error('bang')),
+      };
+      const { pages } = createDialogMocks(alert);
+
+      await expect(pages.acceptDialog()).rejects.toThrow('Failed to accept dialog: boom');
+      await expect(pages.dismissDialog()).rejects.toThrow('Failed to dismiss dialog: bang');
+    });
+  });
 });

tests/integration/dialog.integration.test.ts

@@ -0,0 +1,111 @@
+/**
+ * Integration tests for accept_dialog and dismiss_dialog
+ * Tests with real Firefox browser in headless mode.
+ */
+
+import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
+import type { McpToolResponse } from '@/types/common.js';
+import type { FirefoxClient } from '@/firefox/index.js';
+import { handleAcceptDialog, handleDismissDialog } from '@/tools/utilities.js';
+import { createTestFirefox, closeFirefox, waitFor } from '../helpers/firefox.js';
+
+// The dialog tools resolve the browser through the server's global getFirefox(),
+// so it is redirected to the Firefox instance launched by this test.
+const testState = vi.hoisted(() => ({ firefox: null as FirefoxClient | null }));
+
+vi.mock('../../src/index.js', () => ({
+  getFirefox: async () => testState.firefox,
+}));
+
+const PAGE = 'data:text/html,<h1>dialogs</h1>';
+
+function responseText(response: McpToolResponse): string {
+  return response.content.map((item) => (item.text as string) ?? '').join('\n');
+}
+
+describe('Dialog Integration Tests', () => {
+  let firefox: FirefoxClient;
+
+  beforeAll(async () => {
+    firefox = await createTestFirefox();
+    testState.firefox = firefox;
+  });
+
+  afterAll(async () => {
+    testState.firefox = null;
+    await closeFirefox(firefox);
+  });
+
+  async function evaluateString(expression: string): Promise<string> {
+    return String(await firefox.evaluate(expression));
+  }
+
+  /**
+   * Opens a dialog from a timer so the evaluation returns, then checks that
+   * the page is still blocked in the dialog after the browser had time to
+   * close it on its own.
+   */
+  async function openDialog(call: string): Promise<void> {
+    await firefox.navigate(PAGE);
+    await firefox.evaluate(
+      `window.result = 'pending'; setTimeout(() => { window.result = ${call}; }, 50); 1`
+    );
+    await new Promise((resolve) => setTimeout(resolve, 500));
+    expect(await evaluateString('String(window.result)')).toBe('pending');
+  }
+
+  async function resultOnceSettled(): Promise<string> {
+    await waitFor(async () => (await evaluateString('String(window.result)')) !== 'pending', 5000);
+    return evaluateString('JSON.stringify(window.result)');
+  }
+
+  it('should leave a confirm open until it is accepted', async () => {
+    await openDialog("confirm('Proceed?')");
+
+    const response = await handleAcceptDialog({});
+
+    expect(response.isError).toBeUndefined();
+    expect(responseText(response)).toBe('Accepted');
+    expect(await resultOnceSettled()).toBe('true');
+  }, 30000);
+
+  it('should leave a confirm open until it is dismissed', async () => {
+    await openDialog("confirm('Proceed?')");
+
+    const response = await handleDismissDialog({});
+
+    expect(response.isError).toBeUndefined();
+    expect(responseText(response)).toBe('Dismissed');
+    expect(await resultOnceSettled()).toBe('false');
+  }, 30000);
+
+  it('should return the text given to accept_dialog from a prompt', async () => {
+    await openDialog("prompt('Name?')");
+
+    const response = await handleAcceptDialog({ promptText: 'Ada' });
+
+    expect(response.isError).toBeUndefined();
+    expect(await resultOnceSettled()).toBe('"Ada"');
+  }, 30000);
+
+  it('should return null from a prompt that is dismissed', async () => {
+    await openDialog("prompt('Name?')");
+
+    const response = await handleDismissDialog({});
+
+    expect(response.isError).toBeUndefined();
+    expect(await resultOnceSettled()).toBe('null');
+  }, 30000);
+
+  it('should report no active dialog when none is open', async () => {
+    await firefox.navigate(PAGE);
+
+    const accepted = await handleAcceptDialog({});
+    const dismissed = await handleDismissDialog({});
+
+    expect(accepted.isError).toBe(true);
+    expect(responseText(accepted)).toBe('Error: No active dialog');
+    expect(dismissed.isError).toBe(true);
+    expect(responseText(dismissed)).toBe('Error: No active dialog');
+  }, 30000);
+});