feat(input): perform hit target check during input (#9546)

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.
This commit is contained in:
Dmitry Gozman 2021-11-05 17:31:28 -07:00 committed by GitHub
parent 7f1d6e4c16
commit 61ff52704c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 287 additions and 10 deletions

View file

@ -19,7 +19,7 @@ import * as injectedScriptSource from '../generated/injectedScriptSource';
import * as channels from '../protocol/channels'; import * as channels from '../protocol/channels';
import { isSessionClosedError } from './common/protocolError'; import { isSessionClosedError } from './common/protocolError';
import * as frames from './frames'; 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 { CallMetadata } from './instrumentation';
import * as js from './javascript'; import * as js from './javascript';
import { Page } from './page'; import { Page } from './page';
@ -367,8 +367,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
continue; continue;
} }
if (typeof result === 'object' && 'hitTargetDescription' in result) { 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`); progress.log(` ${result.hitTargetDescription} intercepts pointer events`);
continue; continue;
} }
@ -407,8 +405,16 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
if (typeof maybePoint === 'string') if (typeof maybePoint === 'string')
return maybePoint; return maybePoint;
const point = roundPoint(maybePoint); const point = roundPoint(maybePoint);
progress.metadata.point = point;
if (!force) { 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);
}
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) if ((options as any).__testHookBeforeHitTarget)
await (options as any).__testHookBeforeHitTarget(); await (options as any).__testHookBeforeHitTarget();
progress.log(` checking that element receives pointer events at (${point.x},${point.y})`); progress.log(` checking that element receives pointer events at (${point.x},${point.y})`);
@ -418,7 +424,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
progress.log(` element does receive pointer events`); progress.log(` element does receive pointer events`);
} }
progress.metadata.point = point;
if (options.trial) { if (options.trial) {
progress.log(` trial ${actionName} has finished`); progress.log(` trial ${actionName} has finished`);
return 'done'; return 'done';
@ -446,6 +451,61 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return 'done'; 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> { async hover(metadata: CallMetadata, options: types.PointerActionOptions & types.PointerActionWaitOptions): Promise<void> {
const controller = new ProgressController(metadata, this); const controller = new ProgressController(metadata, this);
return controller.run(async progress => { return controller.run(async progress => {

View file

@ -63,12 +63,17 @@ export type ElementMatch = {
capture: Element | undefined; capture: Element | undefined;
}; };
export type HitTargetInterceptionResult = {
stop: () => 'done' | { hitTargetDescription: string };
};
export class InjectedScript { export class InjectedScript {
private _engines: Map<string, SelectorEngineV2>; private _engines: Map<string, SelectorEngineV2>;
_evaluator: SelectorEvaluatorImpl; _evaluator: SelectorEvaluatorImpl;
private _stableRafCount: number; private _stableRafCount: number;
private _browserName: string; private _browserName: string;
onGlobalListenersRemoved = new Set<() => void>(); onGlobalListenersRemoved = new Set<() => void>();
private _hitTargetInterceptor: undefined | ((event: MouseEvent | PointerEvent | TouchEvent) => void);
constructor(stableRafCount: number, browserName: string, customEngines: { name: string, engine: SelectorEngine}[]) { constructor(stableRafCount: number, browserName: string, customEngines: { name: string, engine: SelectorEngine}[]) {
this._evaluator = new SelectorEvaluatorImpl(new Map()); this._evaluator = new SelectorEvaluatorImpl(new Map());
@ -99,6 +104,7 @@ export class InjectedScript {
this._browserName = browserName; this._browserName = browserName;
this._setupGlobalListenersRemovalDetection(); this._setupGlobalListenersRemovalDetection();
this._setupHitTargetInterceptors();
} }
eval(expression: string): any { eval(expression: string): any {
@ -654,19 +660,24 @@ export class InjectedScript {
if (!element || !element.isConnected) if (!element || !element.isConnected)
return 'error:notconnected'; return 'error:notconnected';
element = element.closest('button, [role=button]') || element; 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[] = []; const hitParents: Element[] = [];
while (hitElement && hitElement !== element) { while (hitElement && hitElement !== targetElement) {
hitParents.push(hitElement); hitParents.push(hitElement);
hitElement = parentElementOrShadowHost(hitElement); hitElement = parentElementOrShadowHost(hitElement);
} }
if (hitElement === element) if (hitElement === targetElement)
return 'done'; 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 // 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 // element's chain. For example, it might be a dialog element that overlays
// the target. // the target.
let rootHitTargetDescription: string | undefined; let rootHitTargetDescription: string | undefined;
let element: Element | undefined = targetElement;
while (element) { while (element) {
const index = hitParents.indexOf(element); const index = hitParents.indexOf(element);
if (index !== -1) { if (index !== -1) {
@ -681,6 +692,55 @@ export class InjectedScript {
return { hitTargetDescription }; 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) { dispatchEvent(node: Node, type: string, eventInit: Object) {
let event; let event;
eventInit = { bubbles: true, cancelable: true, composed: true, ...eventInit }; eventInit = { bubbles: true, cancelable: true, composed: true, ...eventInit };
@ -798,6 +858,16 @@ export class InjectedScript {
}).observe(document, { childList: true }); }).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 } { expectSingleElement(progress: InjectedScriptProgress, element: Element, options: FrameExpectParams): { matches: boolean, received?: any } {
const injected = progress.injectedScript; const injected = progress.injectedScript;
const expression = options.expression; const expression = options.expression;
@ -972,6 +1042,11 @@ const eventType = new Map<string, 'mouse'|'keyboard'|'touch'|'pointer'|'focus'|'
['drop', 'drag'], ['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 { function unescape(s: string): string {
if (!s.includes('\\')) if (!s.includes('\\'))
return s; return s;

108
tests/hit-target.spec.ts Normal file
View file

@ -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);
});

View file

@ -57,6 +57,23 @@ it('should timeout waiting for hit target', async ({ page, server }) => {
expect(error.message).toContain('waiting 500ms'); 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 }) => { it('should report wrong hit target subtree', async ({ page, server }) => {
await page.goto(server.PREFIX + '/input/button.html'); await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button'); const button = await page.$('button');

View file

@ -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('waiting for element to be visible, enabled and stable');
expect(error.message).toContain('element is not stable - waiting'); 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');
});

View file

@ -48,7 +48,8 @@ it('trial run should not tap', async ({ page }) => {
await page.tap('#a'); await page.tap('#a');
const eventsHandle = await trackEvents(await page.$('#b')); const eventsHandle = await trackEvents(await page.$('#b'));
await page.tap('#b', { trial: true }); await page.tap('#b', { trial: true });
expect(await eventsHandle.jsonValue()).toEqual([]); const expected = process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK ? [] : ['pointerover', 'pointerenter', 'pointerout', 'pointerleave'];
expect(await eventsHandle.jsonValue()).toEqual(expected);
}); });
it('should not send mouse events touchstart is canceled', async ({ page }) => { it('should not send mouse events touchstart is canceled', async ({ page }) => {