Skip to content

Commit

Permalink
feat(click): wait for navigation commit upon input and evaluate
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Mar 4, 2020
1 parent 8aa88d5 commit 117816e
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 29 deletions.
5 changes: 5 additions & 0 deletions src/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class CRPage implements PageDelegate {
helper.addEventListener(this._client, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
helper.addEventListener(this._client, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
helper.addEventListener(this._client, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)),
helper.addEventListener(this._client, 'Page.frameRequestedNavigation', event => this._onFrameRequestedNavigation(event)),
helper.addEventListener(this._client, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
helper.addEventListener(this._client, 'Page.javascriptDialogOpening', event => this._onDialog(event)),
helper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event)),
Expand Down Expand Up @@ -172,6 +173,10 @@ export class CRPage implements PageDelegate {
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
}

_onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {
this._page._frameManager.frameRequestedNavigation(payload.frameId);
}

async _ensureIsolatedWorld(name: string) {
if (this._isolatedWorlds.has(name))
return;
Expand Down
63 changes: 40 additions & 23 deletions src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ export class FrameExecutionContext extends js.ExecutionContext {
}));
let result;
try {
result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
result = await this.frame._page._frameManager.waitForNavigationsCreatedBy(async () => {
return this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
});
} finally {
await Promise.all(toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose())));
}
Expand Down Expand Up @@ -248,12 +250,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
const point = offset ? await this._offsetPoint(offset) : await this._clickablePoint();
if (waitFor)
await this._waitForHitTargetAt(point, options);
let restoreModifiers: input.Modifier[] | undefined;
if (options && options.modifiers)
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
await action(point);
if (restoreModifiers)
await this._page.keyboard._ensureModifiers(restoreModifiers);

await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
let restoreModifiers: input.Modifier[] | undefined;
if (options && options.modifiers)
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
await action(point);
if (restoreModifiers)
await this._page.keyboard._ensureModifiers(restoreModifiers);
});
}

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

async fill(value: string): Promise<void> {
assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
if (error)
throw new Error(error);
if (value)
await this._page.keyboard.sendCharacters(value);
else
await this._page.keyboard.press('Delete');
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
const error = await this._evaluateInUtility((injected, node, value) => injected.fill(node, value), value);
if (error)
throw new Error(error);
if (value)
await this._page.keyboard.sendCharacters(value);
else
await this._page.keyboard.press('Delete');
});
}

async setInputFiles(...files: (string | types.FilePayload)[]) {
Expand All @@ -317,7 +326,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
return item;
}));
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
});
}

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

async type(text: string, options?: { delay?: number }) {
await this.focus();
await this._page.keyboard.type(text, options);
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
await this.focus();
await this._page.keyboard.type(text, options);
});
}

async press(key: string, options?: { delay?: number, text?: string }) {
await this.focus();
await this._page.keyboard.press(key, options);
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
await this.focus();
await this._page.keyboard.press(key, options);
});
}

async check(options?: types.WaitForOptions) {
Expand All @@ -352,9 +367,11 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
private async _setChecked(state: boolean, options?: types.WaitForOptions) {
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) === state)
return;
await this.click(options);
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) !== state)
throw new Error('Unable to click checkbox');
await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
await this.click(options);
if (await this._evaluateInUtility((injected, node) => injected.isCheckboxChecked(node)) !== state)
throw new Error('Unable to click checkbox');
});
}

