From 08045babed1d0dadc1b685042507fd82bc08a7c9 Mon Sep 17 00:00:00 2001
From: abozhilov <asen.bozhilov@gmail.com>
Date: Thu, 7 Jul 2022 13:55:17 +0000
Subject: [PATCH] fix(page): fix page.#scrollIntoViewIfNeeded method

This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: #8627, #1805
---
 src/common/ElementHandle.ts    | 68 +++++++++++++++++-----------------
 test/src/click.spec.ts         |  5 ++-
 test/src/elementhandle.spec.ts |  2 +-
 3 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/src/common/ElementHandle.ts b/src/common/ElementHandle.ts
index d40406000ae46..6e56b7d1ff4ba 100644
--- a/src/common/ElementHandle.ts
+++ b/src/common/ElementHandle.ts
@@ -240,50 +240,52 @@ export class ElementHandle<
 
   async #scrollIntoViewIfNeeded(this: ElementHandle<Element>): Promise<void> {
     const error = await this.evaluate(
-      async (element, pageJavascriptEnabled): Promise<string | false> => {
+      async (element): Promise<string | undefined> => {
         if (!element.isConnected) {
           return 'Node is detached from document';
         }
         if (element.nodeType !== Node.ELEMENT_NODE) {
           return 'Node is not of type HTMLElement';
         }
-        // force-scroll if page's javascript is disabled.
-        if (!pageJavascriptEnabled) {
-          element.scrollIntoView({
-            block: 'center',
-            inline: 'center',
-            // @ts-expect-error Chrome still supports behavior: instant but
-            // it's not in the spec so TS shouts We don't want to make this
-            // breaking change in Puppeteer yet so we'll ignore the line.
-            behavior: 'instant',
-          });
-          return false;
-        }
-        const visibleRatio = await new Promise(resolve => {
-          const observer = new IntersectionObserver(entries => {
-            resolve(entries[0]!.intersectionRatio);
-            observer.disconnect();
-          });
-          observer.observe(element);
-        });
-        if (visibleRatio !== 1.0) {
-          element.scrollIntoView({
-            block: 'center',
-            inline: 'center',
-            // @ts-expect-error Chrome still supports behavior: instant but
-            // it's not in the spec so TS shouts We don't want to make this
-            // breaking change in Puppeteer yet so we'll ignore the line.
-            behavior: 'instant',
-          });
-        }
-        return false;
-      },
-      this.#page.isJavaScriptEnabled()
+        return;
+      }
     );
 
     if (error) {
       throw new Error(error);
     }
+
+    try {
+      await this._client.send('DOM.scrollIntoViewIfNeeded', {
+        objectId: this._remoteObject.objectId,
+      });
+    } catch (_err) {
+      // Fallback to Element.scrollIntoView if DOM.scrollIntoViewIfNeeded is not supported
+      await this.evaluate(
+        async (element, pageJavascriptEnabled): Promise<void> => {
+          const visibleRatio = async () => {
+            return await new Promise(resolve => {
+              const observer = new IntersectionObserver(entries => {
+                resolve(entries[0]!.intersectionRatio);
+                observer.disconnect();
+              });
+              observer.observe(element);
+            });
+          };
+          if (!pageJavascriptEnabled || (await visibleRatio()) !== 1.0) {
+            element.scrollIntoView({
+              block: 'center',
+              inline: 'center',
+              // @ts-expect-error Chrome still supports behavior: instant but
+              // it's not in the spec so TS shouts We don't want to make this
+              // breaking change in Puppeteer yet so we'll ignore the line.
+              behavior: 'instant',
+            });
+          }
+        },
+        this.#page.isJavaScriptEnabled()
+      );
+    }
   }
 
   async #getOOPIFOffsets(
diff --git a/test/src/click.spec.ts b/test/src/click.spec.ts
index 9d7880e9d1904..bea173712862e 100644
--- a/test/src/click.spec.ts
+++ b/test/src/click.spec.ts
@@ -164,7 +164,10 @@ describe('Page.click', function () {
     await page.goto(server.PREFIX + '/offscreenbuttons.html');
     const messages: any[] = [];
     page.on('console', msg => {
-      return messages.push(msg.text());
+      if (msg.type() === 'log') {
+        return messages.push(msg.text());
+      }
+      return;
     });
     for (let i = 0; i < 11; ++i) {
       // We might've scrolled to click a button - reset to (0, 0).
diff --git a/test/src/elementhandle.spec.ts b/test/src/elementhandle.spec.ts
index d19d34a3e6ea8..3a11c2b152463 100644
--- a/test/src/elementhandle.spec.ts
+++ b/test/src/elementhandle.spec.ts
@@ -203,7 +203,7 @@ describe('ElementHandle specs', function () {
         })
       ).toBe(true);
     });
-    it('should work for TextNodes', async () => {
+    it('should not work for TextNodes', async () => {
       const {page, server} = getTestState();
 
       await page.goto(server.PREFIX + '/input/button.html');