From 84248f6e489856083810820c1b90a96226322613 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 27 Jan 2022 14:58:43 -0800 Subject: [PATCH] fix(webkit): handle will/didCheckPolicyForNavigation (#11631) --- packages/playwright-core/browsers.json | 2 +- .../src/server/webkit/protocol.d.ts | 26 ++++++++++++------ .../src/server/webkit/wkPage.ts | 27 +++++++++++++++++++ tests/browsercontext-page-event.spec.ts | 3 --- tests/page/page-goto.spec.ts | 2 +- tests/page/page-wait-for-navigation.spec.ts | 4 ++- 6 files changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/playwright-core/browsers.json b/packages/playwright-core/browsers.json index bde674d0f7..4447022816 100644 --- a/packages/playwright-core/browsers.json +++ b/packages/playwright-core/browsers.json @@ -23,7 +23,7 @@ }, { "name": "webkit", - "revision": "1599", + "revision": "1604", "installByDefault": true, "revisionOverrides": { "mac10.14": "1446" diff --git a/packages/playwright-core/src/server/webkit/protocol.d.ts b/packages/playwright-core/src/server/webkit/protocol.d.ts index 8ca718ddd2..caf0edbc7c 100644 --- a/packages/playwright-core/src/server/webkit/protocol.d.ts +++ b/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 . @@ -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; diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 23f873cc3a..242b685fe2 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/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); } diff --git a/tests/browsercontext-page-event.spec.ts b/tests/browsercontext-page-event.spec.ts index 18b17727cf..a603fc88a5 100644 --- a/tests/browsercontext-page-event.spec.ts +++ b/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(); diff --git a/tests/page/page-goto.spec.ts b/tests/page/page-goto.spec.ts index 76c8041cee..cc9cf337c2 100644 --- a/tests/page/page-goto.spec.ts +++ b/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'); }); diff --git a/tests/page/page-wait-for-navigation.spec.ts b/tests/page/page-wait-for-navigation.spec.ts index 91a5c1679a..8dad7dabea 100644 --- a/tests/page/page-wait-for-navigation.spec.ts +++ b/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');