Skip to content

Commit

Permalink
feat(input): perform hit target check during input (#9546)
Browse files Browse the repository at this point in the history
This replaces previous `checkHitTarget` heuristic that took place before the action
with a new `setupHitTargetInterceptor` that works during the action:
- Before the action we set up capturing listeners on the window.
- During the action we ensure that event target is the element we expect to interact with.
- After the action we clear the listeners.

This should catch the "layout shift" issues where things move
between action point calculation and the actual action.

Possible issues:
- **Risk:** `{ trial: true }` might dispatch move events like `mousemove` or `pointerout`,
because we do actually move the mouse but prevent all other events.
- **Timing**: The timing of "hit target check" has moved, so this may affect different web pages
in different ways, for example expose more races. In this case, we should retry the click as before.
- **No risk**: There is still a possibility of mis-targeting with iframes shifting around,
because we only intercept in the target frame. This behavior does not change.

There is an opt-out environment variable PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK that reverts to previous behavior.
  • Loading branch information
dgozman committed Nov 6, 2021
1 parent 7f1d6e4 commit 61ff527
Show file tree
Hide file tree
Showing 6 changed files with 287 additions and 10 deletions.
70 changes: 65 additions & 5 deletions packages/playwright-core/src/server/dom.ts
Expand Up @@ -19,7 +19,7 @@ import * as injectedScriptSource from '../generated/injectedScriptSource';
import * as channels from '../protocol/channels';
import { isSessionClosedError } from './common/protocolError';
import * as frames from './frames';
import type { InjectedScript, InjectedScriptPoll, LogEntry } from './injected/injectedScript';
import type { InjectedScript, InjectedScriptPoll, LogEntry, HitTargetInterceptionResult } from './injected/injectedScript';
import { CallMetadata } from './instrumentation';
import * as js from './javascript';
import { Page } from './page';
Expand Down Expand Up @@ -367,8 +367,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
continue;
}
if (typeof result === 'object' && 'hitTargetDescription' in result) {
if (options.force)
throw new Error(`Element does not receive pointer events, ${result.hitTargetDescription} intercepts them`);
progress.log(` ${result.hitTargetDescription} intercepts pointer events`);
continue;
}
Expand Down Expand Up @@ -407,8 +405,16 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
if (typeof maybePoint === 'string')
return maybePoint;
const point = roundPoint(maybePoint);
progress.metadata.point = point;

if (process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK)
return this._finishPointerAction(progress, actionName, point, options, action);
else
return this._finishPointerActionDetectLayoutShift(progress, actionName, point, options, action);
}

if (!force) {
private async _finishPointerAction(progress: Progress, actionName: string, point: types.Point, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions, action: (point: types.Point) => Promise<void>) {
if (!options.force) {
if ((options as any).__testHookBeforeHitTarget)
await (options as any).__testHookBeforeHitTarget();
progress.log(` checking that element receives pointer events at (${point.x},${point.y})`);
Expand All @@ -418,7 +424,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
progress.log(` element does receive pointer events`);
}

progress.metadata.point = point;
if (options.trial) {
progress.log(` trial ${actionName} has finished`);
return 'done';
Expand Down Expand Up @@ -446,6 +451,61 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return 'done';
}

private async _finishPointerActionDetectLayoutShift(progress: Progress, actionName: string, point: types.Point, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions, action: (point: types.Point) => Promise<void>) {
await progress.beforeInputAction(this);

let hitTargetInterceptionHandle: js.JSHandle<HitTargetInterceptionResult> | undefined;
if (!options.force) {
if ((options as any).__testHookBeforeHitTarget)
await (options as any).__testHookBeforeHitTarget();

const actionType = (actionName === 'hover' || actionName === 'tap') ? actionName : 'mouse';
const handle = await this.evaluateHandleInUtility(([injected, node, { actionType, trial }]) => injected.setupHitTargetInterceptor(node, actionType, trial), { actionType, trial: !!options.trial } as const);
if (handle === 'error:notconnected')
return handle;
if (!handle._objectId)
return handle.rawValue() as 'error:notconnected';
hitTargetInterceptionHandle = handle as any;
progress.cleanupWhenAborted(() => {
// Do not await here, just in case the renderer is stuck (e.g. on alert)
// and we won't be able to cleanup.
hitTargetInterceptionHandle!.evaluate(h => h.stop()).catch(e => {});
});
}

const actionResult = await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
if ((options as any).__testHookBeforePointerAction)
await (options as any).__testHookBeforePointerAction();
progress.throwIfAborted(); // Avoid action that has side-effects.
let restoreModifiers: types.KeyboardModifier[] | undefined;
if (options && options.modifiers)
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
progress.log(` performing ${actionName} action`);
await action(point);
if (restoreModifiers)
await this._page.keyboard._ensureModifiers(restoreModifiers);
if (hitTargetInterceptionHandle) {
const stopHitTargetInterception = hitTargetInterceptionHandle.evaluate(h => h.stop()).catch(e => 'done' as const);
if (!options.noWaitAfter) {
// When noWaitAfter is passed, we do not want to accidentally stall on
// non-committed navigation blocking the evaluate.
const hitTargetResult = await stopHitTargetInterception;
if (hitTargetResult !== 'done')
return hitTargetResult;
}
}
progress.log(` ${options.trial ? 'trial ' : ''}${actionName} action done`);
progress.log(' waiting for scheduled navigations to finish');
if ((options as any).__testHookAfterPointerAction)
await (options as any).__testHookAfterPointerAction();
return 'done';
}, 'input');
if (actionResult !== 'done')
return actionResult;
progress.log(' navigations have finished');
return 'done';
}

