Fix tab list command not recognizing new pages opened via clicks (#275)

## Summary

Fixed an issue where the `tab list` command couldn't recognize new pages that were opened externally (e.g., via `target="_blank"` links or popup windows). The problem occurred because context-level page tracking wasn't properly set up for all browser launch methods, causing new pages created outside of explicit `newTab()` calls to go untracked.

## Changes

- Added `setupContextTracking(context)` calls to `launch()`, `launchIncognito()`, and other context creation methods to ensure all contexts listen for new page events
- Added duplicate page checks (`!this.pages.includes(page)`) in `setupContextTracking()`, `newTab()`, and `launchIncognito()` to prevent the same page from being tracked multiple times
- Fixed `activePageIndex` calculation in `launch()` to properly set the active page index
- Enhanced comments to clarify that `setupContextTracking()` handles externally created pages (popups, new tabs from links)

## Implementation Details

The fix ensures that when a user clicks an element that opens a new tab/window, the browser context's 'page' event listener will automatically detect and track the new page. The duplicate prevention logic handles cases where both the context listener and manual page creation might try to add the same page.

Fixes #273
This commit is contained in:
Chris Tate
2026-01-26 01:25:49 -06:00
committed by GitHub
parent f862e2f7df
commit 1b26ff886c
+26 -14
View File
@@ -778,6 +778,7 @@ export class BrowserManager {
this.browser = browser;
context.setDefaultTimeout(10000);
this.contexts.push(context);
this.setupContextTracking(context);
this.pages.push(page);
this.activePageIndex = 0;
this.setupPageTracking(page);
@@ -1130,11 +1131,15 @@ export class BrowserManager {
context.setDefaultTimeout(60000);
this.contexts.push(context);
this.setupContextTracking(context);
const page = context.pages()[0] ?? (await context.newPage());
this.pages.push(page);
this.activePageIndex = 0;
this.setupPageTracking(page);
// Only add if not already tracked (setupContextTracking may have already added it via 'page' event)
if (!this.pages.includes(page)) {
this.pages.push(page);
this.setupPageTracking(page);
}
this.activePageIndex = this.pages.length > 0 ? this.pages.length - 1 : 0;
}
/**
@@ -1243,12 +1248,16 @@ export class BrowserManager {
}
/**
* Set up tracking for new pages in a context (for CDP connections)
* Set up tracking for new pages in a context (for CDP connections and popups/new tabs)
* This handles pages created externally (e.g., via target="_blank" links)
*/
private setupContextTracking(context: BrowserContext): void {
context.on('page', (page) => {
this.pages.push(page);
this.setupPageTracking(page);
// Only add if not already tracked (avoids duplicates when newTab() creates pages)
if (!this.pages.includes(page)) {
this.pages.push(page);
this.setupPageTracking(page);
}
});
}
@@ -1265,12 +1274,13 @@ export class BrowserManager {
const context = this.contexts[0]; // Use first context for tabs
const page = await context.newPage();
this.pages.push(page);
// Only add if not already tracked (setupContextTracking may have already added it via 'page' event)
if (!this.pages.includes(page)) {
this.pages.push(page);
this.setupPageTracking(page);
}
this.activePageIndex = this.pages.length - 1;
// Set up tracking for the new page
this.setupPageTracking(page);
return { index: this.activePageIndex, total: this.pages.length };
}
@@ -1290,14 +1300,16 @@ export class BrowserManager {
});
context.setDefaultTimeout(60000);
this.contexts.push(context);
this.setupContextTracking(context);
const page = await context.newPage();
this.pages.push(page);
// Only add if not already tracked (setupContextTracking may have already added it via 'page' event)
if (!this.pages.includes(page)) {
this.pages.push(page);
this.setupPageTracking(page);
}
this.activePageIndex = this.pages.length - 1;
// Set up tracking for the new page
this.setupPageTracking(page);
return { index: this.activePageIndex, total: this.pages.length };
}