From 9d6eaadba762eec8b5b81f4ee6e69a08ce4d50b4 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 1 Jul 2020 19:17:27 -0700
Subject: [PATCH] fix(navigation): ensure that goBack/goForward work with file
 urls (#2792)

---
 browsers.json           |  2 +-
 src/firefox/ffPage.ts   |  8 ++++----
 src/firefox/protocol.ts | 11 +++--------
 test/navigation.spec.js | 28 +++++++++++++++++++++++++++-
 test/utils.js           |  2 ++
 5 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/browsers.json b/browsers.json
index a55b1915f0..2a4d08b9f9 100644
--- a/browsers.json
+++ b/browsers.json
@@ -7,7 +7,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1116"
+      "revision": "1117"
     },
     {
       "name": "webkit",
diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts
index 49221f97ca..48c9a40f66 100644
--- a/src/firefox/ffPage.ts
+++ b/src/firefox/ffPage.ts
@@ -315,13 +315,13 @@ export class FFPage implements PageDelegate {
   }
 
   async goBack(): Promise<boolean> {
-    const { navigationId } = await this._session.send('Page.goBack', { frameId: this._page.mainFrame()._id });
-    return navigationId !== null;
+    const { success } = await this._session.send('Page.goBack', { frameId: this._page.mainFrame()._id });
+    return success;
   }
 
   async goForward(): Promise<boolean> {
-    const { navigationId } = await this._session.send('Page.goForward', { frameId: this._page.mainFrame()._id });
-    return navigationId !== null;
+    const { success } = await this._session.send('Page.goForward', { frameId: this._page.mainFrame()._id });
+    return success;
   }
 
   async evaluateOnNewDocument(source: string): Promise<void> {
diff --git a/src/firefox/protocol.ts b/src/firefox/protocol.ts
index 74c1e9e852..8ed71a5244 100644
--- a/src/firefox/protocol.ts
+++ b/src/firefox/protocol.ts
@@ -461,23 +461,18 @@ export module Protocol {
       frameId: string;
     };
     export type goBackReturnValue = {
-      navigationId: string|null;
-      navigationURL: string|null;
+      success: boolean;
     };
     export type goForwardParameters = {
       frameId: string;
     };
     export type goForwardReturnValue = {
-      navigationId: string|null;
-      navigationURL: string|null;
+      success: boolean;
     };
     export type reloadParameters = {
       frameId: string;
     };
-    export type reloadReturnValue = {
-      navigationId: string;
-      navigationURL: string;
-    };
+    export type reloadReturnValue = void;
     export type getBoundingBoxParameters = {
       frameId: string;
       objectId: string;
diff --git a/test/navigation.spec.js b/test/navigation.spec.js
index 87f4e0c717..16340e4a8a 100644
--- a/test/navigation.spec.js
+++ b/test/navigation.spec.js
@@ -18,7 +18,7 @@
 const utils = require('./utils');
 const path = require('path');
 const url = require('url');
-const {FFOX, CHROMIUM, WEBKIT, MAC, WIN, CHANNEL} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, ASSETS_DIR, MAC, WIN, CHANNEL} = utils.testOptions(browserType);
 
 describe('Page.goto', function() {
   it('should work', async({page, server}) => {
@@ -896,6 +896,32 @@ describe('Page.goBack', function() {
     await page.goForward();
     expect(page.url()).toBe(server.PREFIX + '/first.html');
   });
+  it.fail(WEBKIT && MAC)('should work for file urls', async ({page, server}) => {
+    // WebKit embedder fails to go back/forward to the file url.
+    const url1 = WIN
+        ? 'file:///' + path.join(ASSETS_DIR, 'empty.html').replace(/\\/g, '/')
+        : 'file://' + path.join(ASSETS_DIR, 'empty.html');
+    const url2 = server.EMPTY_PAGE;
+    await page.goto(url1);
+    await page.setContent(`<a href='${url2}'>url2</a>`);
+    expect(page.url().toLowerCase()).toBe(url1.toLowerCase());
+
+    await page.click('a');
+    expect(page.url()).toBe(url2);
+
+    await page.goBack();
+    expect(page.url().toLowerCase()).toBe(url1.toLowerCase());
+    // Should be able to evaluate in the new context, and
+    // not reach for the old cross-process one.
+    expect(await page.evaluate(() => window.scrollX)).toBe(0);
+    // Should be able to screenshot.
+    await page.screenshot();
+
+    await page.goForward();
+    expect(page.url()).toBe(url2);
+    expect(await page.evaluate(() => window.scrollX)).toBe(0);
+    await page.screenshot();
+  });
 });
 
 describe('Frame.goto', function() {
diff --git a/test/utils.js b/test/utils.js
index e9da6a5eac..f396ec7743 100644
--- a/test/utils.js
+++ b/test/utils.js
@@ -190,6 +190,7 @@ const utils = module.exports = {
   testOptions(browserType) {
     const GOLDEN_DIR = path.join(__dirname, 'golden-' + browserType.name());
     const OUTPUT_DIR = path.join(__dirname, 'output-' + browserType.name());
+    const ASSETS_DIR = path.join(__dirname, 'assets');
     return {
       FFOX: browserType.name() === 'firefox',
       WEBKIT: browserType.name() === 'webkit',
@@ -200,6 +201,7 @@ const utils = module.exports = {
       browserType,
       GOLDEN_DIR,
       OUTPUT_DIR,
+      ASSETS_DIR,
       USES_HOOKS: !!process.env.PWCHANNEL,
       CHANNEL: !!process.env.PWCHANNEL,
     };