From 247733b879039dc7a4ab30d90cacf3e2bdb07ed0 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Wed, 13 Feb 2019 23:59:38 -0800
Subject: [PATCH] fix(firefox): enable more firefox tests (#4007)

---
 .../puppeteer-firefox/lib/ExecutionContext.js   | 17 ++++++++++++-----
 test/jshandle.spec.js                           | 13 ++++++++-----
 test/navigation.spec.js                         |  2 +-
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/experimental/puppeteer-firefox/lib/ExecutionContext.js b/experimental/puppeteer-firefox/lib/ExecutionContext.js
index 828924c5122..29cff77093d 100644
--- a/experimental/puppeteer-firefox/lib/ExecutionContext.js
+++ b/experimental/puppeteer-firefox/lib/ExecutionContext.js
@@ -59,11 +59,18 @@ class ExecutionContext {
         return {unserializableValue: 'NaN'};
       return {value: arg};
     });
-    const payload = await this._session.send('Page.evaluate', {
-      functionText,
-      args,
-      executionContextId: this._executionContextId
-    });
+    let payload = null;
+    try {
+      payload = await this._session.send('Page.evaluate', {
+        functionText,
+        args,
+        executionContextId: this._executionContextId
+      });
+    } catch (err) {
+      if (err instanceof TypeError && err.message === 'Converting circular structure to JSON')
+        err.message += ' Are you passing a nested JSHandle?';
+      throw err;
+    }
     return createHandle(this, payload.result, payload.exceptionDetails);
   }
 
diff --git a/test/jshandle.spec.js b/test/jshandle.spec.js
index ed0231741b6..a7ad8a6a3e6 100644
--- a/test/jshandle.spec.js
+++ b/test/jshandle.spec.js
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-module.exports.addTests = function({testRunner, expect}) {
+module.exports.addTests = function({testRunner, expect, CHROME}) {
   const {describe, xdescribe, fdescribe} = testRunner;
   const {it, fit, xit, it_fails_ffox} = testRunner;
   const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
 
   describe('Page.evaluateHandle', function() {
-    it_fails_ffox('should work', async({page, server}) => {
+    it('should work', async({page, server}) => {
       const windowHandle = await page.evaluateHandle(() => window);
       expect(windowHandle).toBeTruthy();
     });
@@ -34,7 +34,7 @@ module.exports.addTests = function({testRunner, expect}) {
       const isFive = await page.evaluate(e => Object.is(e, 5), aHandle);
       expect(isFive).toBeTruthy();
     });
-    it_fails_ffox('should warn on nested object handles', async({page, server}) => {
+    it('should warn on nested object handles', async({page, server}) => {
       const aHandle = await page.evaluateHandle(() => document.body);
       let error = null;
       await page.evaluateHandle(
@@ -86,11 +86,14 @@ module.exports.addTests = function({testRunner, expect}) {
       const json = await dateHandle.jsonValue();
       expect(json).toEqual({});
     });
-    it_fails_ffox('should throw for circular objects', async({page, server}) => {
+    it('should throw for circular objects', async({page, server}) => {
       const windowHandle = await page.evaluateHandle('window');
       let error = null;
       await windowHandle.jsonValue().catch(e => error = e);
-      expect(error.message).toContain('Object reference chain is too long');
+      if (CHROME)
+        expect(error.message).toContain('Object reference chain is too long');
+      else
+        expect(error.message).toContain('Object is not serializable');
     });
   });
 
diff --git a/test/navigation.spec.js b/test/navigation.spec.js
index a0c2efe1bd4..b016a46ef0c 100644
--- a/test/navigation.spec.js
+++ b/test/navigation.spec.js
@@ -61,7 +61,7 @@ module.exports.addTests = function({testRunner, expect, Errors, CHROME}) {
       else
         expect(error.message).toContain('NS_BINDING_ABORTED');
     });
-    it_fails_ffox('should navigate to empty page with domcontentloaded', async({page, server}) => {
+    it('should navigate to empty page with domcontentloaded', async({page, server}) => {
       const response = await page.goto(server.EMPTY_PAGE, {waitUntil: 'domcontentloaded'});
       expect(response.status()).toBe(200);
     });