fix(webkit): handle will/didCheckPolicyForNavigation (#11631)

packages/playwright-core/browsers.json

@@ -23,7 +23,7 @@
     },
     {
       "name": "webkit",
-      "revision": "1599",
+      "revision": "1604",
       "installByDefault": true,
       "revisionOverrides": {
         "mac10.14": "1446"

packages/playwright-core/src/server/webkit/protocol.d.ts

@@ -6404,16 +6404,26 @@ the top of the viewport and Y increases as it proceeds towards the bottom of the
       appearance: Appearance;
     }
     /**
-     * Fired when page tries to open a new window.
+     * Fired when page is about to check policy for newly triggered navigation.
      */
-    export type willRequestOpenWindowPayload = {
-      url: string;
+    export type willCheckNavigationPolicyPayload = {
+      /**
+       * Id of the frame.
+       */
+      frameId: Network.FrameId;
     }
     /**
-     * Fired after page did try to open a new window.
+     * Fired when page has received navigation policy decision.
      */
-    export type didRequestOpenWindowPayload = {
-      opened: boolean;
+    export type didCheckNavigationPolicyPayload = {
+      /**
+       * Id of the frame.
+       */
+      frameId: Network.FrameId;
+      /**
+       * True if the navigation will not continue in this frame.
+       */
+      cancel?: boolean;
     }
     /**
      * Fired when the page shows file chooser for it's <input type=file>.
@@ -8760,8 +8770,8 @@ the top of the viewport and Y increases as it proceeds towards the bottom of the
     "Page.frameClearedScheduledNavigation": Page.frameClearedScheduledNavigationPayload;
     "Page.navigatedWithinDocument": Page.navigatedWithinDocumentPayload;
     "Page.defaultAppearanceDidChange": Page.defaultAppearanceDidChangePayload;
-    "Page.willRequestOpenWindow": Page.willRequestOpenWindowPayload;
-    "Page.didRequestOpenWindow": Page.didRequestOpenWindowPayload;
+    "Page.willCheckNavigationPolicy": Page.willCheckNavigationPolicyPayload;
+    "Page.didCheckNavigationPolicy": Page.didCheckNavigationPolicyPayload;
     "Page.fileChooserOpened": Page.fileChooserOpenedPayload;
     "Playwright.pageProxyCreated": Playwright.pageProxyCreatedPayload;
     "Playwright.pageProxyDestroyed": Playwright.pageProxyDestroyedPayload;

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -363,6 +363,8 @@ export class WKPage implements PageDelegate {
       eventsHelper.addEventListener(this._session, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)),
       eventsHelper.addEventListener(this._session, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
       eventsHelper.addEventListener(this._session, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
+      eventsHelper.addEventListener(this._session, 'Page.willCheckNavigationPolicy', event => this._onWillCheckNavigationPolicy(event.frameId)),
+      eventsHelper.addEventListener(this._session, 'Page.didCheckNavigationPolicy', event => this._onDidCheckNavigationPolicy(event.frameId, event.cancel)),
       eventsHelper.addEventListener(this._session, 'Page.frameScheduledNavigation', event => this._onFrameScheduledNavigation(event.frameId)),
       eventsHelper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
       eventsHelper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')),
@@ -404,6 +406,31 @@ export class WKPage implements PageDelegate {
     await Promise.all(sessions.map(session => callback(session).catch(e => {})));
   }
 
+  private _onWillCheckNavigationPolicy(frameId: string) {
+    // It may happen that new policy check occurs while there is an ongoing
+    // provisional load, in this case it should be safe to ignore it as it will
+    // either:
+    // - end up canceled, e.g. ctrl+click opening link in new tab, having no effect
+    //   on this page
+    // - start new provisional load which we will miss in our signal trackers but
+    //   we certainly won't hang waiting for it to finish and there is high chance
+    //   that the current provisional page will commit navigation canceling the new
+    //   one.
+    if (this._provisionalPage)
+      return;
+    this._page._frameManager.frameRequestedNavigation(frameId);
+  }
+
+  private _onDidCheckNavigationPolicy(frameId: string, cancel?: boolean) {
+    if (!cancel)
+      return;
+    // This is a cross-process navigation that is canceled in the original page and continues in
+    // the provisional page. Bail out as we are tracking it.
+    if (this._provisionalPage)
+      return;
+    this._page._frameManager.frameAbortedNavigation(frameId, 'Navigation canceled by policy check');
+  }
+
   private _onFrameScheduledNavigation(frameId: string) {
     this._page._frameManager.frameRequestedNavigation(frameId);
   }

tests/browsercontext-page-event.spec.ts

@@ -158,8 +158,6 @@ it('should fire page lifecycle events', async function({ browser, server }) {
 });
 
 it('should work with Shift-clicking', async ({ browser, server, browserName }) => {
-  it.fixme(browserName === 'webkit', 'WebKit: Shift+Click does not open a new window.');
-
   const context = await browser.newContext();
   const page = await context.newPage();
   await page.goto(server.EMPTY_PAGE);
@@ -173,7 +171,6 @@ it('should work with Shift-clicking', async ({ browser, server, browserName }) =
 });
 
 it('should work with Ctrl-clicking', async ({ browser, server, isMac, browserName }) => {
-  it.fixme(browserName === 'webkit', 'Ctrl+Click does not open a new tab.');
   it.fixme(browserName === 'firefox', 'Reports an opener in this case.');
 
   const context = await browser.newContext();

tests/page/page-goto.spec.ts

@@ -405,7 +405,7 @@ it('should fail when replaced by another navigation', async ({ page, server, bro
   if (browserName === 'chromium')
     expect(error.message).toContain('net::ERR_ABORTED');
   else if (browserName === 'webkit')
-    expect(error.message).toContain('cancelled');
+    expect(error.message).toContain('Navigation interrupted by another one');
   else
     expect(error.message).toContain('NS_BINDING_ABORTED');
 });

tests/page/page-wait-for-navigation.spec.ts

@@ -255,7 +255,9 @@ it('should fail when frame detaches', async ({ page, server }) => {
   server.setRoute('/empty.html', () => {});
   const [error] = await Promise.all([
     frame.waitForNavigation().catch(e => e),
-    page.evaluate('var frame = document.querySelector("iframe"); frame.contentWindow.location.href = "/empty.html"; setTimeout(() => frame.remove())'),
+    page.$eval('iframe', frame => { frame.contentWindow.location.href = '/empty.html'; }),
+    // Make sure policy checks pass and navigation actually begins before removing the frame to avoid other errors
+    server.waitForRequest('/empty.html').then(() => page.$eval('iframe', frame => setTimeout(() => frame.remove(), 0)))
   ]);
   expect(error.message).toContain('waiting for navigation until "load"');
   expect(error.message).toContain('frame was detached');