From 685a60055efcf2847d592edb9851302e07bba022 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Wed, 24 Mar 2021 13:55:56 +0100
Subject: [PATCH 1/2] [PDFHistory] Reduce unnecessary duplication, by
 introducing a helper-method for validating pageNumbers

---
 web/pdf_history.js | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/web/pdf_history.js b/web/pdf_history.js
index 8c4d307ce..13275893c 100644
--- a/web/pdf_history.js
+++ b/web/pdf_history.js
@@ -205,13 +205,7 @@ class PDFHistory {
           `"${explicitDest}" is not a valid explicitDest parameter.`
       );
       return;
-    } else if (
-      !(
-        Number.isInteger(pageNumber) &&
-        pageNumber > 0 &&
-        pageNumber <= this.linkService.pagesCount
-      )
-    ) {
+    } else if (!this._isValidPage(pageNumber)) {
       // Allow an unset `pageNumber` if and only if the history is still empty;
       // please refer to the `this._destination.page = null;` comment above.
       if (pageNumber !== null || this._destination) {
@@ -281,13 +275,7 @@ class PDFHistory {
     if (!this._initialized) {
       return;
     }
-    if (
-      !(
-        Number.isInteger(pageNumber) &&
-        pageNumber > 0 &&
-        pageNumber <= this.linkService.pagesCount
-      )
-    ) {
+    if (!this._isValidPage(pageNumber)) {
       console.error(
         `PDFHistory.pushPage: "${pageNumber}" is not a valid page number.`
       );
@@ -479,6 +467,15 @@ class PDFHistory {
     this._pushOrReplaceState(position, forceReplace);
   }
 
+  /**
+   * @private
+   */
+  _isValidPage(val) {
+    return (
+      Number.isInteger(val) && val > 0 && val <= this.linkService.pagesCount
+    );
+  }
+
   /**
    * @private
    */
@@ -548,14 +545,7 @@ class PDFHistory {
     const nameddest = params.nameddest || "";
     let page = params.page | 0;
 
-    if (
-      !(
-        Number.isInteger(page) &&
-        page > 0 &&
-        page <= this.linkService.pagesCount
-      ) ||
-      (checkNameddest && nameddest.length > 0)
-    ) {
+    if (!this._isValidPage(page) || (checkNameddest && nameddest.length > 0)) {
       page = null;
     }
     return { hash, page, rotation: this.linkService.rotation };

From a10f87c7ead9514fbe030db6e2a6596c77deb820 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Wed, 24 Mar 2021 22:07:29 +0100
Subject: [PATCH 2/2] [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.