From 35533b15c1aa13da694715c1ea762ff853b527b0 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 16 Dec 2020 15:29:42 -0800
Subject: [PATCH] fix(scroll): scroll from under the sticky header (#4641)

When element with position:sticky covers some part of
the scroll container, we could fail to scroll from under it
to perform an action. To fight this, we can try different
scroll alignments and scroll to the top/bottom/center
in the attempt to scroll away from sticky header/footer/sidebar.
---
 src/server/dom.ts       | 32 ++++++++++++++++++++++++++------
 test/page-click.spec.ts | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/src/server/dom.ts b/src/server/dom.ts
index ef84753916..966ea55020 100644
--- a/src/server/dom.ts
+++ b/src/server/dom.ts
@@ -276,7 +276,19 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
     let retry = 0;
     // We progressively wait longer between retries, up to 500ms.
-    const waitTime = [0, 20, 100, 500];
+    const waitTime = [0, 20, 100, 100, 500];
+
+    // By default, we scroll with protocol method to reveal the action point.
+    // However, that might not work to scroll from under position:sticky elements
+    // that overlay the target element. To fight this, we cycle through different
+    // scroll alignments. This works in most scenarios.
+    const scrollOptions: (ScrollIntoViewOptions | undefined)[] = [
+      undefined,
+      { block: 'end', inline: 'end' },
+      { block: 'center', inline: 'center' },
+      { block: 'start', inline: 'start' },
+    ];
+
     while (progress.isRunning()) {
       if (retry) {
         progress.log(`retrying ${actionName} action, attempt #${retry}`);
@@ -288,7 +300,8 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       } else {
         progress.log(`attempting ${actionName} action`);
       }
-      const result = await this._performPointerAction(progress, actionName, waitForEnabled, action, options);
+      const forceScrollOptions = scrollOptions[retry % scrollOptions.length];
+      const result = await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options);
       ++retry;
       if (result === 'error:notvisible') {
         if (options.force)
@@ -313,7 +326,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return 'done';
   }
 
-  async _performPointerAction(progress: Progress, actionName: string, waitForEnabled: boolean, action: (point: types.Point) => Promise<void>, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | { hitTargetDescription: string } | 'done'> {
+  async _performPointerAction(progress: Progress, actionName: string, waitForEnabled: boolean, action: (point: types.Point) => Promise<void>, forceScrollOptions: ScrollIntoViewOptions | undefined, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | { hitTargetDescription: string } | 'done'> {
     const { force = false, position } = options;
     if ((options as any).__testHookBeforeStable)
       await (options as any).__testHookBeforeStable();
@@ -327,9 +340,16 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
 
     progress.log('  scrolling into view if needed');
     progress.throwIfAborted();  // Avoid action that has side-effects.
-    const scrolled = await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
-    if (scrolled !== 'done')
-      return scrolled;
+    if (forceScrollOptions) {
+      await this._evaluateInUtility(([injected, node, options]) => {
+        if (node.nodeType === 1 /* Node.ELEMENT_NODE */)
+          (node as Node as Element).scrollIntoView(options);
+      }, forceScrollOptions);
+    } else {
+      const scrolled = await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
+      if (scrolled !== 'done')
+        return scrolled;
+    }
     progress.log('  done scrolling');
 
     const maybePoint = position ? await this._offsetPoint(position) : await this._clickablePoint();
diff --git a/test/page-click.spec.ts b/test/page-click.spec.ts
index 3b3ff68d9e..ebfd53d8ed 100644
--- a/test/page-click.spec.ts
+++ b/test/page-click.spec.ts
@@ -338,6 +338,43 @@ it('should click the button with fixed position inside an iframe', (test, { brow
   expect(await frame.evaluate(() => window['result'])).toBe('Clicked');
 });
 
+it('should click the button behind sticky header', async ({page}) => {
+  await page.setViewportSize({ width: 500, height: 240 });
+  await page.setContent(`
+    <style>
+    * {
+      padding: 0;
+      margin: 0;
+    }
+    li {
+      height: 80px;
+      border: 1px solid black;
+    }
+    ol {
+      padding-top: 160px;
+    }
+    div.fixed {
+      position: fixed;
+      z-index: 1001;
+      width: 100%;
+      background: red;
+      height: 160px;
+    }
+    </style>
+
+    <div class=fixed></div>
+
+    <ol>
+    <li>hi1</li><li>hi2</li><li>hi3</li><li>hi4</li><li>hi5</li><li>hi6</li><li>hi7</li><li>hi8</li>
+    <li id=target onclick="window.__clicked = true">hi9</li>
+    <li>hi10</li><li>hi11</li><li>hi12</li><li>hi13</li><li id=li14>hi14</li>
+    </ol>
+  `);
+  await page.$eval('#li14', e => e.scrollIntoView());
+  await page.click('#target');
+  expect(await page.evaluate(() => window['__clicked'])).toBe(true);
+});
+
 it('should click the button with deviceScaleFactor set', async ({browser, server}) => {
   const context = await browser.newContext({ viewport: { width: 400, height: 400 }, deviceScaleFactor: 5 });
   const page = await context.newPage();