Skip to content

Commit

Permalink
Revert "feat(click): waitForInteractable option, defaults to true (#934…
Browse files Browse the repository at this point in the history 
…)" (#1013)

Reason: new tests are flaky on all bots.
  • Loading branch information
@dgozman
dgozman authored Feb 14, 2020
1 parent 9413351 commit dbb45d4
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 236 deletions.
40 changes: 7 additions & 33 deletions docs/api.md
View file

Large diffs are not rendered by default.

123 changes: 38 additions & 85 deletions src/dom.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import { Page } from './page';
import * as platform from './platform';
import { Selectors } from './selectors';

export type WaitForInteractableOptions = types.TimeoutOptions & { waitForInteractable?: boolean };

export class FrameExecutionContext extends js.ExecutionContext {
readonly frame: frames.Frame;

Expand Down Expand Up @@ -232,15 +230,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return point;
}

async _performPointerAction(action: (point: types.Point) => Promise<void>, options?: input.PointerActionOptions & WaitForInteractableOptions): Promise<void> {
const { waitForInteractable = true } = (options || {});
if (waitForInteractable)
await this._waitForStablePosition(options);
async _performPointerAction(action: (point: types.Point) => Promise<void>, options?: input.PointerActionOptions): Promise<void> {
const relativePoint = options ? options.relativePoint : undefined;
await this._scrollRectIntoViewIfNeeded(relativePoint ? { x: relativePoint.x, y: relativePoint.y, width: 0, height: 0 } : undefined);
const point = relativePoint ? await this._relativePoint(relativePoint) : await this._clickablePoint();
if (waitForInteractable)
await this._waitForHitTargetAt(point, options);
let restoreModifiers: input.Modifier[] | undefined;
if (options && options.modifiers)
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
Expand All @@ -249,19 +242,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
await this._page.keyboard._ensureModifiers(restoreModifiers);
}

hover(options?: input.PointerActionOptions & WaitForInteractableOptions): Promise<void> {
hover(options?: input.PointerActionOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.move(point.x, point.y), options);
}

click(options?: input.ClickOptions & WaitForInteractableOptions): Promise<void> {
click(options?: input.ClickOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.click(point.x, point.y, options), options);
}

dblclick(options?: input.MultiClickOptions & WaitForInteractableOptions): Promise<void> {
dblclick(options?: input.MultiClickOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.dblclick(point.x, point.y, options), options);
}

tripleclick(options?: input.MultiClickOptions & WaitForInteractableOptions): Promise<void> {
tripleclick(options?: input.MultiClickOptions): Promise<void> {
return this._performPointerAction(point => this._page.mouse.tripleclick(point.x, point.y, options), options);
}

Expand Down Expand Up @@ -409,20 +402,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
await this._page.keyboard.type(text, options);
}

async press(key: string, options?: { delay?: number, text?: string }) {
async press(key: string, options: { delay?: number; text?: string; } | undefined) {
await this.focus();
await this._page.keyboard.press(key, options);
}

async check(options?: WaitForInteractableOptions) {
await this._setChecked(true, options);
async check() {
await this._setChecked(true);
}

async uncheck(options?: WaitForInteractableOptions) {
await this._setChecked(false, options);
async uncheck() {
await this._setChecked(false);
}

private async _setChecked(state: boolean, options: WaitForInteractableOptions = {}) {
private async _setChecked(state: boolean) {
const isCheckboxChecked = async (): Promise<boolean> => {
return this._evaluateInUtility((node: Node) => {
if (node.nodeType !== Node.ELEMENT_NODE)
Expand Down Expand Up @@ -450,7 +442,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {

if (await isCheckboxChecked() === state)
return;
await this.click(options);
await this.click();
if (await isCheckboxChecked() !== state)
throw new Error('Unable to click checkbox');
}
Expand Down Expand Up @@ -505,52 +497,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return visibleRatio;
});
}

async _waitForStablePosition(options: types.TimeoutOptions = {}): Promise<void> {
const context = await this._context.frame._utilityContext();
const stablePromise = context.evaluate((injected: Injected, node: Node, timeout: number) => {
if (!node.isConnected)
throw new Error('Element is not attached to the DOM');
const element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
if (!element)
throw new Error('Element is not attached to the DOM');

let lastRect: types.Rect | undefined;
return injected.poll('raf', undefined, timeout, () => {
const clientRect = element.getBoundingClientRect();
const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height };
const isStable = lastRect && rect.x === lastRect.x && rect.y === lastRect.y && rect.width === lastRect.width && rect.height === lastRect.height;
lastRect = rect;
return isStable;
});
}, await context._injected(), this, options.timeout || 0);
await helper.waitWithTimeout(stablePromise, 'element to stop moving', options.timeout || 0);
}

async _waitForHitTargetAt(point: types.Point, options: types.TimeoutOptions = {}): Promise<void> {
const frame = await this.ownerFrame();
if (frame && frame.parentFrame()) {
const element = await frame.frameElement();
const box = await element.boundingBox();
if (!box)
throw new Error('Element is not attached to the DOM');
// Translate from viewport coordinates to frame coordinates.
point = { x: point.x - box.x, y: point.y - box.y };
}
const context = await this._context.frame._utilityContext();
const hitTargetPromise = context.evaluate((injected: Injected, node: Node, timeout: number, point: types.Point) => {
const element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
if (!element)
throw new Error('Element is not attached to the DOM');
return injected.poll('raf', undefined, timeout, () => {
let hitElement = injected.utils.deepElementFromPoint(document, point.x, point.y);
while (hitElement && hitElement !== element)
hitElement = injected.utils.parentElementOrShadowHost(hitElement);
return hitElement === element;
});
}, await context._injected(), this, options.timeout || 0, point);
await helper.waitWithTimeout(hitTargetPromise, 'element to receive mouse events', options.timeout || 0);
}
}

function normalizeSelector(selector: string): string {
Expand All @@ -568,44 +514,51 @@ function normalizeSelector(selector: string): string {

export type Task = (context: FrameExecutionContext) => Promise<js.JSHandle>;

function assertPolling(polling: types.Polling) {
export function waitForFunctionTask(selector: string | undefined, pageFunction: Function | string, options: types.WaitForFunctionOptions, ...args: any[]) {
const { polling = 'raf' } = options;
if (helper.isString(polling))
assert(polling === 'raf' || polling === 'mutation', 'Unknown polling option: ' + polling);
else if (helper.isNumber(polling))
assert(polling > 0, 'Cannot poll with non-positive interval: ' + polling);
else
throw new Error('Unknown polling options: ' + polling);
}

export function waitForFunctionTask(selector: string | undefined, pageFunction: Function | string, options: types.WaitForFunctionOptions, ...args: any[]): Task {
const { polling = 'raf' } = options;
assertPolling(polling);
const predicateBody = helper.isString(pageFunction) ? 'return (' + pageFunction + ')' : 'return (' + pageFunction + ')(...args)';
if (selector !== undefined)
selector = normalizeSelector(selector);

return async (context: FrameExecutionContext) => context.evaluateHandle((injected: Injected, selector: string | undefined, predicateBody: string, polling: types.Polling, timeout: number, ...args) => {
const innerPredicate = new Function('...args', predicateBody);
return injected.poll(polling, selector, timeout, (element: Element | undefined): any => {
if (polling === 'raf')
return injected.pollRaf(selector, predicate, timeout);
if (polling === 'mutation')
return injected.pollMutation(selector, predicate, timeout);
return injected.pollInterval(selector, polling, predicate, timeout);

function predicate(element: Element | undefined): any {
if (selector === undefined)
return innerPredicate(...args);
return innerPredicate(element, ...args);
});
}
}, await context._injected(), selector, predicateBody, polling, options.timeout || 0, ...args);
}

export function waitForSelectorTask(selector: string, visibility: types.Visibility, timeout: number): Task {
selector = normalizeSelector(selector);
return async (context: FrameExecutionContext) => context.evaluateHandle((injected: Injected, selector: string, visibility: types.Visibility, timeout: number) => {
const polling = visibility === 'any' ? 'mutation' : 'raf';
return injected.poll(polling, selector, timeout, (element: Element | undefined): Element | boolean => {
if (!element)
return visibility === 'hidden';
if (visibility === 'any')
return element;
return injected.isVisible(element) === (visibility === 'visible') ? element : false;
});
}, await context._injected(), selector, visibility, timeout);
return async (context: FrameExecutionContext) => {
selector = normalizeSelector(selector);
return context.evaluateHandle((injected: Injected, selector: string, visibility: types.Visibility, timeout: number) => {
if (visibility !== 'any')
return injected.pollRaf(selector, predicate, timeout);
return injected.pollMutation(selector, predicate, timeout);

function predicate(element: Element | undefined): Element | boolean {
if (!element)
return visibility === 'hidden';
if (visibility === 'any')
return element;
return injected.isVisible(element) === (visibility === 'visible') ? element : false;
}
}, await context._injected(), selector, visibility, timeout);
};
}

export const setFileInputFunction = async (element: HTMLInputElement, payloads: types.FilePayload[]) => {
Expand Down
16 changes: 8 additions & 8 deletions src/frames.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -780,19 +780,19 @@ export class Frame {
return result!;
}

async click(selector: string, options?: WaitForOptions & ClickOptions & dom.WaitForInteractableOptions) {
async click(selector: string, options?: WaitForOptions & ClickOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.click(options);
await handle.dispose();
}

async dblclick(selector: string, options?: WaitForOptions & MultiClickOptions & dom.WaitForInteractableOptions) {
async dblclick(selector: string, options?: WaitForOptions & MultiClickOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.dblclick(options);
await handle.dispose();
}

async tripleclick(selector: string, options?: WaitForOptions & MultiClickOptions & dom.WaitForInteractableOptions) {
async tripleclick(selector: string, options?: WaitForOptions & MultiClickOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.tripleclick(options);
await handle.dispose();
Expand All @@ -810,7 +810,7 @@ export class Frame {
await handle.dispose();
}

async hover(selector: string, options?: WaitForOptions & PointerActionOptions & dom.WaitForInteractableOptions) {
async hover(selector: string, options?: WaitForOptions & PointerActionOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.hover(options);
await handle.dispose();
Expand All @@ -830,15 +830,15 @@ export class Frame {
await handle.dispose();
}

async check(selector: string, options?: WaitForOptions & dom.WaitForInteractableOptions) {
async check(selector: string, options?: WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.check(options);
await handle.check();
await handle.dispose();
}

async uncheck(selector: string, options?: WaitForOptions & dom.WaitForInteractableOptions) {
async uncheck(selector: string, options?: WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.uncheck(options);
await handle.uncheck();
await handle.dispose();
}

Expand Down
14 changes: 3 additions & 11 deletions src/injected/injected.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class Injected {
return !!(rect.top || rect.bottom || rect.width || rect.height);
}

private _pollMutation(selector: string | undefined, predicate: Predicate, timeout: number): Promise<any> {
pollMutation(selector: string | undefined, predicate: Predicate, timeout: number): Promise<any> {
let timedOut = false;
if (timeout)
setTimeout(() => timedOut = true, timeout);
Expand Down Expand Up @@ -178,7 +178,7 @@ class Injected {
return result;
}

private _pollRaf(selector: string | undefined, predicate: Predicate, timeout: number): Promise<any> {
pollRaf(selector: string | undefined, predicate: Predicate, timeout: number): Promise<any> {
let timedOut = false;
if (timeout)
setTimeout(() => timedOut = true, timeout);
Expand All @@ -203,7 +203,7 @@ class Injected {
return result;
}

private _pollInterval(selector: string | undefined, pollInterval: number, predicate: Predicate, timeout: number): Promise<any> {
pollInterval(selector: string | undefined, pollInterval: number, predicate: Predicate, timeout: number): Promise<any> {
let timedOut = false;
if (timeout)
setTimeout(() => timedOut = true, timeout);
Expand All @@ -226,14 +226,6 @@ class Injected {
onTimeout();
return result;
}

poll(polling: 'raf' | 'mutation' | number, selector: string | undefined, timeout: number, predicate: Predicate): Promise<any> {
if (polling === 'raf')
return this._pollRaf(selector, predicate, timeout);
if (polling === 'mutation')
return this._pollMutation(selector, predicate, timeout);
return this._pollInterval(selector, polling, predicate, timeout);
}
}

export default Injected;
12 changes: 6 additions & 6 deletions src/page.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,15 +485,15 @@ export class Page extends platform.EventEmitter {
return this._closed;
}

async click(selector: string, options?: frames.WaitForOptions & input.ClickOptions & dom.WaitForInteractableOptions) {
async click(selector: string, options?: frames.WaitForOptions & input.ClickOptions) {
return this.mainFrame().click(selector, options);
}

async dblclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions & dom.WaitForInteractableOptions) {
async dblclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions) {
return this.mainFrame().dblclick(selector, options);
}

async tripleclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions & dom.WaitForInteractableOptions) {
async tripleclick(selector: string, options?: frames.WaitForOptions & input.MultiClickOptions) {
return this.mainFrame().tripleclick(selector, options);
}

Expand All @@ -505,7 +505,7 @@ export class Page extends platform.EventEmitter {
return this.mainFrame().focus(selector, options);
}

async hover(selector: string, options?: frames.WaitForOptions & input.PointerActionOptions & dom.WaitForInteractableOptions) {
async hover(selector: string, options?: frames.WaitForOptions & input.PointerActionOptions) {
return this.mainFrame().hover(selector, options);
}

Expand All @@ -517,11 +517,11 @@ export class Page extends platform.EventEmitter {
return this.mainFrame().type(selector, text, options);
}

async check(selector: string, options?: frames.WaitForOptions & dom.WaitForInteractableOptions) {
async check(selector: string, options?: frames.WaitForOptions) {
return this.mainFrame().check(selector, options);
}

async uncheck(selector: string, options?: frames.WaitForOptions & dom.WaitForInteractableOptions) {
async uncheck(selector: string, options?: frames.WaitForOptions) {
return this.mainFrame().uncheck(selector, options);
}

Expand Down
2 changes: 0 additions & 2 deletions test/assets/input/button.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
window.pageX = undefined;
window.pageY = undefined;
window.shiftKey = undefined;
window.pageX = undefined;
window.pageY = undefined;
document.querySelector('button').addEventListener('click', e => {
result = 'Clicked';
offsetX = e.offsetX;
Expand Down
Loading

0 comments on commit dbb45d4

Please sign in to comment.