fix(navigation): waitForNavigation to pick up aborted navigation (#244)

src/chromium/FrameManager.ts

@@ -201,7 +201,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       return;
     if (event.name === 'init') {
       frame._firedLifecycleEvents.clear();
-      frame._onExpectedNewDocumentNavigation(event.loaderId);
+      frame._expectNewDocumentNavigation(event.loaderId);
     } else if (event.name === 'load') {
       frame._lifecycleEvent('load');
     } else if (event.name === 'DOMContentLoaded') {

src/chromium/NetworkManager.ts

@@ -197,8 +197,13 @@ export class NetworkManager extends EventEmitter {
         redirectChain = request.request._redirectChain;
       }
     }
+    // TODO: how can frame be null here?
     const frame = event.frameId ? this._frameManager.frame(event.frameId) : null;
-    const request = new InterceptableRequest(this._client, frame, interceptionId, this._userRequestInterceptionEnabled, event, redirectChain);
+    const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document';
+    const documentId = isNavigationRequest ? event.loaderId : undefined;
+    if (documentId && frame)
+      frame._expectNewDocumentNavigation(documentId, event.request.url);
+    const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectChain);
     this._requestIdToRequest.set(event.requestId, request);
     this.emit(NetworkManagerEvents.Request, request.request);
   }
@@ -259,6 +264,14 @@ export class NetworkManager extends EventEmitter {
       response._requestFinished();
     this._requestIdToRequest.delete(request._requestId);
     this._attemptedAuthentications.delete(request._interceptionId);
+    if (request._documentId && request.request.frame()) {
+      const isCurrentDocument = request.request.frame()._lastDocumentId === request._documentId;
+      let errorText = event.errorText;
+      if (event.canceled)
+        errorText += '; maybe frame was detached?';
+      if (!isCurrentDocument)
+        request.request.frame()._onAbortedNewDocumentNavigation(request._documentId, errorText);
+    }
     this.emit(NetworkManagerEvents.RequestFailed, request.request);
   }
 }
