Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e0d1741

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

File tree

Expand file treeCollapse file tree

3 files changed

+74
-41
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+74
-41
lines changed
Open diff view settings
Collapse file

‎packages/injected/src/injectedScript.ts‎

Copy file name to clipboardExpand all lines: packages/injected/src/injectedScript.ts
+7-16Lines 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.
Collapse file

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

Copy file name to clipboardExpand all lines: packages/playwright-core/src/server/dom.ts
+19-25Lines 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()) {
Collapse file

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

Copy file name to clipboardExpand all lines: tests/page/page-click.spec.ts
+48Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.