From b58d3199260cad1700187e7b5b8924ee79d011e3 Mon Sep 17 00:00:00 2001
From: JoelEinbinder <joel.einbinder@gmail.com>
Date: Tue, 7 Nov 2017 13:54:40 -0800
Subject: [PATCH] fix(Page.click): throw a meaningful message for invisible
 elements (#1309)

This patch starts throwing a meaningful error message when trying to click the hidden
node.

References #1294
---
 docs/api.md          |  2 +-
 lib/ElementHandle.js | 19 ++++++++++++-------
 test/test.js         | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/docs/api.md b/docs/api.md
index cfa74d0ba4a..ee1671cdcd7 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -1786,7 +1786,7 @@ The method runs `element.querySelectorAll` within the page. If no elements match
     - width <[number]> the width of the element in pixels.
     - height <[number]> the height of the element in pixels.
 
-This method returns the bounding box of the element (relative to the main frame), or `null` if element is detached from dom.
+This method returns the bounding box of the element (relative to the main frame), or `null` if the element is not visible.
 
 #### elementHandle.click([options])
 - `options` <[Object]>
diff --git a/lib/ElementHandle.js b/lib/ElementHandle.js
index b83e7dbfacf..3a70676fe09 100644
--- a/lib/ElementHandle.js
+++ b/lib/ElementHandle.js
@@ -15,7 +15,7 @@
  */
 const path = require('path');
 const {JSHandle} = require('./ExecutionContext');
-const {helper} = require('./helper');
+const {helper, debugError} = require('./helper');
 
 class ElementHandle extends JSHandle {
   /**
@@ -59,6 +59,8 @@ class ElementHandle extends JSHandle {
   async _visibleCenter() {
     await this._scrollIntoViewIfNeeded();
     const box = await this.boundingBox();
+    if (!box)
+      throw new Error('Node is not visible');
     return {
       x: box.x + box.width / 2,
       y: box.y + box.height / 2
@@ -116,16 +118,17 @@ class ElementHandle extends JSHandle {
   }
 
   /**
-   * @return {!Promise<{x: number, y: number, width: number, height: number}>}
+   * @return {!Promise<?{x: number, y: number, width: number, height: number}>}
    */
   async boundingBox() {
-    const {model} = await this._client.send('DOM.getBoxModel', {
+    const result = await this._client.send('DOM.getBoxModel', {
       objectId: this._remoteObject.objectId
-    });
-    if (!model)
-      throw new Error('Node is detached from document');
+    }).catch(error => void debugError(error));
 
-    const quad = model.border;
+    if (!result)
+      return null;
+
+    const quad = result.model.border;
     const x = Math.min(quad[0], quad[2], quad[4], quad[6]);
     const y = Math.min(quad[1], quad[3], quad[5], quad[7]);
     const width = Math.max(quad[0], quad[2], quad[4], quad[6]) - x;
@@ -142,6 +145,8 @@ class ElementHandle extends JSHandle {
   async screenshot(options = {}) {
     await this._scrollIntoViewIfNeeded();
     const boundingBox = await this.boundingBox();
+    if (!boundingBox)
+      throw new Error('Node is not visible');
 
     return await this._page.screenshot(Object.assign({}, {
       clip: boundingBox
diff --git a/test/test.js b/test/test.js
index 53f86d2a977..a47eeb83998 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1688,6 +1688,11 @@ describe('Page', function() {
       const box = await elementHandle.boundingBox();
       expect(box).toEqual({ x: 28, y: 260, width: 264, height: 18 });
     }));
+    it('should return null for invisible elements', SX(async function() {
+      await page.setContent('<div style="display:none">hi</div>');
+      const element = await page.$('div');
+      expect(await element.boundingBox()).toBe(null);
+    }));
   });
 
   describe('ElementHandle.click', function() {
@@ -1718,6 +1723,26 @@ describe('Page', function() {
       await button.click().catch(err => error = err);
       expect(error.message).toBe('Node is detached from document');
     }));
+    it('should throw for hidden nodes', SX(async function() {
+      await page.goto(PREFIX + '/input/button.html');
+      const button = await page.$('button');
+      await page.evaluate(button => button.style.display = 'none', button);
+      const error = await button.click().catch(err => err);
+      expect(error.message).toBe('Node is not visible');
+    }));
+    it('should throw for recursively hidden nodes', SX(async function() {
+      await page.goto(PREFIX + '/input/button.html');
+      const button = await page.$('button');
+      await page.evaluate(button => button.parentElement.style.display = 'none', button);
+      const error = await button.click().catch(err => err);
+      expect(error.message).toBe('Node is not visible');
+    }));
+    it('should throw for <br> elements', SX(async function() {
+      await page.setContent('hello<br>goodbye');
+      const br = await page.$('br');
+      const error = await br.click().catch(err => err);
+      expect(error.message).toBe('Node is not visible');
+    }));
   });
 
   describe('ElementHandle.hover', function() {
@@ -2583,6 +2608,14 @@ describe('Page', function() {
       expect(await page.evaluate(() => window.innerWidth)).toBe(375);
       expect(await page.evaluate(() => navigator.userAgent)).toContain('Safari');
     }));
+    it('should support clicking', SX(async function() {
+      await page.emulate(iPhone);
+      await page.goto(PREFIX + '/input/button.html');
+      const button = await page.$('button');
+      await page.evaluate(button => button.style.marginTop = '200px', button);
+      await button.click();
+      expect(await page.evaluate(() => result)).toBe('Clicked');
+    }));
   });
 
   describe('Page.emulateMedia', function() {