@@ -273,17 +286,19 @@ class InterceptableRequest {
   readonly request: network.Request;
   _requestId: string;
   _interceptionId: string;
+  _documentId: string;
   private _client: CDPSession;
   private _allowInterception: boolean;
   private _interceptionHandled = false;
 
-  constructor(client: CDPSession, frame: frames.Frame | null, interceptionId: string, allowInterception: boolean, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[]) {
+  constructor(client: CDPSession, frame: frames.Frame | null, interceptionId: string, documentId: string | undefined, allowInterception: boolean, event: Protocol.Network.requestWillBeSentPayload, redirectChain: network.Request[]) {
     this._client = client;
     this._requestId = event.requestId;
     this._interceptionId = interceptionId;
+    this._documentId = documentId;
     this._allowInterception = allowInterception;
 
-    this.request = new network.Request(frame, redirectChain, event.requestId === event.loaderId && event.type === 'Document',
+    this.request = new network.Request(frame, redirectChain, !!documentId,
         event.request.url, event.type.toLowerCase(), event.request.method, event.request.postData, headersObject(event.request.headers));
     (this.request as any)[interceptableRequestSymbol] = this;
   }

src/chromium/features/permissions.ts

@@ -40,8 +40,8 @@ export class Permissions {
       ['gyroscope', 'sensors'],
       ['magnetometer', 'sensors'],
       ['accessibility-events', 'accessibilityEvents'],
-      ['clipboard-read', 'clipboardRead'],
+      ['clipboard-read', 'clipboardReadWrite'],
-      ['clipboard-write', 'clipboardWrite'],
+      ['clipboard-write', 'clipboardSanitizedWrite'],
       ['payment-handler', 'paymentHandler'],
       // chrome-specific permissions we have.
       ['midi-sysex', 'midiSysex'],

src/firefox/FrameManager.ts

@@ -138,7 +138,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
 
   _onNavigationStarted(params) {
     const frame = this._frames.get(params.frameId);
-    frame._onExpectedNewDocumentNavigation(params.navigationId, params.url);
+    frame._expectNewDocumentNavigation(params.navigationId, params.url);
   }
 
   _onNavigationAborted(params) {

src/frames.ts

@@ -422,7 +422,7 @@ export class Frame {
     return context.evaluate(() => document.title);
   }
 
-  _onExpectedNewDocumentNavigation(documentId: string, url?: string) {
+  _expectNewDocumentNavigation(documentId: string, url?: string) {
     for (const watcher of this._page._lifecycleWatchers)
       watcher._onExpectedNewDocumentNavigation(this, documentId, url);
   }

src/webkit/FrameManager.ts

@@ -231,7 +231,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     // Append session id to avoid cross-process loaderId clash.
     const documentId = this._session._sessionId + '::' + framePayload.loaderId;
     if (!initial)
-      frame._onExpectedNewDocumentNavigation(documentId);
+      frame._expectNewDocumentNavigation(documentId);
     frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, documentId);
 
     this._page.emit(Events.Page.FrameNavigated, frame);

src/webkit/NetworkManager.ts

@@ -102,7 +102,7 @@ export class NetworkManager extends EventEmitter {
     const isNavigationRequest = event.type === 'Document';
     const documentId = isNavigationRequest ? this._session._sessionId + '::' + event.loaderId : undefined;
     if (documentId)
-      frame._onExpectedNewDocumentNavigation(documentId, event.request.url);
+      frame._expectNewDocumentNavigation(documentId, event.request.url);
     const request = new InterceptableRequest(frame, undefined, event, redirectChain, documentId);
     this._requestIdToRequest.set(event.requestId, request);
     this.emit(NetworkManagerEvents.Request, request.request);
@@ -166,11 +166,11 @@ export class NetworkManager extends EventEmitter {
     this._attemptedAuthentications.delete(request._interceptionId);
     if (request._documentId) {
       const isCurrentDocument = request.request.frame()._lastDocumentId === request._documentId;
-      // When frame was detached during load, "cancelled" comes before detach.
+      let errorText = event.errorText;
-      // Ignore it and hope for the best.
+      if (errorText.includes('cancelled'))
-      const wasCanceled = event.errorText.includes('cancelled');
+        errorText += '; maybe frame was detached?';
-      if (!isCurrentDocument && !wasCanceled)
+      if (!isCurrentDocument)
-        request.request.frame()._onAbortedNewDocumentNavigation(request._documentId, event.errorText);
+        request.request.frame()._onAbortedNewDocumentNavigation(request._documentId, errorText);
     }
     this.emit(NetworkManagerEvents.RequestFailed, request.request);
   }

test/navigation.spec.js

@@ -157,32 +157,14 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
       page.on('requestfailed', request => expect(request).toBeTruthy());
       let error = null;
       await page.goto(httpsServer.EMPTY_PAGE).catch(e => error = e);
-      if (CHROME) {
+      expectSSLError(error.message);
-        expect(error.message).toContain('net::ERR_CERT_AUTHORITY_INVALID');
-      } else if (WEBKIT) {
-        if (process.platform === 'darwin')
-          expect(error.message).toContain('The certificate for this server is invalid');
-        else
-          expect(error.message).toContain('Unacceptable TLS certificate');
-      } else {
-        expect(error.message).toContain('SSL_ERROR_UNKNOWN');
-      }
     });
     it('should fail when navigating to bad SSL after redirects', async({page, server, httpsServer}) => {
       server.setRedirect('/redirect/1.html', '/redirect/2.html');
       server.setRedirect('/redirect/2.html', '/empty.html');
       let error = null;
       await page.goto(httpsServer.PREFIX + '/redirect/1.html').catch(e => error = e);
-      if (CHROME) {
+      expectSSLError(error.message);
-        expect(error.message).toContain('net::ERR_CERT_AUTHORITY_INVALID');
-      } else if (WEBKIT) {
-        if (process.platform === 'darwin')
-          expect(error.message).toContain('The certificate for this server is invalid');
-        else
-          expect(error.message).toContain('Unacceptable TLS certificate');
-      } else {
-        expect(error.message).toContain('SSL_ERROR_UNKNOWN');
-      }
     });
     xit('should throw if networkidle is passed as an option', async({page, server}) => {
       let error = null;
@@ -443,6 +425,15 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
       expect(response).toBe(null);
       expect(page.url()).toBe(server.EMPTY_PAGE + '#foobar');
     });
+    it('should work with clicking on links which do not commit navigation', async({page, server, httpsServer}) => {
+      await page.goto(server.EMPTY_PAGE);
+      await page.setContent(`<a href='${httpsServer.EMPTY_PAGE}'>foobar</a>`);
+      const [error] = await Promise.all([
+        page.waitForNavigation().catch(e => e),
+        page.click('a'),
+      ]);
+      expectSSLError(error.message);
+    });
     it('should work with history.pushState()', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       await page.setContent(`
@@ -566,7 +557,7 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
 
       await page.$eval('iframe', frame => frame.remove());
       const error = await navigationPromise;
-      expect(error.message).toBe('Navigating frame was detached');
+      expect(error.message).toContain('frame was detached');
     });
     it('should return matching responses', async({page, server}) => {
       // Disable cache: otherwise, chromium will cache similar requests.
@@ -623,7 +614,7 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
       ]);
       await page.$eval('iframe', frame => frame.remove());
       await navigationPromise;
-      expect(error.message).toBe('Navigating frame was detached');
+      expect(error.message).toContain('frame was detached');
     });
   });
 
@@ -635,4 +626,17 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
       expect(await page.evaluate(() => window._foo)).toBe(undefined);
     });
   });
+
+  function expectSSLError(errorMessage) {
+    if (CHROME) {
+      expect(errorMessage).toContain('net::ERR_CERT_AUTHORITY_INVALID');
+    } else if (WEBKIT) {
+      if (process.platform === 'darwin')
+        expect(errorMessage).toContain('The certificate for this server is invalid');
+      else
+        expect(errorMessage).toContain('Unacceptable TLS certificate');
+    } else {
+      expect(errorMessage).toContain('SSL_ERROR_UNKNOWN');
+    }
+  }
 };