Skip to content

Commit 05c0b3d

Browse files
pavelfeldmanpavelfeldman
committed
feat(click): wait for navigation commit upon input and evaluate
1 parent 8aa88d5 commit 05c0b3d

File tree

5 files changed

+174
-29
lines changed

5 files changed

+174
-29
lines changed

‎src/chromium/crPage.ts

+5
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export class CRPage implements PageDelegate {
8080
helper.addEventListener(this._client, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
8181
helper.addEventListener(this._client, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
8282
helper.addEventListener(this._client, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)),
83+
helper.addEventListener(this._client, 'Page.frameRequestedNavigation', event => this._onFrameRequestedNavigation(event)),
8384
helper.addEventListener(this._client, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
8485
helper.addEventListener(this._client, 'Page.javascriptDialogOpening', event => this._onDialog(event)),
8586
helper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event)),
@@ -172,6 +173,10 @@ export class CRPage implements PageDelegate {
172173
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
173174
}
174175

176+
_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {
177+
this._page._frameManager.frameRequestedNavigation(payload.frameId);
178+
}
179+
175180
async _ensureIsolatedWorld(name: string) {
176181
if (this._isolatedWorlds.has(name))
177182
return;

‎src/dom.ts

+40-23
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ export class FrameExecutionContext extends js.ExecutionContext {
6565
}));
6666
let result;
6767
try {
68-
result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
68+
result = await this.frame._page._frameManager.waitForNavigationsCreatedBy(async () => {
69+
return this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
70+
});
6971
} finally {
7072
await Promise.all(toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose())));
7173
}
@@ -248,12 +250,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
248250
const point = offset ? await this._offsetPoint(offset) : await this._clickablePoint();
249251
if (waitFor)
250252
await this._waitForHitTargetAt(point, options);
251-
let restoreModifiers: input.Modifier[] | undefined;
252-
if (options && options.modifiers)
253-
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
254-
await action(point);
255-
if (restoreModifiers)
256-
await this._page.keyboard._ensureModifiers(restoreModifiers);
253+
254+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
255+
let restoreModifiers: input.Modifier[] | undefined;
256+
if (options && options.modifiers)
257+
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
258+
await action(point);
259+
if (restoreModifiers)
260+
await this._page.keyboard._ensureModifiers(restoreModifiers);
261+
});
257262
}
258263

259264
hover(options?: PointerActionOptions & types.WaitForOptions): Promise<void> {
@@ -284,18 +289,22 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
284289
if (option.index !== undefined)
285290
assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
286291
}
287-
return this._evaluateInUtility((injected, node, ...optionsToSelect) => injected.selectOptions(node, optionsToSelect), ...options);
292+
return await this._page._frameManager.waitForNavigationsCreatedBy<string[]>(async () => {
293+
return this._evaluateInUtility((injected, node, ...optionsToSelect) => injected.selectOptions(node, optionsToSelect), ...options);
294+
});
288295
}
289296

290297
async fill(value: string): Promise<void> {
291298
assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
292-
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
293-
if (error)
294-
throw new Error(error);
295-
if (value)
296-
await this._page.keyboard.sendCharacters(value);
297-
else
298-
await this._page.keyboard.press('Delete');
299+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
300+
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
301+
if (error)
302+
throw new Error(error);
303+
if (value)
304+
await this._page.keyboard.sendCharacters(value);
305+
else
306+
await this._page.keyboard.press('Delete');
307+
});
299308
}
300309

301310
async setInputFiles(...files: (string | types.FilePayload)[]) {
@@ -317,7 +326,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
317326
}
318327
return item;
319328
}));
320-
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
329+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
330+
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
331+
});
321332
}
322333

323334
async focus() {
@@ -332,13 +343,17 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
332343
}
333344

334345
async type(text: string, options?: { delay?: number }) {
335-
await this.focus();
336-
await this._page.keyboard.type(text, options);
346+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
347+
await this.focus();
348+
await this._page.keyboard.type(text, options);
349+
});
337350
}
338351

339352
async press(key: string, options?: { delay?: number, text?: string }) {
340-
await this.focus();
341-
await this._page.keyboard.press(key, options);
353+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
354+
await this.focus();
355+
await this._page.keyboard.press(key, options);
356+
});
342357
}
343358

