fix(agent): clear frames when url context changes (#98)

ulixee · Jun 13, 2024 · 0eb95dc · 0eb95dc
1 parent bdca4b0
commit 0eb95dc
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 2 deletions.
diff --git a/agent/main/lib/FramesManager.ts b/agent/main/lib/FramesManager.ts
@@ -418,9 +418,14 @@ export default class FramesManager extends TypedEventEmitter<IFrameManagerEvents
     navigatedEvent: FrameNavigatedEvent,
   ): Promise<void> {
     await this.isReady;
+    const startUrl = this.main?.url;
     const frame = this.recordFrame(devtoolsSession, navigatedEvent.frame);
     // if main frame, clear out other frames
     if (!frame.parentId) {
+      if (startUrl !== navigatedEvent.frame.url) {
+        this.attachedFrameIds.clear();
+        this.attachedFrameIds.add(frame.id);
+      }
       this.clearChildFrames();
     }
     frame.onNavigated(navigatedEvent.frame, navigatedEvent);

diff --git a/agent/main/test/navigation.test.ts b/agent/main/test/navigation.test.ts
@@ -122,7 +122,7 @@ describe.each([
     await new Promise(resolve => setTimeout(resolve, 100));
 
     const resources = page.browserContext.resources.getForTab(page.tabId);
-    expect(resources.filter(x => !x.url.endsWith("favicon.ico"))).toHaveLength(20);
+    expect(resources.filter(x => !x.url.endsWith('favicon.ico'))).toHaveLength(20);
   });
 
   it('can goto a page multiple times', async () => {
@@ -177,6 +177,57 @@ describe.each([
     expect(text.value).toBe('Reloaded');
   });
 
+  it('can handle frames changing in the same context', async () => {
+    const { page } = await createAgent(enableMitm);
+    koaServer.get('/testBrowserTop', ctx => {
+      ctx.body = `<html>
+<body>
+    <h1>Main Page</h1>
+    <iframe id="testFrame" src="/testFrame"></iframe>
+    <br />
+    <img src="/doesntmatter.png"> <!-- without this element we get different error: Timeout waiting for navigation "JavascriptReady" -->
+</body>
+</html>`;
+    });
+    koaServer.get('/testFrame', ctx => {
+      ctx.body = `<html>
+<body>
+    <h2>This is the Frame Page 1</h2>
+    <a id='changeTopDocument' href='/any' target='_top'>Change whole page, not only the frame.</a>
+</body>
+</html>`;
+    });
+
+    await page.goto(`${koaServer.baseUrl}/testBrowserTop`);
+    await page.waitForLoad('PaintingStable');
+    expect(page.frames).toHaveLength(2);
+    await page.frames[1].waitForLoad({ loadStatus: LocationStatus.DomContentLoaded });
+    expect(page.frames[1].url).toBe(`${koaServer.baseUrl}/testFrame`);
+    await page.goto(`${koaServer.baseUrl}/testBrowserTop?test=1`);
+    await page.waitForLoad('PaintingStable');
+    expect(page.frames.map(x => [x.frameId, x.url])).toHaveLength(2);
+    await page.frames[1].waitForLoad({ loadStatus: LocationStatus.DomContentLoaded });
+    expect(page.frames[1].url).toBe(`${koaServer.baseUrl}/testFrame`);
+    const frameElem = await page.mainFrame.jsPath.getNodePointer([
+      'document',
+      ['querySelector', '#testFrame'],
+    ]);
+    expect(frameElem.nodePointer.type).toBe('HTMLIFrameElement');
+
+    for (const frame of page.frames) {
+      const frameNodePointerId = await frame.getFrameElementNodePointerId();
+      if (frameNodePointerId === frameElem.nodePointer.id) {
+        const top = await frame.jsPath.getNodePointer([
+          'document',
+          ['getElementById', 'changeTopDocument'],
+        ]);
+        expect(top.nodePointer.type).toBe('HTMLAnchorElement');
+        return;
+      }
+    }
+    throw new Error('Frame not found');
+  });
+
   it('can reload a page', async () => {
     const startingUrl = `${koaServer.baseUrl}/pagex`;
     const { page } = await createAgent(enableMitm);

diff --git a/plugins/default-browser-emulator/injected-scripts/_proxyUtils.ts b/plugins/default-browser-emulator/injected-scripts/_proxyUtils.ts
@@ -394,7 +394,7 @@ function defaultProxyApply<T, K extends keyof T>(
     }
   }
 
-  // Try to make clean error stacks for tenables, but don't crash if
+  // Try to make clean error stacks for thenables, but don't crash if
   // for some reason this doesn't work. Crashing here could have
   // a huge impact on other things.
   try {