fix(scrollIntoView): ensure similar behavior across browsers, handle errors (#2599)

browsers.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1108"
+      "revision": "1111"
     },
     {
       "name": "webkit",

src/dom.ts

@@ -213,7 +213,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   async scrollIntoViewIfNeeded() {
-    throwIfNotConnected(await this._scrollRectIntoViewIfNeeded());
+    const result = await this._scrollRectIntoViewIfNeeded();
+    if (result === 'notvisible')
+      throw new Error('Element is not visible');
+    throwIfNotConnected(result);
   }
 
   private async _clickablePoint(): Promise<types.Point | 'notvisible' | 'notinviewport'> {

src/firefox/ffPage.ts

@@ -418,6 +418,8 @@ export class FFPage implements PageDelegate {
     }).then(() => 'done' as const).catch(e => {
       if (e instanceof Error && e.message.includes('Node is detached from document'))
         return 'notconnected';
+      if (e instanceof Error && e.message.includes('Node does not have a layout object'))
+        return 'notvisible';
       throw e;
     });
   }

test/elementhandle.spec.js

@@ -323,6 +323,41 @@ describe('ElementHandle.scrollIntoViewIfNeeded', function() {
       await page.evaluate(() => window.scrollTo(0, 0));
     }
   });
+  it('should throw for detached element', async({page, server}) => {
+    await page.setContent('<div>Hello</div>');
+    const div = await page.$('div');
+    await div.evaluate(div => div.remove());
+    const error = await div.scrollIntoViewIfNeeded().catch(e => e);
+    expect(error.message).toContain('Element is not attached to the DOM');
+  });
+  it('should throw for display:none element', async({page, server}) => {
+    await page.setContent('<div style="display:none">Hello</div>');
+    const div = await page.$('div');
+    const error = await div.scrollIntoViewIfNeeded().catch(e => e);
+    expect(error.message).toContain('Element is not visible');
+  });
+  it('should throw for nested display:none element', async({page, server}) => {
+    await page.setContent('<span style="display:none"><div>Hello</div></span>');
+    const div = await page.$('div');
+    const error = await div.scrollIntoViewIfNeeded().catch(e => e);
+    expect(error.message).toContain('Element is not visible');
+  });
+  it('should throw for display:contents element', async({page, server}) => {
+    await page.setContent('<div style="display:contents">Hello</div>');
+    const div = await page.$('div');
+    const error = await div.scrollIntoViewIfNeeded().catch(e => e);
+    expect(error.message).toContain('Element is not visible');
+  });
+  it('should scroll a zero-sized element', async({page, server}) => {
+    await page.setContent('<br>');
+    const br = await page.$('br');
+    await br.scrollIntoViewIfNeeded();
+  });
+  it('should scroll a visibility:hidden element', async({page, server}) => {
+    await page.setContent('<div style="visibility:hidden">Hello</div>');
+    const div = await page.$('div');
+    await div.scrollIntoViewIfNeeded();
+  });
 });
 
 describe('ElementHandle.fill', function() {