344359
async check(options?: types.WaitForOptions) {
@@ -352,9 +367,11 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
352367
private async _setChecked(state: boolean, options?: types.WaitForOptions) {
353368
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) === state)
354369
return;
355-
await this.click(options);
356-
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) !== state)
357-
throw new Error('Unable to click checkbox');
370+
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
371+
await this.click(options);
372+
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) !== state)
373+
throw new Error('Unable to click checkbox');
374+
});
358375
}
359376

360377
async boundingBox(): Promise<types.Rect | null> {

‎src/frames.ts

+31
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export class FrameManager {
5959
private _mainFrame: Frame;
6060
readonly _lifecycleWatchers = new Set<() => void>();
6161
readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
62+
private _navigationRequestCollectors = new Set<Set<string>>();
6263

6364
constructor(page: Page) {
6465
this._page = page;
@@ -107,7 +108,33 @@ export class FrameManager {
107108
}
108109
}
109110

111+
async waitForNavigationsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
112+
const frameIds = new Set<string>();
113+
this._navigationRequestCollectors.add(frameIds);
114+
try {
115+
const result = await action();
116+
if (!frameIds.size)
117+
return result;
118+
const frames = Array.from(frameIds.values()).map(frameId => this._frames.get(frameId));
119+
await Promise.all(frames.map(frame => frame!.waitForNavigation({ waitUntil: []}))).catch(e => {});
120+
return result;
121+
} finally {
122+
this._navigationRequestCollectors.delete(frameIds);
123+
}
124+
}
125+
126+
frameRequestedNavigation(frameId: string) {
127+
for (const frameIds of this._navigationRequestCollectors)
128+
frameIds.add(frameId);
129+
}
130+
131+
_cancelFrameRequestedNavigation(frameId: string) {
132+
for (const frameIds of this._navigationRequestCollectors)
133+
frameIds.delete(frameId);
134+
}
135+
110136
frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) {
137+
this._cancelFrameRequestedNavigation(frameId);
111138
const frame = this._frames.get(frameId)!;
112139
for (const child of frame.childFrames())
113140
this._removeFramesRecursively(child);
@@ -122,9 +149,11 @@ export class FrameManager {
122149
}
123150

124151
frameCommittedSameDocumentNavigation(frameId: string, url: string) {
152+
this._cancelFrameRequestedNavigation(frameId);
125153
const frame = this._frames.get(frameId);
126154
if (!frame)
127155
return;
156+
this._cancelFrameRequestedNavigation(frameId);
128157
frame._url = url;
129158
for (const watcher of frame._sameDocumentNavigationWatchers)
130159
watcher();
@@ -138,6 +167,7 @@ export class FrameManager {
138167
}
139168

140169
frameStoppedLoading(frameId: string) {
170+
this._cancelFrameRequestedNavigation(frameId);
141171
const frame = this._frames.get(frameId);
142172
if (!frame)
143173
return;
@@ -223,6 +253,7 @@ export class FrameManager {
223253
}
224254

225255
private _removeFramesRecursively(frame: Frame) {
256+
this._cancelFrameRequestedNavigation(frame._id);
226257
for (const child of frame.childFrames())
227258
this._removeFramesRecursively(child);
228259
frame._onDetached();

‎src/webkit/wkPage.ts

+5
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ export class WKPage implements PageDelegate {
201201
helper.addEventListener(this._session, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)),
202202
helper.addEventListener(this._session, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
203203
helper.addEventListener(this._session, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
204+
helper.addEventListener(this._session, 'Page.frameScheduledNavigation', event => this._onFrameScheduledNavigation(event.frameId)),
204205
helper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
205206
helper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')),
206207
helper.addEventListener(this._session, 'Page.domContentEventFired', event => this._onLifecycleEvent(event.frameId, 'domcontentloaded')),
@@ -234,6 +235,10 @@ export class WKPage implements PageDelegate {
234235
await Promise.all(sessions.map(session => callback(session).catch(debugError)));
235236
}
236237

238+
private _onFrameScheduledNavigation(frameId: string) {
239+
this._page._frameManager.frameRequestedNavigation(frameId);
240+
}
241+
237242
private _onFrameStoppedLoading(frameId: string) {
238243
this._page._frameManager.frameStoppedLoading(frameId);
239244
}

‎test/navigation.spec.js

+93-6
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,93 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
800800
});
801801
});
802802

803+
describe('Page.automaticWaiting', () => {
804+
it.fail(FFOX)('clicking anchor should await navigation', async({page, server}) => {
805+
const messages = [];
806+
server.setRoute('/empty.html', async (req, res) => {
807+
messages.push('route');
808+
res.end('done');
809+
});
810+
811+
await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
812+
813+
await Promise.all([
814+
page.click('a').then(() => messages.push('click')),
815+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
816+
]);
817+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
818+
});
819+
it.fail(FFOX)('clicking anchor should await cross-process navigation', async({page, server}) => {
820+
const messages = [];
821+
server.setRoute('/empty.html', async (req, res) => {
822+
messages.push('route');
823+
res.end('done');
824+
});
825+
826+
await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
827+
828+
await Promise.all([
829+
page.click('a').then(() => messages.push('click')),
830+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
831+
]);
832+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
833+
});
834+
it.fail(CHROMIUM || FFOX)('should submit form-get that causes navigation', async({page, server}) => {
835+
const messages = [];
836+
server.setRoute('/empty.html?foo=bar', async (req, res) => {
837+
messages.push('route');
838+
res.end('done');
839+
});
840+
841+
await page.setContent(`
842+
<form action="${server.EMPTY_PAGE}" method="get">
843+
<input name="foo" value="bar">
844+
<input type="submit" value="Submit">
845+
</form>`);
846+
847+
await Promise.all([
848+
page.click('input[type=submit]').then(() => messages.push('click')),
849+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
850+
]);
851+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
852+
});
853+
it.fail(CHROMIUM || FFOX)('should submit form-post that causes navigation', async({page, server}) => {
854+
const messages = [];
855+
server.setRoute('/empty.html', async (req, res) => {
856+
messages.push('route');
857+
res.end('done');
858+
});
859+
860+
await page.setContent(`
861+
<form action="${server.EMPTY_PAGE}" method="post">
862+
<input name="foo" value="bar">
863+
<input type="submit" value="Submit">
864+
</form>`);
865+
866+
await Promise.all([
867+
page.click('input[type=submit]').then(() => messages.push('click')),
868+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
869+
]);
870+
expect(messages.join('|')).toBe('route|waitForNavigation|click');
871+
});
872+
it('should not throw when clicking on links which do not commit navigation', async({page, server, httpsServer}) => {
873+
await page.goto(server.EMPTY_PAGE);
874+
await page.setContent(`<a href='${httpsServer.EMPTY_PAGE}'>foobar</a>`);
875+
await page.click('a');
876+
});
877+
it('should not throw when clicking on download link', async({page, server, httpsServer}) => {
878+
await page.setContent(`<a href="${server.PREFIX}/wasm/table2.wasm" download=true>table2.wasm</a>`);
879+
await page.click('a');
880+
});
881+
it.fail(CHROMIUM || FFOX)('should not hang on window.stop', async({page, server, httpsServer}) => {
882+
server.setRoute('/empty.html', async (req, res) => {
883+
await page.evaluate('window.stop()');
884+
});
885+
await page.setContent(`<a href="${server.EMPTY_PAGE}">example.html</a>`);
886+
await page.click('a');
887+
});
888+
});
889+
803890
describe('Page.waitForLoadState', () => {
804891
it('should pick up ongoing navigation', async({page, server}) => {
805892
let response = null;
@@ -950,13 +1037,13 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
9501037

9511038
server.setRoute('/empty.html', () => {});
9521039
let error = null;
953-
const navigationPromise = frame.waitForNavigation().catch(e => error = e);
9541040
await Promise.all([
955-
server.waitForRequest('/empty.html'),
956-
frame.evaluate(() => window.location = '/empty.html')
957-
]);
958-
await page.$eval('iframe', frame => frame.remove());
959-
await navigationPromise;
1041+
frame.waitForNavigation().catch(e => error = e),
1042+
server.waitForRequest('/empty.html').then(() => {
1043+
page.$eval('iframe', frame => frame.remove());
1044+
}),
1045+
frame.evaluate(() => window.location = '/empty.html'),
1046+
]).catch(e => error = e);
9601047
expect(error.message).toContain('frame was detached');
9611048
});
9621049
});

0 commit comments

Comments
 (0)