async hover(metadata: CallMetadata, options: types.PointerActionOptions & types.PointerActionWaitOptions): Promise<void> {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
Expand Down
83 changes: 79 additions & 4 deletions packages/playwright-core/src/server/injected/injectedScript.ts
Expand Up @@ -63,12 +63,17 @@ export type ElementMatch = {
capture: Element | undefined;
};

export type HitTargetInterceptionResult = {
stop: () => 'done' | { hitTargetDescription: string };
};

export class InjectedScript {
private _engines: Map<string, SelectorEngineV2>;
_evaluator: SelectorEvaluatorImpl;
private _stableRafCount: number;
private _browserName: string;
onGlobalListenersRemoved = new Set<() => void>();
private _hitTargetInterceptor: undefined | ((event: MouseEvent | PointerEvent | TouchEvent) => void);

constructor(stableRafCount: number, browserName: string, customEngines: { name: string, engine: SelectorEngine}[]) {
this._evaluator = new SelectorEvaluatorImpl(new Map());
Expand Down Expand Up @@ -99,6 +104,7 @@ export class InjectedScript {
this._browserName = browserName;

this._setupGlobalListenersRemovalDetection();
this._setupHitTargetInterceptors();
}

eval(expression: string): any {
Expand Down Expand Up @@ -654,19 +660,24 @@ export class InjectedScript {
if (!element || !element.isConnected)
return 'error:notconnected';
element = element.closest('button, [role=button]') || element;
let hitElement = this.deepElementFromPoint(document, point.x, point.y);
const hitElement = this.deepElementFromPoint(document, point.x, point.y);
return this._expectHitTargetParent(hitElement, element);
}

private _expectHitTargetParent(hitElement: Element | undefined, targetElement: Element) {
const hitParents: Element[] = [];
while (hitElement && hitElement !== element) {
while (hitElement && hitElement !== targetElement) {
hitParents.push(hitElement);
hitElement = parentElementOrShadowHost(hitElement);
}
if (hitElement === element)
if (hitElement === targetElement)
return 'done';
const hitTargetDescription = this.previewNode(hitParents[0]);
const hitTargetDescription = this.previewNode(hitParents[0] || document.documentElement);
// Root is the topmost element in the hitTarget's chain that is not in the
// element's chain. For example, it might be a dialog element that overlays
// the target.
let rootHitTargetDescription: string | undefined;
let element: Element | undefined = targetElement;
while (element) {
const index = hitParents.indexOf(element);
if (index !== -1) {
Expand All @@ -681,6 +692,55 @@ export class InjectedScript {
return { hitTargetDescription };
}

setupHitTargetInterceptor(node: Node, action: 'hover' | 'tap' | 'mouse', blockAllEvents: boolean): HitTargetInterceptionResult | 'error:notconnected' {
const maybeElement: Element | null | undefined = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
if (!maybeElement || !maybeElement.isConnected)
return 'error:notconnected';
const element = maybeElement.closest('button, [role=button]') || maybeElement;

const events = {
'hover': kHoverHitTargetInterceptorEvents,
'tap': kTapHitTargetInterceptorEvents,
'mouse': kMouseHitTargetInterceptorEvents,
}[action];
let result: 'done' | { hitTargetDescription: string } | undefined;

const listener = (event: PointerEvent | MouseEvent | TouchEvent) => {
// Ignore events that we do not expect to intercept.
if (!events.has(event.type))
return;

// Playwright only issues trusted events, so allow any custom events originating from
// the page or content scripts.
if (!event.isTrusted)
return;

// Determine the event point. Note that Firefox does not always have window.TouchEvent.
const point = (!!window.TouchEvent && (event instanceof window.TouchEvent)) ? event.touches[0] : (event as MouseEvent | PointerEvent);
if (!!point && (result === undefined || result === 'done')) {
// Determine whether we hit the target element, unless we have already failed.
const hitElement = this.deepElementFromPoint(document, point.clientX, point.clientY);
result = this._expectHitTargetParent(hitElement, element);
}
if (blockAllEvents || result !== 'done') {
event.preventDefault();
event.stopPropagation();
event.stopImmediatePropagation();
}
};

const stop = () => {
if (this._hitTargetInterceptor === listener)
this._hitTargetInterceptor = undefined;
return result!;
};

// Note: this removes previous listener, just in case there are two concurrent clicks
// or something went wrong and we did not cleanup.
this._hitTargetInterceptor = listener;
return { stop };
}

dispatchEvent(node: Node, type: string, eventInit: Object) {
let event;
eventInit = { bubbles: true, cancelable: true, composed: true, ...eventInit };
Expand Down Expand Up @@ -798,6 +858,16 @@ export class InjectedScript {
}).observe(document, { childList: true });
}

private _setupHitTargetInterceptors() {
const listener = (event: PointerEvent | MouseEvent | TouchEvent) => this._hitTargetInterceptor?.(event);
const addHitTargetInterceptorListeners = () => {
for (const event of kAllHitTargetInterceptorEvents)
window.addEventListener(event as any, listener, { capture: true, passive: false });
};
addHitTargetInterceptorListeners();
this.onGlobalListenersRemoved.add(addHitTargetInterceptorListeners);
}

expectSingleElement(progress: InjectedScriptProgress, element: Element, options: FrameExpectParams): { matches: boolean, received?: any } {
const injected = progress.injectedScript;
const expression = options.expression;
Expand Down Expand Up @@ -972,6 +1042,11 @@ const eventType = new Map<string, 'mouse'|'keyboard'|'touch'|'pointer'|'focus'|'
['drop', 'drag'],
]);

const kHoverHitTargetInterceptorEvents = new Set(['mousemove']);
const kTapHitTargetInterceptorEvents = new Set(['pointerdown', 'pointerup', 'touchstart', 'touchend', 'touchcancel']);
const kMouseHitTargetInterceptorEvents = new Set(['mousedown', 'mouseup', 'pointerdown', 'pointerup', 'click', 'auxclick', 'dblclick', 'contextmenu']);
const kAllHitTargetInterceptorEvents = new Set([...kHoverHitTargetInterceptorEvents, ...kTapHitTargetInterceptorEvents, ...kMouseHitTargetInterceptorEvents]);

function unescape(s: string): string {
if (!s.includes('\\'))
return s;
Expand Down
108 changes: 108 additions & 0 deletions tests/hit-target.spec.ts
@@ -0,0 +1,108 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { contextTest as it, expect } from './config/browserTest';

it('should block all events when hit target is wrong', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.evaluate(() => {
const blocker = document.createElement('div');
blocker.style.position = 'absolute';
blocker.style.width = '400px';
blocker.style.height = '400px';
blocker.style.left = '0';
blocker.style.top = '0';
document.body.appendChild(blocker);

const allEvents = [];
(window as any).allEvents = allEvents;
for (const name of ['mousedown', 'mouseup', 'click', 'dblclick', 'auxclick', 'contextmenu', 'pointerdown', 'pointerup']) {
window.addEventListener(name, e => allEvents.push(e.type));
blocker.addEventListener(name, e => allEvents.push(e.type));
}
});

const error = await page.click('button', { timeout: 1000 }).catch(e => e);
expect(error.message).toContain('page.click: Timeout 1000ms exceeded.');

// Give it some time, just in case.
await page.waitForTimeout(1000);
const allEvents = await page.evaluate(() => (window as any).allEvents);
expect(allEvents).toEqual([]);
});

it('should block click when mousedown succeeds but mouseup fails', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', button => {
button.addEventListener('mousedown', () => {
button.style.marginLeft = '100px';
});

const allEvents = [];
(window as any).allEvents = allEvents;
for (const name of ['mousedown', 'mouseup', 'click', 'dblclick', 'auxclick', 'contextmenu', 'pointerdown', 'pointerup'])
button.addEventListener(name, e => allEvents.push(e.type));
});

await page.click('button');
expect(await page.evaluate('result')).toBe('Clicked');
const allEvents = await page.evaluate(() => (window as any).allEvents);
expect(allEvents).toEqual([
// First attempt failed.
'pointerdown', 'mousedown',
// Second attempt succeeded.
'pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click',
]);
});

it('should not block programmatic events', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', button => {
button.addEventListener('mousedown', () => {
button.style.marginLeft = '100px';
button.dispatchEvent(new MouseEvent('click'));
});

const allEvents = [];
(window as any).allEvents = allEvents;
button.addEventListener('click', e => {
if (!e.isTrusted)
allEvents.push(e.type);
});
});

await page.click('button');
expect(await page.evaluate('result')).toBe('Clicked');
const allEvents = await page.evaluate(() => (window as any).allEvents);
// We should get one programmatic click on each attempt.
expect(allEvents).toEqual([
'click', 'click',
]);
});

