Skip to content

Commit e0d1741

Browse files
dgozmandgozman
committed
cherry-pick(#36346): Revert "chore: reduce scrolling during clicks (#36175)"
1 parent c568980 commit e0d1741

File tree

3 files changed

+74
-41
lines changed

3 files changed

+74
-41
lines changed

‎packages/injected/src/injectedScript.ts

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,8 @@ export type ElementState = 'visible' | 'hidden' | 'enabled' | 'disabled' | 'edit
5353
export type ElementStateWithoutStable = Exclude<ElementState, 'stable'>;
5454
export type ElementStateQueryResult = { matches: boolean, received?: string | 'error:notconnected' };
5555

56-
export type HitTargetError = { hitTargetDescription: string, hasPositionStickyOrFixed: boolean };
5756
export type HitTargetInterceptionResult = {
58-
stop: () => 'done' | HitTargetError;
57+
stop: () => 'done' | { hitTargetDescription: string };
5958
};
6059

6160
interface WebKitLegacyDeviceOrientationEvent extends DeviceOrientationEvent {
@@ -924,7 +923,7 @@ export class InjectedScript {
924923
input.dispatchEvent(new Event('change', { bubbles: true }));
925924
}
926925

927-
expectHitTarget(hitPoint: { x: number, y: number }, targetElement: Element): 'done' | HitTargetError {
926+
expectHitTarget(hitPoint: { x: number, y: number }, targetElement: Element) {
928927
const roots: (Document | ShadowRoot)[] = [];
929928

930929
// Get all component roots leading to the target element.
@@ -977,21 +976,14 @@ export class InjectedScript {
977976

978977
// Check whether hit target is the target or its descendant.
979978
const hitParents: Element[] = [];
980-
const isHitParentPositionStickyOrFixed: boolean[] = [];
981979
while (hitElement && hitElement !== targetElement) {
982980
hitParents.push(hitElement);
983-
isHitParentPositionStickyOrFixed.push(['sticky', 'fixed'].includes(this.window.getComputedStyle(hitElement).position));
984981
hitElement = parentElementOrShadowHost(hitElement);
985982
}
986983
if (hitElement === targetElement)
987984
return 'done';
988985

989-
// The description of the element that was hit instead of the target element.
990986
const hitTargetDescription = this.previewNode(hitParents[0] || this.document.documentElement);
991-
// Whether any ancestor of the hit target has position: static. In this case, it could be
992-
// beneficial to scroll the target element into different positions to reveal it.
993-
let hasPositionStickyOrFixed = isHitParentPositionStickyOrFixed.some(x => x);
994-
995987
// Root is the topmost element in the hitTarget's chain that is not in the
996988
// element's chain. For example, it might be a dialog element that overlays
997989
// the target.
@@ -1002,14 +994,13 @@ export class InjectedScript {
1002994
if (index !== -1) {
1003995
if (index > 1)
1004996
rootHitTargetDescription = this.previewNode(hitParents[index - 1]);
1005-
hasPositionStickyOrFixed = isHitParentPositionStickyOrFixed.slice(0, index).some(x => x);
1006997
break;
1007998
}
1008999
element = parentElementOrShadowHost(element);
10091000
}
10101001
if (rootHitTargetDescription)
1011-
return { hitTargetDescription: `${hitTargetDescription} from ${rootHitTargetDescription} subtree`, hasPositionStickyOrFixed };
1012-
return { hitTargetDescription, hasPositionStickyOrFixed };
1002+
return { hitTargetDescription: `${hitTargetDescription} from ${rootHitTargetDescription} subtree` };
1003+
return { hitTargetDescription };
10131004
}
10141005

10151006
// Life of a pointer action, for example click.
@@ -1042,7 +1033,7 @@ export class InjectedScript {
10421033
// 2k. (injected) Event interceptor is removed.
10431034
// 2l. All navigations triggered between 2g-2k are awaited to be either committed or canceled.
10441035
// 2m. If failed, wait for increasing amount of time before the next retry.
1045-
setupHitTargetInterceptor(node: Node, action: 'hover' | 'tap' | 'mouse' | 'drag', hitPoint: { x: number, y: number } | undefined, blockAllEvents: boolean): HitTargetInterceptionResult | 'error:notconnected' | string /* JSON.stringify(hitTargetDescription) */ {
1036+
setupHitTargetInterceptor(node: Node, action: 'hover' | 'tap' | 'mouse' | 'drag', hitPoint: { x: number, y: number } | undefined, blockAllEvents: boolean): HitTargetInterceptionResult | 'error:notconnected' | string /* hitTargetDescription */ {
10461037
const element = this.retarget(node, 'button-link');
10471038
if (!element || !element.isConnected)
10481039
return 'error:notconnected';
@@ -1052,7 +1043,7 @@ export class InjectedScript {
10521043
// intercepting the action.
10531044
const preliminaryResult = this.expectHitTarget(hitPoint, element);
10541045
if (preliminaryResult !== 'done')
1055-
return JSON.stringify(preliminaryResult);
1046+
return preliminaryResult.hitTargetDescription;
10561047
}
10571048

10581049
// When dropping, the "element that is being dragged" often stays under the cursor,
@@ -1067,7 +1058,7 @@ export class InjectedScript {
10671058
'tap': this._tapHitTargetInterceptorEvents,
10681059
'mouse': this._mouseHitTargetInterceptorEvents,
10691060
}[action];
1070-
let result: 'done' | HitTargetError | undefined;
1061+
let result: 'done' | { hitTargetDescription: string } | undefined;
10711062

10721063
const listener = (event: PointerEvent | MouseEvent | TouchEvent) => {
10731064
// Ignore events that we do not expect to intercept.

‎packages/playwright-core/src/server/dom.ts

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { isSessionClosedError } from './protocolError';
2424
import * as rawInjectedScriptSource from '../generated/injectedScriptSource';
2525

2626
import type * as frames from './frames';
27-
import type { ElementState, HitTargetError, HitTargetInterceptionResult, InjectedScript, InjectedScriptOptions } from '@injected/injectedScript';
27+
import type { ElementState, HitTargetInterceptionResult, InjectedScript, InjectedScriptOptions } from '@injected/injectedScript';
2828
import type { CallMetadata } from './instrumentation';
2929
import type { Page } from './page';
3030
import type { Progress } from './progress';
@@ -39,7 +39,7 @@ export type InputFilesItems = {
3939
};
4040

4141
type ActionName = 'click' | 'hover' | 'dblclick' | 'tap' | 'move and up' | 'move and down';
42-
type PerformActionResult = 'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | 'error:optionsnotfound' | { missingState: ElementState } | HitTargetError | 'done';
42+
type PerformActionResult = 'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | 'error:optionsnotfound' | { missingState: ElementState } | { hitTargetDescription: string } | 'done';
4343

4444
export class NonRecoverableDOMError extends Error {
4545
}
@@ -332,7 +332,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
332332
};
333333
}
334334

335-
async _retryAction(progress: Progress, actionName: string, action: () => Promise<PerformActionResult>, options: { trial?: boolean, force?: boolean, skipActionPreChecks?: boolean }): Promise<'error:notconnected' | 'done'> {
335+
async _retryAction(progress: Progress, actionName: string, action: (retry: number) => Promise<PerformActionResult>, options: { trial?: boolean, force?: boolean, skipActionPreChecks?: boolean }): Promise<'error:notconnected' | 'done'> {
336336
let retry = 0;
337337
// We progressively wait longer between retries, up to 500ms.
338338
const waitTime = [0, 20, 100, 100, 500];
@@ -352,7 +352,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
352352
}
353353
if (!options.skipActionPreChecks && !options.force)
354354
await this._frame._page.performActionPreChecks(progress);
355-
const result = await action();
355+
const result = await action(retry);
356356
++retry;
357357
if (result === 'error:notvisible') {
358358
if (options.force)
@@ -387,25 +387,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
387387
options: { waitAfter: boolean | 'disabled' } & types.PointerActionOptions & types.PointerActionWaitOptions): Promise<'error:notconnected' | 'done'> {
388388
// Note: do not perform locator handlers checkpoint to avoid moving the mouse in the middle of a drag operation.
389389
const skipActionPreChecks = actionName === 'move and up';
390-
// By default, we scroll with protocol method to reveal the action point.
391-
// However, that might not work to scroll from under position:sticky and position:fixed elements
392-
// that overlay the target element. To fight this, we cycle through different
393-
// scroll alignments. This works in most scenarios.
394-
const scrollOptions: (ScrollIntoViewOptions | undefined)[] = [
395-
undefined,
396-
{ block: 'end', inline: 'end' },
397-
{ block: 'center', inline: 'center' },
398-
{ block: 'start', inline: 'start' },
399-
];
400-
let scrollOptionIndex = 0;
401-
return await this._retryAction(progress, actionName, async () => {
402-
const forceScrollOptions = scrollOptions[scrollOptionIndex % scrollOptions.length];
403-
const result = await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options);
404-
if (typeof result === 'object' && 'hasPositionStickyOrFixed' in result && result.hasPositionStickyOrFixed)
405-
++scrollOptionIndex;
406-
else
407-
scrollOptionIndex = 0;
408-
return result;
390+
return await this._retryAction(progress, actionName, async retry => {
391+
// By default, we scroll with protocol method to reveal the action point.
392+
// However, that might not work to scroll from under position:sticky elements
393+
// that overlay the target element. To fight this, we cycle through different
394+
// scroll alignments. This works in most scenarios.
395+
const scrollOptions: (ScrollIntoViewOptions | undefined)[] = [
396+
undefined,
397+
{ block: 'end', inline: 'end' },
398+
{ block: 'center', inline: 'center' },
399+
{ block: 'start', inline: 'start' },
400+
];
401+
const forceScrollOptions = scrollOptions[retry % scrollOptions.length];
402+
return await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options);
409403
}, { ...options, skipActionPreChecks });
410404
}
411405

@@ -488,7 +482,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
488482
const error = handle.rawValue() as string;
489483
if (error === 'error:notconnected')
490484
return error;
491-
return JSON.parse(error) as HitTargetError; // It is safe to parse, because we evaluated in utility.
485+
return { hitTargetDescription: error };
492486
}
493487
hitTargetInterceptionHandle = handle as any;
494488
progress.cleanupWhenAborted(() => {
@@ -917,7 +911,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
917911
return this;
918912
}
919913

920-
async _checkFrameIsHitTarget(point: types.Point): Promise<{ framePoint: types.Point | undefined } | 'error:notconnected' | HitTargetError> {
914+
async _checkFrameIsHitTarget(point: types.Point): Promise<{ framePoint: types.Point | undefined } | 'error:notconnected' | { hitTargetDescription: string }> {
921915
let frame = this._frame;
922916
const data: { frame: frames.Frame, frameElement: ElementHandle<Element> | null, pointInFrame: types.Point }[] = [];
923917
while (frame.parentFrame()) {

‎tests/page/page-click.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,54 @@ it('should click the button behind sticky header', async ({ page }) => {
419419
expect(await page.evaluate(() => window['__clicked'])).toBe(true);
420420
});
421421

422+
it('should click the button behind position:absolute header', {
423+
annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/36339' },
424+
}, async ({ page }) => {
425+
await page.setViewportSize({ width: 500, height: 240 });
426+
await page.setContent(`
427+
<style>
428+
* {
429+
padding: 0;
430+
margin: 0;
431+
}
432+
li {
433+
height: 80px;
434+
border: 1px solid black;
435+
}
436+
ol {
437+
height: 100vh;
438+
overflow: scroll;
439+
padding-top: 160px;
440+
}
441+
body {
442+
position: relative;
443+
}
444+
div.fixed {
445+
position: absolute;
446+
top: 0;
447+
z-index: 1001;
448+
width: 100%;
449+
background: red;
450+
height: 160px;
451+
}
452+
</style>
453+
454+
<ol>
455+
<li>hi1</li><li>hi2</li><li>hi3</li><li>hi4</li><li>hi5</li><li>hi6</li><li>hi7</li><li>hi8</li>
456+
<li id=target onclick="window.__clicked = true">hi9</li>
457+
<li>hi10</li><li>hi11</li><li>hi12</li><li>hi13</li><li id=li14>hi14</li>
458+
</ol>
459+
460+
<div class=fixed>Overlay</div>
461+
`);
462+
await page.$eval('ol', e => {
463+
const target = document.querySelector('#target') as HTMLElement;
464+
e.scrollTo({ top: target.offsetTop, behavior: 'instant' });
465+
});
466+
await page.click('#target');
467+
expect(await page.evaluate(() => window['__clicked'])).toBe(true);
468+
});
469+
422470
it('should click the button with px border with offset', async ({ page, server, browserName }) => {
423471
await page.goto(server.PREFIX + '/input/button.html');
424472
await page.$eval('button', button => button.style.borderWidth = '8px');

0 commit comments

Comments
 (0)