fix(cli): do not extend injected script on same-document navigations (#5002)

Otherwise, the injected script has to be ready for reentrancy.

Author: dgozman

src/cli/injected/recorder.ts

@@ -372,7 +372,7 @@ export class Recorder {
     if (event.key === '@' && event.code === 'KeyL')
       return false;
     // Allow and ignore common used shortcut for pasting.
-    if (process.platform === 'darwin') {
+    if (navigator.platform.includes('Mac')) {
       if (event.key === 'v' && event.metaKey)
         return false;
     } else {

src/server/browserContext.ts

@@ -382,7 +382,7 @@ export abstract class BrowserContext extends EventEmitter {
   async extendInjectedScript(source: string, arg?: any) {
     const installInFrame = (frame: frames.Frame) => frame.extendInjectedScript(source, arg).catch(e => {});
     const installInPage = (page: Page) => {
-      page.on(Page.Events.FrameNavigated, installInFrame);
+      page.on(Page.Events.InternalFrameNavigatedToNewDocument, installInFrame);
       return Promise.all(page.frames().map(installInFrame));
     };
     this.on(BrowserContext.Events.Page, installInPage);

src/server/frames.ts

@@ -199,7 +199,7 @@ export class FrameManager {
     frame.emit(Frame.Events.Navigation, navigationEvent);
     if (!initial) {
       debugLogger.log('api', `  navigated to "${url}"`);
-      this._page.frameNavigated(frame);
+      this._page.frameNavigatedToNewDocument(frame);
     }
     // Restore pending if any - see comments above about keepPending.
     frame._pendingDocument = keepPending;
@@ -213,7 +213,6 @@ export class FrameManager {
     const navigationEvent: NavigationEvent = { url, name: frame._name };
     frame.emit(Frame.Events.Navigation, navigationEvent);
     debugLogger.log('api', `  navigated to "${url}"`);
-    this._page.frameNavigated(frame);
   }
 
   frameAbortedNavigation(frameId: string, errorText: string, documentId?: string) {

src/server/page.ts

@@ -110,7 +110,7 @@ export class Page extends EventEmitter {
     RequestFinished: 'requestfinished',
     FrameAttached: 'frameattached',
     FrameDetached: 'framedetached',
-    FrameNavigated: 'framenavigated',
+    InternalFrameNavigatedToNewDocument: 'internalframenavigatedtonewdocument',
     Load: 'load',
     Popup: 'popup',
     WebSocket: 'websocket',
@@ -457,8 +457,8 @@ export class Page extends EventEmitter {
     this.emit(Page.Events.VideoStarted, video);
   }
 
-  frameNavigated(frame: frames.Frame) {
-    this.emit(Page.Events.FrameNavigated, frame);
+  frameNavigatedToNewDocument(frame: frames.Frame) {
+    this.emit(Page.Events.InternalFrameNavigatedToNewDocument, frame);
     const url = frame.url();
     if (!url.startsWith('http'))
       return;

test/browsercontext-expose-function.spec.ts

@@ -82,3 +82,26 @@ it('exposeBindingHandle should work', async ({context}) => {
   expect(await target.evaluate(x => x.foo)).toBe(42);
   expect(result).toEqual(17);
 });
+
+it('extendInjectedScript should work', async ({ context, server }) => {
+  await (context as any)._extendInjectedScript(`var pwExport = (() => {
+    class Foo {
+      constructor() {
+        window._counter = (window._counter || 0) + 1;
+      }
+    }
+    return Foo;
+  })()`);
+
+  const page = await context.newPage();
+  await page.waitForFunction(() => (window as any)._counter === 1);
+
+  await page.goto(server.EMPTY_PAGE);
+  await page.waitForFunction(() => (window as any)._counter === 1);
+
+  await Promise.all([
+    page.waitForNavigation(),
+    page.evaluate(() => history.pushState({}, '', '/url.html'))
+  ]);
+  expect(await page.evaluate(() => (window as any)._counter)).toBe(1);
+});

test/cli/cli-codegen.spec.ts

@@ -40,6 +40,34 @@ describe('cli codegen', (test, { browserName, headful }) => {
     expect(message.text()).toBe('click');
   });
 
+  it('should click after same-document navigation', async ({ page, recorder, httpServer }) => {
+    httpServer.setHandler((req: http.IncomingMessage, res: http.ServerResponse) => {
+      res.setHeader('Content-Type', 'text/html; charset=utf-8');
+      res.end('');
+    });
+    await recorder.setContentAndWait(`<button onclick="console.log('click')">Submit</button>`, httpServer.PREFIX + '/foo.html');
+    await Promise.all([
+      page.waitForNavigation(),
+      page.evaluate(() => history.pushState({}, '', '/url.html')),
+    ]);
+    // This is the only way to give recorder a chance to install
+    // the second unnecessary copy of the recorder script.
+    await page.waitForTimeout(1000);
+
+    const selector = await recorder.hoverOverElement('button');
+    expect(selector).toBe('text="Submit"');
+
+    const [message] = await Promise.all([
+      page.waitForEvent('console'),
+      recorder.waitForOutput('click'),
+      page.dispatchEvent('button', 'click', { detail: 1 })
+    ]);
+    expect(recorder.output()).toContain(`
+  // Click text="Submit"
+  await page.click('text="Submit"');`);
+    expect(message.text()).toBe('click');
+  });
+
   it('should not target selector preview by text regexp', async ({ page, recorder }) => {
     await recorder.setContentAndWait(`<span>dummy</span>`);