it('should click the button again after document.write', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.click('button');
expect(await page.evaluate('result')).toBe('Clicked');

await page.evaluate(() => {
document.open();
document.write('<button onclick="window.result2 = true"></button>');
document.close();
});
await page.click('button');
expect(await page.evaluate('result2')).toBe(true);
});
17 changes: 17 additions & 0 deletions tests/page/page-click-timeout-3.spec.ts
Expand Up @@ -57,6 +57,23 @@ it('should timeout waiting for hit target', async ({ page, server }) => {
expect(error.message).toContain('waiting 500ms');
});

it('should still click when force but hit target is obscured', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button');
await page.evaluate(() => {
document.body.style.position = 'relative';
const blocker = document.createElement('div');
blocker.id = 'blocker';
blocker.style.position = 'absolute';
blocker.style.width = '400px';
blocker.style.height = '200px';
blocker.style.left = '0';
blocker.style.top = '0';
document.body.appendChild(blocker);
});
await button.click({ force: true });
});

it('should report wrong hit target subtree', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button');
Expand Down
16 changes: 16 additions & 0 deletions tests/page/page-click-timeout-4.spec.ts
Expand Up @@ -29,3 +29,19 @@ it('should timeout waiting for stable position', async ({ page, server }) => {
expect(error.message).toContain('waiting for element to be visible, enabled and stable');
expect(error.message).toContain('element is not stable - waiting');
});

it('should click for the second time after first timeout', async ({ page, server, mode }) => {
it.skip(mode !== 'default');

await page.goto(server.PREFIX + '/input/button.html');
const __testHookBeforePointerAction = () => new Promise(f => setTimeout(f, 1500));
const error = await page.click('button', { timeout: 1000, __testHookBeforePointerAction } as any).catch(e => e);
expect(error.message).toContain('page.click: Timeout 1000ms exceeded.');

expect(await page.evaluate('result')).toBe('Was not clicked');
await page.waitForTimeout(2000);
expect(await page.evaluate('result')).toBe('Was not clicked');

await page.click('button');
expect(await page.evaluate('result')).toBe('Clicked');
});

0 comments on commit 61ff527

Please sign in to comment.