Skip to content

Commit 72aba07

Browse files
csamujuliandescottes
authored andcommitted
test: add unit and integration tests for moz-extension:// navigation fix
1 parent cd0bc6e commit 72aba07

5 files changed

Lines changed: 387 additions & 1 deletion

File tree

src/firefox/pages.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import { log, logDebug } from '../utils/logger.js';
77

88
const PRIVILEGED_URL_SCHEMES = ['moz-extension:'];
99

10-
function isPrivilegedUrl(url: string): boolean {
10+
/**
11+
* Check if a URL uses a privileged scheme that requires BiDi navigation.
12+
* @internal Exported for testability only; not a stable public API.
13+
*/
14+
export function isPrivilegedUrl(url: string): boolean {
1115
try {
1216
return PRIVILEGED_URL_SCHEMES.includes(new URL(url).protocol);
1317
} catch {

tests/firefox/pages.test.ts

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/**
2+
* Unit tests for PageManagement and isPrivilegedUrl
3+
*
4+
* The moz-extension:// fix: geckodriver's driver.get() hangs on extension URLs
5+
* because it waits for BiDi navigation completion events that the Remote Agent
6+
* never emits for extension contexts. The fix routes moz-extension:// URLs
7+
* through BiDi browsingContext.navigate with wait:"none", which returns
8+
* immediately without waiting for load events.
9+
*/
10+
11+
import { describe, it, expect, vi } from 'vitest';
12+
import { isPrivilegedUrl, PageManagement } from '@/firefox/pages.js';
13+
14+
// -- isPrivilegedUrl ----------------------------------------------------------
15+
16+
describe('isPrivilegedUrl', () => {
17+
it('returns true for moz-extension:// URLs', () => {
18+
expect(isPrivilegedUrl('moz-extension://abc123/popup.html')).toBe(true);
19+
});
20+
21+
it('returns false for https:// URLs', () => {
22+
expect(isPrivilegedUrl('https://example.com')).toBe(false);
23+
});
24+
25+
it('returns false for http:// URLs', () => {
26+
expect(isPrivilegedUrl('http://localhost:3000')).toBe(false);
27+
});
28+
29+
it('returns false for file:// URLs', () => {
30+
expect(isPrivilegedUrl('file:///path/to/page.html')).toBe(false);
31+
});
32+
33+
it('returns false for about: URLs (not in PRIVILEGED_URL_SCHEMES)', () => {
34+
expect(isPrivilegedUrl('about:blank')).toBe(false);
35+
});
36+
37+
// Malformed strings make URL constructor throw; the catch block returns false
38+
it('returns false for malformed URLs', () => {
39+
expect(isPrivilegedUrl('not-a-url')).toBe(false);
40+
});
41+
42+
it('returns false for empty string', () => {
43+
expect(isPrivilegedUrl('')).toBe(false);
44+
});
45+
});
46+
47+
// -- PageManagement -----------------------------------------------------------
48+
49+
describe('PageManagement', () => {
50+
type BiDiCommandFn = (method: string, params: Record<string, any>) => Promise<any>;
51+
52+
/**
53+
* Build mocked PageManagement dependencies.
54+
*
55+
* The contextId handling is subtle: null and undefined mean different things.
56+
* - contextId: null → explicitly null (no browsing context available)
57+
* - contextId omitted → defaults to 'ctx-1' (normal case)
58+
*
59+
* We use `'contextId' in overrides` to distinguish these, because
60+
* `null ?? 'ctx-1'` would incorrectly collapse null into the default.
61+
*/
62+
function createMocks(overrides?: { contextId?: string | null; sendBiDiCommand?: BiDiCommandFn }) {
63+
const driver = { get: vi.fn().mockResolvedValue(undefined) } as any;
64+
const hasContextId = 'contextId' in (overrides ?? {});
65+
const contextId = hasContextId ? overrides!.contextId : 'ctx-1';
66+
const getCurrentContextId = vi.fn().mockImplementation(() => contextId);
67+
const setCurrentContextId = vi.fn();
68+
69+
// When sendBiDiCommand is provided, wrap it in a mock so we can assert calls.
70+
// When omitted, it stays undefined — simulates BiDi being unavailable.
71+
const sendBiDiCommand = overrides?.sendBiDiCommand
72+
? vi.fn().mockImplementation(overrides.sendBiDiCommand)
73+
: undefined;
74+
75+
const pages = new PageManagement(driver, getCurrentContextId, setCurrentContextId, sendBiDiCommand as any);
76+
return { pages, driver, getCurrentContextId, setCurrentContextId, sendBiDiCommand: sendBiDiCommand as any };
77+
}
78+
79+
describe('navigate', () => {
80+
// Core fix: moz-extension:// must use BiDi, not driver.get()
81+
it('uses BiDi browsingContext.navigate with wait:none for moz-extension:// URLs', async () => {
82+
const bidiFn = vi.fn().mockResolvedValue({});
83+
const { pages, driver, sendBiDiCommand } = createMocks({ sendBiDiCommand: bidiFn });
84+
85+
await pages.navigate('moz-extension://abc123/popup.html');
86+
87+
expect(sendBiDiCommand).toHaveBeenCalledWith('browsingContext.navigate', {
88+
context: 'ctx-1',
89+
url: 'moz-extension://abc123/popup.html',
90+
wait: 'none',
91+
});
92+
expect(driver.get).not.toHaveBeenCalled();
93+
});
94+
95+
// Guard: no context ID means we can't send a BiDi command.
96+
// Also verify sendBiDiCommand was never called (not just that it threw).
97+
it('throws when context ID is null for moz-extension:// URL', async () => {
98+
const bidiFn = vi.fn().mockResolvedValue({});
99+
const { pages, sendBiDiCommand } = createMocks({ contextId: null, sendBiDiCommand: bidiFn });
100+
101+
await expect(pages.navigate('moz-extension://abc123/popup.html')).rejects.toThrow(
102+
'Cannot navigate to privileged URL moz-extension://abc123/popup.html: no browsing context ID'
103+
);
104+
expect(sendBiDiCommand).not.toHaveBeenCalled();
105+
});
106+
107+
// Fallback: if BiDi is not available (e.g. no Remote Agent), we still try
108+
// driver.get(). This will hang for moz-extension://, but there's no alternative.
109+
it('falls through to driver.get() for moz-extension:// when BiDi is unavailable', async () => {
110+
const { pages, driver, sendBiDiCommand } = createMocks();
111+
112+
await pages.navigate('moz-extension://abc123/popup.html');
113+
114+
expect(driver.get).toHaveBeenCalledWith('moz-extension://abc123/popup.html');
115+
expect(sendBiDiCommand).toBeUndefined();
116+
});
117+
118+
// Non-privileged URLs always use the standard path
119+
it('uses driver.get() for normal URLs', async () => {
120+
const bidiFn = vi.fn().mockResolvedValue({});
121+
const { pages, driver, sendBiDiCommand } = createMocks({ sendBiDiCommand: bidiFn });
122+
123+
await pages.navigate('https://example.com');
124+
125+
expect(driver.get).toHaveBeenCalledWith('https://example.com');
126+
expect(sendBiDiCommand).not.toHaveBeenCalled();
127+
});
128+
129+
// If the BiDi command rejects, the error should propagate (no silent swallow)
130+
it('propagates BiDi navigation errors for moz-extension:// URLs', async () => {
131+
const bidiFn = vi.fn().mockRejectedValue(new Error('BiDi error: invalid context'));
132+
const { pages } = createMocks({ sendBiDiCommand: bidiFn });
133+
134+
await expect(pages.navigate('moz-extension://abc123/popup.html')).rejects.toThrow(
135+
'BiDi error: invalid context'
136+
);
137+
});
138+
});
139+
140+
describe('createNewPage', () => {
141+
// createNewPage must delegate to navigate(), not call driver.get() directly,
142+
// so that the moz-extension:// BiDi path is also used for new tabs
143+
it('uses BiDi navigate for moz-extension:// URL after creating the tab', async () => {
144+
const bidiFn = vi.fn().mockResolvedValue({});
145+
const allHandles = ['handle-1', 'handle-2'];
146+
const driver = {
147+
get: vi.fn().mockResolvedValue(undefined),
148+
switchTo: vi.fn().mockReturnValue({ newWindow: vi.fn().mockResolvedValue(undefined) }),
149+
getAllWindowHandles: vi.fn().mockResolvedValue(allHandles),
150+
} as any;
151+
const getCurrentContextId = vi.fn().mockReturnValue('handle-2');
152+
const setCurrentContextId = vi.fn();
153+
154+
const pages = new PageManagement(driver, getCurrentContextId, setCurrentContextId, bidiFn as any);
155+
156+
const idx = await pages.createNewPage('moz-extension://abc123/popup.html');
157+
158+
// Verify the BiDi navigate was used (not driver.get)
159+
expect(bidiFn).toHaveBeenCalledWith('browsingContext.navigate', {
160+
context: 'handle-2',
161+
url: 'moz-extension://abc123/popup.html',
162+
wait: 'none',
163+
});
164+
// Verify context was set before navigation
165+
expect(setCurrentContextId).toHaveBeenCalledWith('handle-2');
166+
expect(driver.get).not.toHaveBeenCalled();
167+
expect(idx).toBe(1);
168+
});
169+
170+
// With a normal URL, createNewPage delegates to navigate() which uses driver.get()
171+
it('uses driver.get() for normal URLs after creating the tab', async () => {
172+
const bidiFn = vi.fn().mockResolvedValue({});
173+
const allHandles = ['handle-1', 'handle-2'];
174+
const driver = {
175+
get: vi.fn().mockResolvedValue(undefined),
176+
switchTo: vi.fn().mockReturnValue({ newWindow: vi.fn().mockResolvedValue(undefined) }),
177+
getAllWindowHandles: vi.fn().mockResolvedValue(allHandles),
178+
} as any;
179+
const getCurrentContextId = vi.fn().mockReturnValue('handle-2');
180+
const setCurrentContextId = vi.fn();
181+
182+
const pages = new PageManagement(driver, getCurrentContextId, setCurrentContextId, bidiFn as any);
183+
184+
const idx = await pages.createNewPage('https://example.com');
185+
186+
expect(driver.get).toHaveBeenCalledWith('https://example.com');
187+
expect(bidiFn).not.toHaveBeenCalled();
188+
expect(idx).toBe(1);
189+
});
190+
});
191+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"manifest_version": 2,
3+
"name": "MCP Test Extension",
4+
"version": "1.0",
5+
"description": "Minimal extension for testing moz-extension:// navigation",
6+
"browser_action": {
7+
"default_popup": "popup.html"
8+
}
9+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<title>MCP Test Extension</title>
6+
</head>
7+
<body>
8+
<h1>MCP Test Extension</h1>
9+
<p id="status">extension-loaded</p>
10+
</body>
11+
</html>
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/**
2+
* Integration tests for moz-extension:// navigation
3+
* Tests with real Firefox browser in headless mode
4+
*/
5+
6+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
7+
import { createTestFirefox, closeFirefox, waitFor } from '../helpers/firefox.js';
8+
import type { FirefoxClient } from '@/firefox/index.js';
9+
import { resolve } from 'node:path';
10+
import { fileURLToPath } from 'node:url';
11+
import { execFileSync } from 'node:child_process';
12+
import { unlinkSync, existsSync } from 'node:fs';
13+
14+
const __dirname = fileURLToPath(new URL('.', import.meta.url));
15+
const fixturesPath = resolve(__dirname, '../fixtures');
16+
const extensionDir = resolve(fixturesPath, 'test-extension');
17+
const xpiPath = resolve(fixturesPath, 'test-extension.xpi');
18+
19+
function packExtension(): void {
20+
// Remove stale .xpi so zip creates a fresh archive
21+
if (existsSync(xpiPath)) {
22+
unlinkSync(xpiPath);
23+
}
24+
// Use execFileSync to avoid shell interpolation of paths
25+
execFileSync('zip', ['-j', xpiPath, 'manifest.json', 'popup.html'], {
26+
cwd: extensionDir,
27+
stdio: 'pipe',
28+
});
29+
}
30+
31+
/**
32+
* Get the moz-extension:// hostname for an installed extension by switching
33+
* to a chrome-privileged context and querying WebExtensionPolicy.
34+
* Requires MOZ_REMOTE_ALLOW_SYSTEM_ACCESS=1.
35+
*/
36+
async function getExtensionHostname(firefox: FirefoxClient, extensionId: string): Promise<string> {
37+
const treeResult = await firefox.sendBiDiCommand('browsingContext.getTree', {
38+
'moz:scope': 'chrome',
39+
});
40+
const contexts = treeResult.contexts || [];
41+
if (contexts.length === 0) {
42+
throw new Error('No chrome contexts available');
43+
}
44+
45+
const driver = firefox.getDriver();
46+
const originalContextId = firefox.getCurrentContextId();
47+
const chromeContextId = contexts[0].context;
48+
49+
try {
50+
await driver.switchTo().window(chromeContextId);
51+
await driver.setContext('chrome');
52+
53+
const hostname = await driver.executeAsyncScript(`
54+
const callback = arguments[arguments.length - 1];
55+
const extensionId = ${JSON.stringify(extensionId)};
56+
(async () => {
57+
try {
58+
const policy = WebExtensionPolicy.getByID(extensionId);
59+
callback(policy ? policy.mozExtensionHostname : '');
60+
} catch { callback(''); }
61+
})();
62+
`);
63+
64+
if (!hostname) {
65+
throw new Error(
66+
`Could not resolve hostname for extension ${extensionId}, got: ${JSON.stringify(hostname)}`
67+
);
68+
}
69+
return hostname as string;
70+
} finally {
71+
try {
72+
await driver.setContext('content');
73+
if (originalContextId) {
74+
await driver.switchTo().window(originalContextId);
75+
}
76+
} catch (e) {
77+
console.warn('Failed to restore context after getExtensionHostname:', e);
78+
}
79+
}
80+
}
81+
82+
describe('moz-extension:// Navigation Integration Tests', () => {
83+
let firefox: FirefoxClient;
84+
let extensionUrl: string;
85+
let extensionId: string;
86+
87+
beforeAll(async () => {
88+
packExtension();
89+
90+
// MOZ_REMOTE_ALLOW_SYSTEM_ACCESS=1 is required to resolve the extension hostname
91+
firefox = await createTestFirefox({
92+
env: { MOZ_REMOTE_ALLOW_SYSTEM_ACCESS: '1' },
93+
});
94+
95+
// Install the test extension
96+
const installResult = await firefox.sendBiDiCommand('webExtension.install', {
97+
extensionData: { type: 'archivePath', path: xpiPath },
98+
});
99+
extensionId = installResult?.extension;
100+
if (!extensionId) {
101+
throw new Error(`Extension install returned no ID, got: ${JSON.stringify(installResult)}`);
102+
}
103+
104+
// Resolve the moz-extension:// URL from the extension hostname
105+
const hostname = await getExtensionHostname(firefox, extensionId);
106+
extensionUrl = `moz-extension://${hostname}/popup.html`;
107+
}, 60000);
108+
109+
afterAll(async () => {
110+
// Uninstall the test extension to avoid accumulation across test runs
111+
if (extensionId) {
112+
try {
113+
await firefox.sendBiDiCommand('webExtension.uninstall', { extension: extensionId });
114+
} catch {}
115+
}
116+
await closeFirefox(firefox);
117+
// Remove the packed .xpi so no build artifact remains on disk
118+
if (existsSync(xpiPath)) {
119+
unlinkSync(xpiPath);
120+
}
121+
});
122+
123+
it('should navigate to moz-extension:// URL without hanging', async () => {
124+
// Before the fix, driver.get() would hang indefinitely because geckodriver
125+
// waits for BiDi navigation completion events that the Remote Agent does
126+
// not emit for extension contexts.
127+
// wait:"none" returns in milliseconds, so a 5s timeout proves no hang
128+
// while tolerating slow CI.
129+
const result = await Promise.race([
130+
firefox.navigate(extensionUrl).then(() => 'navigated'),
131+
new Promise<string>((resolve) =>
132+
setTimeout(() => resolve('timeout'), 5000)
133+
),
134+
]);
135+
136+
expect(result).toBe('navigated');
137+
138+
// Poll for the page URL since wait:none returns before the page loads
139+
const driver = firefox.getDriver();
140+
await waitFor(async () => {
141+
const url = await driver.getCurrentUrl();
142+
return url.includes('moz-extension://');
143+
}, 5000);
144+
145+
// Verify the extension page content actually loaded
146+
const title = await driver.getTitle();
147+
expect(title).toBe('MCP Test Extension');
148+
}, 15000);
149+
150+
it('should create new page with moz-extension:// URL without hanging', async () => {
151+
const result = await Promise.race([
152+
firefox.createNewPage(extensionUrl).then((idx: number) => `created-${idx}`),
153+
new Promise<string>((resolve) =>
154+
setTimeout(() => resolve('timeout'), 5000)
155+
),
156+
]);
157+
158+
expect(result).toMatch(/^created-/);
159+
160+
// Poll for the page URL since wait:none returns before the page loads
161+
const driver = firefox.getDriver();
162+
await waitFor(async () => {
163+
const url = await driver.getCurrentUrl();
164+
return url.includes('moz-extension://');
165+
}, 5000);
166+
167+
// Verify the extension page content actually loaded
168+
const title = await driver.getTitle();
169+
expect(title).toBe('MCP Test Extension');
170+
}, 15000);
171+
});

0 commit comments

Comments
 (0)