From 0ac1fba2f9f6e3f40935dbf1e1747aeb0258a6ab Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 11 Aug 2017 13:54:30 +0200
Subject: [PATCH] Prevent `PDFHistory._tryPushCurrentPosition()` from
 effectively becoming a no-op if the document hash contains an
 invalid/non-existent destination

By using the (heuristic) `POSITION_UPDATED_THRESHOLD` constant, we can ensure that the current document position will be added to the browser history when a sufficiently "large" number of `updateviewarea` events have been dispatched.
---
 web/pdf_history.js | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/web/pdf_history.js b/web/pdf_history.js
index 8c4a208a2..7d90fe001 100644
--- a/web/pdf_history.js
+++ b/web/pdf_history.js
@@ -18,6 +18,8 @@ import { getGlobalEventBus } from './dom_events';
 
 // Heuristic value used when force-resetting `this._blockHashChange`.
 const HASH_CHANGE_TIMEOUT = 1000; // milliseconds
+// Heuristic value used when adding the current position to the browser history.
+const POSITION_UPDATED_THRESHOLD = 50;
 // Heuristic value used when adding a temporary position to the browser history.
 const UPDATE_VIEWAREA_TIMEOUT = 2000; // milliseconds
 
@@ -63,12 +65,16 @@ class PDFHistory {
 
     this._boundEvents = Object.create(null);
     this._isViewerInPresentationMode = false;
+    this._isPagesLoaded = false;
 
-    // Ensure that we don't miss a 'presentationmodechanged' event, by
-    // registering the listener immediately.
+    // Ensure that we don't miss either a 'presentationmodechanged' or a
+    // 'pagesloaded' event, by registering the listeners immediately.
     this.eventBus.on('presentationmodechanged', (evt) => {
       this._isViewerInPresentationMode = evt.active || evt.switchInProgress;
     });
+    this.eventBus.on('pagesloaded', (evt) => {
+      this._isPagesLoaded = !!evt.pagesCount;
+    });
   }
 
   /**
@@ -97,6 +103,7 @@ class PDFHistory {
     this._popStateInProgress = false;
     this._blockHashChange = 0;
     this._currentHash = getCurrentHash();
+    this._numPositionUpdates = 0;
 
     this._currentUid = this._uid = 0;
     this._destination = null;
@@ -299,7 +306,9 @@ class PDFHistory {
     if (this._destination.hash === position.hash) {
       return; // The current document position has not changed.
     }
-    if (!this._destination.page) {
+    if (!this._destination.page &&
+        (POSITION_UPDATED_THRESHOLD <= 0 ||
+         this._numPositionUpdates <= POSITION_UPDATED_THRESHOLD)) {
       // `this._destination` was set through the user changing the hash of
       // the document. Do not add `this._position` to the browser history,
       // to avoid "flooding" it with lots of (nearly) identical entries,
@@ -357,6 +366,8 @@ class PDFHistory {
     this._destination = destination;
     this._currentUid = uid;
     this._uid = this._currentUid + 1;
+    // This should always be reset when `this._destination` is updated.
+    this._numPositionUpdates = 0;
   }
 
   /**
@@ -379,6 +390,19 @@ class PDFHistory {
       return;
     }
 
+    if (POSITION_UPDATED_THRESHOLD > 0 && this._isPagesLoaded &&
+        this._destination && !this._destination.page) {
+      // If the current destination was set through the user changing the hash
+      // of the document, we will usually not try to push the current position
+      // to the browser history; see `this._tryPushCurrentPosition()`.
+      //
+      // To prevent `this._tryPushCurrentPosition()` from effectively being
+      // reduced to a no-op in this case, we will assume that the position
+      // *did* in fact change if the 'updateviewarea' event was dispatched
+      // more than `POSITION_UPDATED_THRESHOLD` times.
+      this._numPositionUpdates++;
+    }
+
     if (UPDATE_VIEWAREA_TIMEOUT > 0) {
       // When closing the browser, a 'pagehide' event will be dispatched which
       // *should* allow us to push the current position to the browser history.