diff --git a/packages/playwright-core/src/client/harRouter.ts b/packages/playwright-core/src/client/harRouter.ts index 1add67028f..99250c84e4 100644 --- a/packages/playwright-core/src/client/harRouter.ts +++ b/packages/playwright-core/src/client/harRouter.ts @@ -56,7 +56,7 @@ export class HarRouter { if (response.action === 'redirect') { debugLogger.log('api', `HAR: ${route.request().url()} redirected to ${response.redirectURL}`); - await route._abort(undefined, response.redirectURL); + await route._redirectNavigationRequest(response.redirectURL!); return; } diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index 9489dc6ef2..5c97d47cea 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -282,12 +282,14 @@ export class Route extends ChannelOwner implements api.Ro } async abort(errorCode?: string) { - await this._abort(errorCode); + this._checkNotHandled(); + await this._raceWithPageClose(this._channel.abort({ errorCode })); + this._reportHandled(true); } - async _abort(errorCode?: string, redirectAbortedNavigationToUrl?: string) { + async _redirectNavigationRequest(url: string) { this._checkNotHandled(); - await this._raceWithPageClose(this._channel.abort({ errorCode, redirectAbortedNavigationToUrl })); + await this._raceWithPageClose(this._channel.redirectNavigationRequest({ url })); this._reportHandled(true); } diff --git a/packages/playwright-core/src/protocol/channels.ts b/packages/playwright-core/src/protocol/channels.ts index 263a724a2e..25308e57d4 100644 --- a/packages/playwright-core/src/protocol/channels.ts +++ b/packages/playwright-core/src/protocol/channels.ts @@ -3158,17 +3158,23 @@ export interface RouteEventTarget { } export interface RouteChannel extends RouteEventTarget, Channel { _type_Route: boolean; + redirectNavigationRequest(params: RouteRedirectNavigationRequestParams, metadata?: Metadata): Promise; abort(params: RouteAbortParams, metadata?: Metadata): Promise; continue(params: RouteContinueParams, metadata?: Metadata): Promise; fulfill(params: RouteFulfillParams, metadata?: Metadata): Promise; } +export type RouteRedirectNavigationRequestParams = { + url: string, +}; +export type RouteRedirectNavigationRequestOptions = { + +}; +export type RouteRedirectNavigationRequestResult = void; export type RouteAbortParams = { errorCode?: string, - redirectAbortedNavigationToUrl?: string, }; export type RouteAbortOptions = { errorCode?: string, - redirectAbortedNavigationToUrl?: string, }; export type RouteAbortResult = void; export type RouteContinueParams = { diff --git a/packages/playwright-core/src/protocol/protocol.yml b/packages/playwright-core/src/protocol/protocol.yml index 8f72916c1e..0077065d2a 100644 --- a/packages/playwright-core/src/protocol/protocol.yml +++ b/packages/playwright-core/src/protocol/protocol.yml @@ -2491,10 +2491,13 @@ Route: commands: + redirectNavigationRequest: + parameters: + url: string + abort: parameters: errorCode: string? - redirectAbortedNavigationToUrl: string? continue: parameters: diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index fe98041a66..be0aa90502 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -1181,9 +1181,11 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { }); scheme.RequestResponseParams = tOptional(tObject({})); scheme.RequestRawRequestHeadersParams = tOptional(tObject({})); + scheme.RouteRedirectNavigationRequestParams = tObject({ + url: tString, + }); scheme.RouteAbortParams = tObject({ errorCode: tOptional(tString), - redirectAbortedNavigationToUrl: tOptional(tString), }); scheme.RouteContinueParams = tObject({ url: tOptional(tString), diff --git a/packages/playwright-core/src/server/dispatchers/networkDispatchers.ts b/packages/playwright-core/src/server/dispatchers/networkDispatchers.ts index 046ccff8f3..715a7bfe68 100644 --- a/packages/playwright-core/src/server/dispatchers/networkDispatchers.ts +++ b/packages/playwright-core/src/server/dispatchers/networkDispatchers.ts @@ -135,7 +135,11 @@ export class RouteDispatcher extends Dispatcher im } async abort(params: channels.RouteAbortParams): Promise { - await this._object.abort(params.errorCode || 'failed', params.redirectAbortedNavigationToUrl); + await this._object.abort(params.errorCode || 'failed'); + } + + async redirectNavigationRequest(params: channels.RouteRedirectNavigationRequestParams): Promise { + await this._object.redirectNavigationRequest(params.url); } } diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index dc37709e55..e321053d0e 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -269,7 +269,7 @@ export class FrameManager { name: frame._name, newDocument: frame.pendingDocument(), error: new NavigationAbortedError(documentId, errorText), - isPublic: !frame._pendingNavigationRedirectAfterAbort + isPublic: !(documentId && frame._redirectedNavigations.has(documentId)), }; frame.setPendingDocument(undefined); frame.emit(Frame.Events.InternalNavigation, navigationEvent); @@ -467,7 +467,7 @@ export class Frame extends SdkObject { readonly _detachedPromise: Promise; private _detachedCallback = () => {}; private _raceAgainstEvaluationStallingEventsPromises = new Set>(); - _pendingNavigationRedirectAfterAbort: { url: string, documentId: string } | undefined; + readonly _redirectedNavigations = new Map }>(); // documentId -> data constructor(page: Page, id: string, parentFrame: Frame | null) { super(page, 'frame'); @@ -604,12 +604,11 @@ export class Frame extends SdkObject { this._page._crashedPromise.then(() => { throw new Error('Navigation failed because page crashed!'); }), this._detachedPromise.then(() => { throw new Error('Navigating frame was detached!'); }), action().catch(e => { - if (this._pendingNavigationRedirectAfterAbort && e instanceof NavigationAbortedError) { - const { url, documentId } = this._pendingNavigationRedirectAfterAbort; - this._pendingNavigationRedirectAfterAbort = undefined; - if (e.documentId === documentId) { - progress.log(`redirecting navigation to "${url}"`); - return this._gotoAction(progress, url, options); + if (e instanceof NavigationAbortedError && e.documentId) { + const data = this._redirectedNavigations.get(e.documentId); + if (data) { + progress.log(`waiting for redirected navigation to "${data.url}"`); + return data.gotoPromise; } } throw e; @@ -617,8 +616,14 @@ export class Frame extends SdkObject { ]); } - redirectNavigationAfterAbort(url: string, documentId: string) { - this._pendingNavigationRedirectAfterAbort = { url, documentId }; + redirectNavigation(url: string, documentId: string, referer: string | undefined) { + const controller = new ProgressController(serverSideCallMetadata(), this); + const data = { + url, + gotoPromise: controller.run(progress => this._gotoAction(progress, url, { referer }), 0), + }; + this._redirectedNavigations.set(documentId, data); + data.gotoPromise.finally(() => this._redirectedNavigations.delete(documentId)); } async goto(metadata: CallMetadata, url: string, options: types.GotoOptions = {}): Promise { @@ -659,7 +664,7 @@ export class Frame extends SdkObject { if (event.newDocument!.documentId !== navigateResult.newDocumentId) { // This is just a sanity check. In practice, new navigation should // cancel the previous one and report "request cancelled"-like error. - throw new Error('Navigation interrupted by another one'); + throw new NavigationAbortedError(navigateResult.newDocumentId, 'Navigation interrupted by another one'); } if (event.error) throw event.error; diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index 61c0259e99..c5edcd15d9 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -244,13 +244,17 @@ export class Route extends SdkObject { return this._request; } - async abort(errorCode: string = 'failed', redirectAbortedNavigationToUrl?: string) { + async abort(errorCode: string = 'failed') { this._startHandling(); - if (redirectAbortedNavigationToUrl) - this._request.frame().redirectNavigationAfterAbort(redirectAbortedNavigationToUrl, this._request._documentId!); await this._delegate.abort(errorCode); } + async redirectNavigationRequest(url: string) { + this._startHandling(); + assert(this._request.isNavigationRequest()); + this._request.frame().redirectNavigation(url, this._request._documentId!, this._request.headerValue('referer')); + } + async fulfill(overrides: channels.RouteFulfillParams) { this._startHandling(); let body = overrides.body; diff --git a/tests/library/browsercontext-har.spec.ts b/tests/library/browsercontext-har.spec.ts index 070d2a62f8..f4c1ea93ca 100644 --- a/tests/library/browsercontext-har.spec.ts +++ b/tests/library/browsercontext-har.spec.ts @@ -115,6 +115,7 @@ it('should change document URL after redirected navigation', async ({ contextFac const page = await context.newPage(); const [response] = await Promise.all([ page.waitForNavigation(), + page.waitForURL('https://www.theverge.com/'), page.goto('https://theverge.com/') ]); await expect(page).toHaveURL('https://www.theverge.com/'); @@ -122,6 +123,23 @@ it('should change document URL after redirected navigation', async ({ contextFac expect(await page.evaluate(() => location.href)).toBe('https://www.theverge.com/'); }); +it('should change document URL after redirected navigation on click', async ({ server, contextFactory, isAndroid, asset }) => { + it.fixme(isAndroid); + + const path = asset('har-redirect.har'); + const context = await contextFactory({ har: { path, urlFilter: /.*theverge.*/ } }); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + await page.setContent(`click me`); + const [response] = await Promise.all([ + page.waitForNavigation(), + page.click('text=click me'), + ]); + await expect(page).toHaveURL('https://www.theverge.com/'); + expect(response.request().url()).toBe('https://www.theverge.com/'); + expect(await page.evaluate(() => location.href)).toBe('https://www.theverge.com/'); +}); + it('should goBack to redirected navigation', async ({ contextFactory, isAndroid, asset, server }) => { it.fixme(isAndroid);