mirror of
https://github.com/microsoft/playwright.git
synced 2025-06-26 21:40:17 +00:00
fix(page): "load" event should fire before "waitForLoadState" resolves (#14897)
Currently, `loadstate` and `load` are two separate events in the protocol, and are fired in this order. As a result, `waitForLoadState()` sometimes resolves before the `'load'` event is fired, which is unexpected. Also fixes a flaky test that assumed `load` event comes after `domcontentloaded` for the empty page, which is not always a case in Chromium.
This commit is contained in:
parent
79378dd1eb
commit
cdb862767f
@ -75,6 +75,10 @@ export class Frame extends ChannelOwner<channels.FrameChannel> implements api.Fr
|
||||
}
|
||||
if (event.remove)
|
||||
this._loadStates.delete(event.remove);
|
||||
if (!this._parentFrame && event.add === 'load' && this._page)
|
||||
this._page.emit(Events.Page.Load, this._page);
|
||||
if (!this._parentFrame && event.add === 'domcontentloaded' && this._page)
|
||||
this._page.emit(Events.Page.DOMContentLoaded, this._page);
|
||||
});
|
||||
this._channel.on('navigated', event => {
|
||||
this._url = event.url;
|
||||
|
||||
@ -137,7 +137,6 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
|
||||
dialog.dismiss().catch(() => {});
|
||||
}
|
||||
});
|
||||
this._channel.on('domcontentloaded', () => this.emit(Events.Page.DOMContentLoaded, this));
|
||||
this._channel.on('download', ({ url, suggestedFilename, artifact }) => {
|
||||
const artifactObject = Artifact.from(artifact);
|
||||
this.emit(Events.Page.Download, new Download(this, url, suggestedFilename, artifactObject));
|
||||
@ -145,7 +144,6 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
|
||||
this._channel.on('fileChooser', ({ element, isMultiple }) => this.emit(Events.Page.FileChooser, new FileChooser(this, ElementHandle.from(element), isMultiple)));
|
||||
this._channel.on('frameAttached', ({ frame }) => this._onFrameAttached(Frame.from(frame)));
|
||||
this._channel.on('frameDetached', ({ frame }) => this._onFrameDetached(Frame.from(frame)));
|
||||
this._channel.on('load', () => this.emit(Events.Page.Load, this));
|
||||
this._channel.on('pageError', ({ error }) => this.emit(Events.Page.PageError, parseError(error)));
|
||||
this._channel.on('route', ({ route, request }) => this._onRoute(Route.from(route), Request.from(request)));
|
||||
this._channel.on('video', ({ artifact }) => {
|
||||
|
||||
@ -1331,11 +1331,9 @@ export interface PageEventTarget {
|
||||
on(event: 'crash', callback: (params: PageCrashEvent) => void): this;
|
||||
on(event: 'dialog', callback: (params: PageDialogEvent) => void): this;
|
||||
on(event: 'download', callback: (params: PageDownloadEvent) => void): this;
|
||||
on(event: 'domcontentloaded', callback: (params: PageDomcontentloadedEvent) => void): this;
|
||||
on(event: 'fileChooser', callback: (params: PageFileChooserEvent) => void): this;
|
||||
on(event: 'frameAttached', callback: (params: PageFrameAttachedEvent) => void): this;
|
||||
on(event: 'frameDetached', callback: (params: PageFrameDetachedEvent) => void): this;
|
||||
on(event: 'load', callback: (params: PageLoadEvent) => void): this;
|
||||
on(event: 'pageError', callback: (params: PagePageErrorEvent) => void): this;
|
||||
on(event: 'route', callback: (params: PageRouteEvent) => void): this;
|
||||
on(event: 'video', callback: (params: PageVideoEvent) => void): this;
|
||||
@ -1396,7 +1394,6 @@ export type PageDownloadEvent = {
|
||||
suggestedFilename: string,
|
||||
artifact: ArtifactChannel,
|
||||
};
|
||||
export type PageDomcontentloadedEvent = {};
|
||||
export type PageFileChooserEvent = {
|
||||
element: ElementHandleChannel,
|
||||
isMultiple: boolean,
|
||||
@ -1407,7 +1404,6 @@ export type PageFrameAttachedEvent = {
|
||||
export type PageFrameDetachedEvent = {
|
||||
frame: FrameChannel,
|
||||
};
|
||||
export type PageLoadEvent = {};
|
||||
export type PagePageErrorEvent = {
|
||||
error: SerializedError,
|
||||
};
|
||||
@ -1836,11 +1832,9 @@ export interface PageEvents {
|
||||
'crash': PageCrashEvent;
|
||||
'dialog': PageDialogEvent;
|
||||
'download': PageDownloadEvent;
|
||||
'domcontentloaded': PageDomcontentloadedEvent;
|
||||
'fileChooser': PageFileChooserEvent;
|
||||
'frameAttached': PageFrameAttachedEvent;
|
||||
'frameDetached': PageFrameDetachedEvent;
|
||||
'load': PageLoadEvent;
|
||||
'pageError': PagePageErrorEvent;
|
||||
'route': PageRouteEvent;
|
||||
'video': PageVideoEvent;
|
||||
|
||||
@ -1318,8 +1318,6 @@ Page:
|
||||
suggestedFilename: string
|
||||
artifact: Artifact
|
||||
|
||||
domcontentloaded:
|
||||
|
||||
fileChooser:
|
||||
parameters:
|
||||
element: ElementHandle
|
||||
@ -1333,8 +1331,6 @@ Page:
|
||||
parameters:
|
||||
frame: Frame
|
||||
|
||||
load:
|
||||
|
||||
pageError:
|
||||
parameters:
|
||||
error: SerializedError
|
||||
|
||||
@ -65,7 +65,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel> imple
|
||||
});
|
||||
page.on(Page.Events.Console, message => this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(this._scope, message) }));
|
||||
page.on(Page.Events.Crash, () => this._dispatchEvent('crash'));
|
||||
page.on(Page.Events.DOMContentLoaded, () => this._dispatchEvent('domcontentloaded'));
|
||||
page.on(Page.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this._scope, dialog) }));
|
||||
page.on(Page.Events.Download, (download: Download) => {
|
||||
this._dispatchEvent('download', { url: download.url, suggestedFilename: download.suggestedFilename(), artifact: new ArtifactDispatcher(scope, download.artifact) });
|
||||
@ -76,7 +75,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel> imple
|
||||
}));
|
||||
page.on(Page.Events.FrameAttached, frame => this._onFrameAttached(frame));
|
||||
page.on(Page.Events.FrameDetached, frame => this._onFrameDetached(frame));
|
||||
page.on(Page.Events.Load, () => this._dispatchEvent('load'));
|
||||
page.on(Page.Events.PageError, error => this._dispatchEvent('pageError', { error: serializeError(error) }));
|
||||
page.on(Page.Events.WebSocket, webSocket => this._dispatchEvent('webSocket', { webSocket: new WebSocketDispatcher(this._scope, webSocket) }));
|
||||
page.on(Page.Events.Worker, worker => this._dispatchEvent('worker', { worker: new WorkerDispatcher(this._scope, worker) }));
|
||||
|
||||
@ -577,10 +577,6 @@ export class Frame extends SdkObject {
|
||||
this.emit(Frame.Events.AddLifecycle, event);
|
||||
if (this === mainFrame && this._url !== 'about:blank')
|
||||
debugLogger.log('api', ` "${event}" event fired`);
|
||||
if (this === mainFrame && event === 'load')
|
||||
this._page.emit(Page.Events.Load);
|
||||
if (this === mainFrame && event === 'domcontentloaded')
|
||||
this._page.emit(Page.Events.DOMContentLoaded);
|
||||
}
|
||||
}
|
||||
for (const event of this._subtreeLifecycleEvents) {
|
||||
|
||||
@ -19,7 +19,7 @@ import type { APIRequestEvent, APIRequestFinishedEvent } from '../fetch';
|
||||
import { APIRequestContext } from '../fetch';
|
||||
import { helper } from '../helper';
|
||||
import * as network from '../network';
|
||||
import { Page } from '../page';
|
||||
import type { Page } from '../page';
|
||||
import type * as har from './har';
|
||||
import { calculateSha1, monotonicTime } from '../../utils';
|
||||
import type { RegisteredListener } from '../../utils/eventsHelper';
|
||||
@ -28,6 +28,8 @@ import { mime } from '../../utilsBundle';
|
||||
import { ManualPromise } from '../../utils/manualPromise';
|
||||
import { getPlaywrightVersion } from '../../common/userAgent';
|
||||
import { urlMatches } from '../../common/netUtils';
|
||||
import { Frame } from '../frames';
|
||||
import type { LifecycleEvent } from '../types';
|
||||
|
||||
const FALLBACK_HTTP_VERSION = 'HTTP/1.1';
|
||||
|
||||
@ -93,8 +95,12 @@ export class HarTracer {
|
||||
private _ensurePageEntry(page: Page) {
|
||||
let pageEntry = this._pageEntries.get(page);
|
||||
if (!pageEntry) {
|
||||
page.on(Page.Events.DOMContentLoaded, () => this._onDOMContentLoaded(page));
|
||||
page.on(Page.Events.Load, () => this._onLoad(page));
|
||||
page.mainFrame().on(Frame.Events.AddLifecycle, (event: LifecycleEvent) => {
|
||||
if (event === 'load')
|
||||
this._onLoad(page);
|
||||
if (event === 'domcontentloaded')
|
||||
this._onDOMContentLoaded(page);
|
||||
});
|
||||
|
||||
pageEntry = {
|
||||
startedDateTime: new Date(),
|
||||
|
||||
@ -128,14 +128,12 @@ export class Page extends SdkObject {
|
||||
Dialog: 'dialog',
|
||||
Download: 'download',
|
||||
FileChooser: 'filechooser',
|
||||
DOMContentLoaded: 'domcontentloaded',
|
||||
// Can't use just 'error' due to node.js special treatment of error events.
|
||||
// @see https://nodejs.org/api/events.html#events_error_events
|
||||
PageError: 'pageerror',
|
||||
FrameAttached: 'frameattached',
|
||||
FrameDetached: 'framedetached',
|
||||
InternalFrameNavigatedToNewDocument: 'internalframenavigatedtonewdocument',
|
||||
Load: 'load',
|
||||
ScreencastFrame: 'screencastframe',
|
||||
Video: 'video',
|
||||
WebSocket: 'websocket',
|
||||
|
||||
@ -193,9 +193,9 @@ it('should work with waitForLoadState(load)', async ({ page, server }) => {
|
||||
await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
|
||||
await Promise.all([
|
||||
page.click('a').then(() => page.waitForLoadState('load')).then(() => messages.push('clickload')),
|
||||
page.waitForEvent('framenavigated').then(() => page.waitForLoadState('domcontentloaded')).then(() => messages.push('domcontentloaded')),
|
||||
page.waitForEvent('load').then(() => messages.push('load')),
|
||||
]);
|
||||
expect(messages.join('|')).toBe('route|domcontentloaded|clickload');
|
||||
expect(messages.join('|')).toBe('route|load|clickload');
|
||||
});
|
||||
|
||||
it('should work with goto following click', async ({ page, server }) => {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user