async boundingBox(): Promise<types.Rect | null> {
Expand Down
31 changes: 31 additions & 0 deletions src/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export class FrameManager {
private _mainFrame: Frame;
readonly _lifecycleWatchers = new Set<() => void>();
readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
private _navigationRequestCollectors = new Set<Set<string>>();

constructor(page: Page) {
this._page = page;
Expand Down Expand Up @@ -107,7 +108,33 @@ export class FrameManager {
}
}

async waitForNavigationsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
const frameIds = new Set<string>();
this._navigationRequestCollectors.add(frameIds);
try {
const result = await action();
if (!frameIds.size)
return result;
const frames = Array.from(frameIds.values()).map(frameId => this._frames.get(frameId));
await Promise.all(frames.map(frame => frame!.waitForNavigation({ waitUntil: []}))).catch(e => {});
return result;
} finally {
this._navigationRequestCollectors.delete(frameIds);
}
}

frameRequestedNavigation(frameId: string) {
for (const frameIds of this._navigationRequestCollectors)
frameIds.add(frameId);
}

_cancelFrameRequestedNavigation(frameId: string) {
for (const frameIds of this._navigationRequestCollectors)
frameIds.delete(frameId);
}

frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) {
this._cancelFrameRequestedNavigation(frameId);
const frame = this._frames.get(frameId)!;
for (const child of frame.childFrames())
this._removeFramesRecursively(child);
Expand All @@ -122,9 +149,11 @@ export class FrameManager {
}

frameCommittedSameDocumentNavigation(frameId: string, url: string) {
this._cancelFrameRequestedNavigation(frameId);
const frame = this._frames.get(frameId);
if (!frame)
return;
this._cancelFrameRequestedNavigation(frameId);
frame._url = url;
for (const watcher of frame._sameDocumentNavigationWatchers)
watcher();
Expand All @@ -138,6 +167,7 @@ export class FrameManager {
}

frameStoppedLoading(frameId: string) {
this._cancelFrameRequestedNavigation(frameId);
const frame = this._frames.get(frameId);
if (!frame)
return;
Expand Down Expand Up @@ -223,6 +253,7 @@ export class FrameManager {
}

private _removeFramesRecursively(frame: Frame) {
this._cancelFrameRequestedNavigation(frame._id);
for (const child of frame.childFrames())
this._removeFramesRecursively(child);
frame._onDetached();
Expand Down
80 changes: 74 additions & 6 deletions test/navigation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,74 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
});
});

describe.fail(true)('Page.automaticWaiting', () => {
it.fail(FFOX || WEBKIT)('clicking anchor should await navigation', async({page, server}) => {
const messages = [];
server.setRoute('/empty.html', async (req, res) => {
messages.push('route');
res.end('done');
});

await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);

await Promise.all([
page.click('a').then(() => messages.push('click')),
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
]);
expect(messages.join('|')).toBe('route|waitForNavigation|click');
});
it.fail(FFOX || WEBKIT)('clicking anchor should await cross-process navigation', async({page, server}) => {
const messages = [];
server.setRoute('/empty.html', async (req, res) => {
messages.push('route');
res.end('done');
});

await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);

await Promise.all([
page.click('a').then(() => messages.push('click')),
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
]);
expect(messages.join('|')).toBe('route|waitForNavigation|click');
});
it.fail(true)('should submit form that causes navigation', async({page, server}) => {
const messages = [];
server.setRoute('/empty.html', async (req, res) => {
messages.push('route');
res.end('done');
});

await page.setContent(`
<form action="${server.EMPTY_PAGE}" method="get">
<input name="foo" value="bar">
<input type="submit" value="Submit">
</form>`);

await Promise.all([
page.click('input[type=submit]').then(() => messages.push('click')),
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation'))
]);
expect(messages.join('|')).toBe('route|waitForNavigation|click');
});
it('should not throw when clicking on links which do not commit navigation', async({page, server, httpsServer}) => {
await page.goto(server.EMPTY_PAGE);
await page.setContent(`<a href='${httpsServer.EMPTY_PAGE}'>foobar</a>`);
await page.click('a');
});
it('should not throw when clicking on download link', async({page, server, httpsServer}) => {
await page.setContent(`<a href="${server.PREFIX}/wasm/table2.wasm" download=true>table2.wasm</a>`);
await page.click('a');
});
it.fail(true)('should not hang on window.stop', async({page, server, httpsServer}) => {
server.setRoute('/empty.html', async (req, res) => {});
await page.setContent(`<a href="${server.EMPTY_PAGE}">example.html</a>`);
const clickIt = page.click('a');
await page.evaluate('window.stop()');
await clickIt;
});
});

describe('Page.waitForLoadState', () => {
it('should pick up ongoing navigation', async({page, server}) => {
let response = null;
Expand Down Expand Up @@ -950,13 +1018,13 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF

server.setRoute('/empty.html', () => {});
let error = null;
const navigationPromise = frame.waitForNavigation().catch(e => error = e);
await Promise.all([
server.waitForRequest('/empty.html'),
frame.evaluate(() => window.location = '/empty.html')
]);
await page.$eval('iframe', frame => frame.remove());
await navigationPromise;
frame.waitForNavigation().catch(e => error = e),
server.waitForRequest('/empty.html').then(() => {
page.$eval('iframe', frame => frame.remove());
}),
frame.evaluate(() => window.location = '/empty.html'),
]).catch(e => error = e);
expect(error.message).toContain('frame was detached');
});
});
Expand Down

0 comments on commit 117816e

Please sign in to comment.