Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: start listening for navigation events before navigation starts #32237

Merged
merged 1 commit into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions packages/playwright-core/src/server/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -659,18 +659,24 @@ export class Frame extends SdkObject {
}
url = helper.completeUserURL(url);

const sameDocument = helper.waitForEvent(progress, this, Frame.Events.InternalNavigation, (e: NavigationEvent) => !e.newDocument);
const navigateResult = await this._page._delegate.navigateFrame(this, url, referer);
const navigationEvents: NavigationEvent[] = [];
const collectNavigations = (arg: NavigationEvent) => navigationEvents.push(arg);
this.on(Frame.Events.InternalNavigation, collectNavigations);
const navigateResult = await this._page._delegate.navigateFrame(this, url, referer).finally(
() => this.off(Frame.Events.InternalNavigation, collectNavigations));

let event: NavigationEvent;
if (navigateResult.newDocumentId) {
sameDocument.dispose();
event = await helper.waitForEvent(progress, this, Frame.Events.InternalNavigation, (event: NavigationEvent) => {
const predicate = (event: NavigationEvent) => {
// We are interested either in this specific document, or any other document that
// did commit and replaced the expected document.
return event.newDocument && (event.newDocument.documentId === navigateResult.newDocumentId || !event.error);
}).promise;

};
const events = navigationEvents.filter(predicate);
if (events.length)
event = events[0];
else
event = await helper.waitForEvent(progress, this, Frame.Events.InternalNavigation, predicate).promise;
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.
Expand All @@ -679,7 +685,13 @@ export class Frame extends SdkObject {
if (event.error)
throw event.error;
} else {
event = await sameDocument.promise;
// Wait for same document navigation.
const predicate = (e: NavigationEvent) => !e.newDocument;
const events = navigationEvents.filter(predicate);
if (events.length)
event = events[0];
else
event = await helper.waitForEvent(progress, this, Frame.Events.InternalNavigation, predicate).promise;
}

if (!this._firedLifecycleEvents.has(waitUntil))
Expand Down
3 changes: 2 additions & 1 deletion tests/page/page-goto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,9 @@ it('should work with Cross-Origin-Opener-Policy after redirect', async ({ page,

it('should properly cancel Cross-Origin-Opener-Policy navigation', {
annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32107' },
}, async ({ page, server, browserName, isLinux }) => {
}, async ({ page, server, browserName, isLinux, headless }) => {
it.fixme(browserName === 'webkit' && isLinux, 'Started failing after https://commits.webkit.org/281488@main');
it.fixme(browserName === 'chromium' && headless, 'COOP navigation cancels the one that starts later');
server.setRoute('/empty.html', (req, res) => {
res.setHeader('Cross-Origin-Opener-Policy', 'same-origin');
res.end();
Expand Down
Loading