From a10f87c7ead9514fbe030db6e2a6596c77deb820 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 24 Mar 2021 22:07:29 +0100 Subject: [PATCH] [PDFHistory] Correctly simulate an internal destination in the `pushPage`-method (PR 12493 follow-up) The intention, in PR 12493, was that the page we're adding to the browser history should behave as if it were a "regular" internal destination (to properly convey user intent). Unfortunately, since I didn't consider all the edge-cases correctly, it ended up behaving like a URL-hash instead which obviously wasn't intended. Note that currently this isn't a problem, however it can become an issue (in some cases) with upcoming re-factoring around `PDFHistory` and OpenAction support[1]. --- [1] I've started working on fixing the following TODO, which will require a couple of smaller tweaks here and there: https://github.com/mozilla/pdf.js/blob/9d0ce6e79fc70b1b47d6320c8ee18fd6cbe8467d/web/app.js#L1680-L1681 --- web/pdf_history.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/pdf_history.js b/web/pdf_history.js index 13275893c..042338082 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -292,6 +292,8 @@ class PDFHistory { } this._pushOrReplaceState({ + // Simulate an internal destination, for `this._tryPushCurrentPosition`: + dest: null, hash: `page=${pageNumber}`, page: pageNumber, rotation: this.linkService.rotation, @@ -458,7 +460,7 @@ class PDFHistory { // - contains an internal destination, since in this case we // cannot ensure that the document position has actually changed. // - was set through the user changing the hash of the document. - if (this._destination.dest || !this._destination.first) { + if (this._destination.dest !== undefined || !this._destination.first) { return; } // To avoid "flooding" the browser history, replace the current entry.