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

@@ -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(

test/assets/offscreenbuttons.html

@@ -6,18 +6,19 @@
     height: 20px;
   }
 
-  #btn0 { right: 0px; top: 0; }
-  #btn1 { right: -10px; top: 25px; }
-  #btn2 { right: -20px; top: 50px; }
-  #btn3 { right: -30px; top: 75px; }
-  #btn4 { right: -40px; top: 100px; }
-  #btn5 { right: -50px; top: 125px; }
+  #btn0 { right: -120px; top: 0; }
+  #btn1 { right: -110px; top: 25px; }
+  #btn2 { right: -100px; top: 50px; }
+  #btn3 { right: -90px; top: 75px; }
+  #btn4 { right: -80px; top: 100px; }
+  #btn5 { right: -70px; top: 125px; }
   #btn6 { right: -60px; top: 150px; }
-  #btn7 { right: -70px; top: 175px; }
-  #btn8 { right: -80px; top: 200px; }
-  #btn9 { right: -90px; top: 225px; }
-  #btn10 { right: -100px; top: 250px; }
-  #btn11 { right: -99.999px; top: 275px; }
+  #btn7 { right: -50px; top: 175px; }
+  #btn8 { right: -40px; top: 200px; }
+  #btn9 { right: -30px; top: 225px; }
+  #btn10 { right: -20px; top: 250px; }
+  #btn11 { right: -10px; top: 275px; }
+  #btn12 { right: 0; top: 300px; }
 </style>
 <button id=btn0>0</button>
 <button id=btn1>1</button>
@@ -31,6 +32,7 @@
 <button id=btn9>9</button>
 <button id=btn10>10</button>
 <button id=btn11>11</button>
+<button id=btn12>12</button>
 <script>
   for (const button of document.querySelectorAll('button')) {
     button.addEventListener('click', () => {

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).

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');