fix: preserve console history across same-document navigations (#2676)

Fixes #2650

Alternative to #2655: that PR is currently failing all CI checks and its
diff reverts the `background` support added in #2658. This
implementation is based on the latest `main` and keeps that feature
intact.

src/collectors/PageCollector.ts

@@ -11,7 +11,7 @@ import type {
   Protocol,
   Issue,
 } from '../third_party/index.js';
-import {DevTools} from '../third_party/index.js';
+import {DevTools, FrameEvent} from '../third_party/index.js';
 import {
   type Frame,
   type Handler,
@@ -48,6 +48,8 @@ export type ListenerMap<EventMap extends PageEvents = PageEvents> = {
 export class PageCollector<T> {
   protected pptrPage: Page;
   #listeners?: ListenerMap<PageEvents>;
+  #pendingSameDocumentNavigation = false;
+  #frameNavigatedWithinDocumentTarget?: Frame;
   protected maxNavigationSaved = 3;
 
   /**
@@ -63,6 +65,7 @@ export class PageCollector<T> {
     maxResourcesPerNavigation?: number,
   ) {
     this.pptrPage = page;
+    this.#attachFrameNavigatedWithinDocumentListener();
 
     const idGenerator = createIdGenerator();
 
@@ -86,6 +89,18 @@ export class PageCollector<T> {
       if (frame !== this.pptrPage.mainFrame()) {
         return;
       }
+      // Re-attach the listener to the current main frame: a cross-document
+      // navigation may have replaced the frame object, so the reference
+      // captured at construction time would be stale.
+      this.#attachFrameNavigatedWithinDocumentListener();
+      // Same-document (SPA) navigations also emit `framenavigated`, but they
+      // must not rotate the retained history. Puppeteer emits
+      // `FrameNavigatedWithinDocument` right before `FrameNavigated` for such
+      // navigations, so consume the flag here to skip the split.
+      if (this.#pendingSameDocumentNavigation) {
+        this.#pendingSameDocumentNavigation = false;
+        return;
+      }
       this.splitAfterNavigation();
     };
 
@@ -97,6 +112,7 @@ export class PageCollector<T> {
   }
 
   dispose() {
+    this.#detachFrameNavigatedWithinDocumentListener();
     if (this.#listeners) {
       for (const [name, listener] of Object.entries(this.#listeners)) {
         this.pptrPage.off(name, listener as Handler<unknown>);
@@ -104,6 +120,32 @@ export class PageCollector<T> {
     }
   }
 
+  // Puppeteer emits this right before the page-level navigation event for
+  // same-document (SPA) navigations, which lets `framenavigated` skip the
+  // history rotation for those navigations.
+  #onFrameNavigatedWithinDocument = () => {
+    this.#pendingSameDocumentNavigation = true;
+  };
+
+  #attachFrameNavigatedWithinDocumentListener() {
+    this.#detachFrameNavigatedWithinDocumentListener();
+    this.#frameNavigatedWithinDocumentTarget = this.pptrPage.mainFrame();
+    this.#frameNavigatedWithinDocumentTarget.on(
+      FrameEvent.FrameNavigatedWithinDocument,
+      this.#onFrameNavigatedWithinDocument,
+    );
+  }
+
+  #detachFrameNavigatedWithinDocumentListener() {
+    if (this.#frameNavigatedWithinDocumentTarget) {
+      this.#frameNavigatedWithinDocumentTarget.off(
+        FrameEvent.FrameNavigatedWithinDocument,
+        this.#onFrameNavigatedWithinDocument,
+      );
+      this.#frameNavigatedWithinDocumentTarget = undefined;
+    }
+  }
+
   protected splitAfterNavigation() {
     // Add the latest navigation first
     this.storage.unshift([]);
@@ -183,6 +225,8 @@ class PageEventSubscriber {
   #page: Page;
   #session: CDPSession;
   #targetId: string;
+  #pendingSameDocumentNavigation = false;
+  #frameNavigatedWithinDocumentTarget?: Frame;
 
   constructor(page: Page) {
     this.#page = page;
@@ -212,11 +256,13 @@ class PageEventSubscriber {
     this.#page.on('framenavigated', this.#onFrameNavigated);
     this.#page.on('issue', this.#onIssueAdded);
     this.#session.on('Runtime.exceptionThrown', this.#onExceptionThrown);
+    this.#attachFrameNavigatedWithinDocumentListener();
   }
 
   unsubscribe() {
     this.#seenKeys.clear();
     this.#seenIssues.clear();
+    this.#detachFrameNavigatedWithinDocumentListener();
     this.#page.off('framenavigated', this.#onFrameNavigated);
     this.#page.off('issue', this.#onIssueAdded);
     this.#session.off('Runtime.exceptionThrown', this.#onExceptionThrown);
@@ -251,11 +297,45 @@ class PageEventSubscriber {
     if (frame !== frame.page().mainFrame()) {
       return;
     }
+    // Re-attach the listener to the current main frame: a cross-document
+    // navigation may have replaced the frame object, so the reference
+    // captured at construction time would be stale.
+    this.#attachFrameNavigatedWithinDocumentListener();
+    if (this.#pendingSameDocumentNavigation) {
+      this.#pendingSameDocumentNavigation = false;
+      return;
+    }
     this.#seenKeys.clear();
     this.#seenIssues.clear();
     this.#resetIssueAggregator();
   };
 
+  // Puppeteer emits this right before the page-level navigation event for
+  // same-document (SPA) navigations, which lets `framenavigated` skip the
+  // issue aggregation reset for those navigations.
+  #onFrameNavigatedWithinDocument = () => {
+    this.#pendingSameDocumentNavigation = true;
+  };
+
+  #attachFrameNavigatedWithinDocumentListener() {
+    this.#detachFrameNavigatedWithinDocumentListener();
+    this.#frameNavigatedWithinDocumentTarget = this.#page.mainFrame();
+    this.#frameNavigatedWithinDocumentTarget.on(
+      FrameEvent.FrameNavigatedWithinDocument,
+      this.#onFrameNavigatedWithinDocument,
+    );
+  }
+
+  #detachFrameNavigatedWithinDocumentListener() {
+    if (this.#frameNavigatedWithinDocumentTarget) {
+      this.#frameNavigatedWithinDocumentTarget.off(
+        FrameEvent.FrameNavigatedWithinDocument,
+        this.#onFrameNavigatedWithinDocument,
+      );
+      this.#frameNavigatedWithinDocumentTarget = undefined;
+    }
+  }
+
   #onIssueAdded = (inspectorIssue: Issue) => {
     try {
       // @ts-expect-error The types are missmatched but they

src/third_party/index.ts

@@ -45,9 +45,11 @@ export {
 export {default as puppeteer} from 'puppeteer-core';
 export type * from 'puppeteer-core';
 export {PipeTransport} from 'puppeteer-core/internal/node/PipeTransport.js';
+export {CdpFrame} from 'puppeteer-core/internal/cdp/Frame.js';
 export {CdpPage} from 'puppeteer-core/internal/cdp/Page.js';
 export type {CdpWebWorker} from 'puppeteer-core/internal/cdp/WebWorker.js';
 export type {Realm} from 'puppeteer-core/internal/api/Realm.js';
+export {FrameEvent} from 'puppeteer-core/internal/api/Frame.js';
 export type {JSONSchema7, JSONSchema7Definition} from 'json-schema';
 export {Mutex} from 'puppeteer-core/internal/util/Mutex.js';
 export {

tests/collectors/PageCollector.test.ts

@@ -16,7 +16,7 @@ import {
   NetworkCollector,
   PageCollector,
 } from '../../src/collectors/PageCollector.js';
-import {DevTools} from '../../src/third_party/index.js';
+import {DevTools, FrameEvent} from '../../src/third_party/index.js';
 
 import {getMockRequest, getMockBrowser} from '../utils.js';
 
@@ -59,6 +59,56 @@ describe('PageCollector', () => {
     assert.equal(collector.getData().length, 0);
   });
 
+  it('does not clean up after same-document navigation', async () => {
+    const browser = getMockBrowser();
+    const page = (await browser.pages())[0];
+    const mainFrame = page.mainFrame();
+    const request = getMockRequest();
+    const collector = new PageCollector(page, collect => {
+      return {
+        request: req => {
+          collect(req);
+        },
+      } as ListenerMap;
+    });
+
+    page.emit('request', request);
+
+    assert.equal(collector.getData()[0], request);
+
+    // Simulate a same-document (SPA) navigation: Puppeteer emits
+    // `FrameNavigatedWithinDocument` right before `framenavigated`.
+    mainFrame.emit(FrameEvent.FrameNavigatedWithinDocument, undefined);
+    page.emit('framenavigated', mainFrame);
+
+    assert.equal(collector.getData()[0], request);
+  });
+
+  it('cleans up after a cross-document navigation following a same-document one', async () => {
+    const browser = getMockBrowser();
+    const page = (await browser.pages())[0];
+    const mainFrame = page.mainFrame();
+    const request = getMockRequest();
+    const collector = new PageCollector(page, collect => {
+      return {
+        request: req => {
+          collect(req);
+        },
+      } as ListenerMap;
+    });
+
+    page.emit('request', request);
+
+    // Same-document navigation: history is kept.
+    mainFrame.emit(FrameEvent.FrameNavigatedWithinDocument, undefined);
+    page.emit('framenavigated', mainFrame);
+    assert.equal(collector.getData()[0], request);
+
+    // A real cross-document navigation must still rotate the history.
+    page.emit('framenavigated', mainFrame);
+    assert.equal(collector.getData().length, 0);
+  });
+
   it('does not clean up after sub frame navigation', async () => {
     const browser = getMockBrowser();
     const page = (await browser.pages())[0];

tests/mocks.ts

@@ -28,7 +28,7 @@ import sinon from 'sinon';
 import {McpContext} from '../src/McpContext.js';
 import {McpPage} from '../src/McpPage.js';
 import {McpResponse} from '../src/McpResponse.js';
-import {CdpPage, DevTools} from '../src/third_party/index.js';
+import {CdpFrame, CdpPage, DevTools} from '../src/third_party/index.js';
 import type {Page} from '../src/third_party/index.js';
 
 export type MockMcpPage = sinon.SinonStubbedInstance<McpPage> & {
@@ -64,10 +64,18 @@ export function mockListener() {
       }
     },
     off(
-      _eventName: string | symbol | number,
-      _listener?: (data: unknown) => void,
+      eventName: string | symbol | number,
+      listener?: (data: unknown) => void,
     ) {
-      // no-op
+      const arr = listeners[eventName];
+      if (!arr) {
+        return;
+      }
+      if (!listener) {
+        delete listeners[eventName];
+        return;
+      }
+      listeners[eventName] = arr.filter(entry => entry !== listener);
     },
     emit(eventName: string | symbol | number, data?: unknown) {
       for (const listener of listeners[eventName] ?? []) {
@@ -84,8 +92,25 @@ export function createMockPuppeteerPage(): sinon.SinonStubbedInstance<Page> {
 
   // mainFrame() must return a stable object so tests can pass it back into
   // page.emit('framenavigated', mainFrame) and have it recognized as the
-  // same frame instance across calls.
-  page.mainFrame.returns({} as Frame);
+  // same frame instance across calls. It needs real on/off/emit so the
+  // PageCollector can subscribe to FrameNavigatedWithinDocument and tests
+  // can trigger it.
+  const mainFrameStub = sinon.createStubInstance(CdpFrame);
+  const mainFrameListener = mockListener();
+  mainFrameStub.on.callsFake((eventName, handler) => {
+    mainFrameListener.on(eventName, handler);
+    return mainFrameStub;
+  });
+  mainFrameStub.off.callsFake((eventName, handler) => {
+    mainFrameListener.off(eventName, handler);
+    return mainFrameStub;
+  });
+  mainFrameStub.emit.callsFake((eventName, data) => {
+    mainFrameListener.emit(eventName, data);
+    return true;
+  });
+  // SinonStubbedInstance<CdpFrame> is not assignable to Frame due to private fields.
+  page.mainFrame.returns(mainFrameStub as unknown as Frame);
 
   // _client() is a private internal Puppeteer API used by ConsoleCollector
   // in the McpPage constructor. Not on the CdpPage prototype, so added