From 04d4faefc4b0b50a7210346c2f397bb8d7bfd73d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 5 Oct 2017 13:38:25 +0200 Subject: [PATCH 1/2] Remove the `this._currentUid` property from `PDFHistory`, since it's no longer needed Commit https://github.com/mozilla/pdf.js/pull/8885/commits/938dffb06b8dcd54ea5c7c2ad531d145b5fdaa5e, in PR 8885, removed the only actual usage of `this._currentUid` and it can thus be removed. --- web/pdf_history.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/web/pdf_history.js b/web/pdf_history.js index cca49c476..0edc0933f 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -109,7 +109,7 @@ class PDFHistory { this._currentHash = getCurrentHash(); this._numPositionUpdates = 0; - this._currentUid = this._uid = 0; + this._uid = 0; this._destination = null; this._position = null; @@ -266,7 +266,7 @@ class PDFHistory { let shouldReplace = forceReplace || !this._destination; let newState = { fingerprint: this.fingerprint, - uid: shouldReplace ? this._currentUid : this._uid, + uid: shouldReplace ? this._uid : (this._uid + 1), destination, }; @@ -392,8 +392,7 @@ class PDFHistory { delete destination.temporary; } this._destination = destination; - this._currentUid = uid; - this._uid = this._currentUid + 1; + this._uid = uid; // This should always be reset when `this._destination` is updated. this._numPositionUpdates = 0; } @@ -468,7 +467,7 @@ class PDFHistory { (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('CHROME') && state.chromecomState && !this._isValidState(state))) { // This case corresponds to the user changing the hash of the document. - this._currentUid = this._uid; + this._uid++; let { hash, page, rotation, } = parseCurrentHash(this.linkService); this._pushOrReplaceState({ hash, page, rotation, }, From e7721243399773fbc549ecb9ce83822df08dacde Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 5 Oct 2017 13:56:40 +0200 Subject: [PATCH 2/2] Fix a regression that (effectively) makes `PDFHistory.forward` a no-op *It appears that this accidentally broken in PR 8775.* Note that `PDFHistory.forward` is only used with certain named actions, and these aren't that commonly used, which ought to explain why this error managed to sneak in. Steps to reproduce the issue (and verify the fix): 1. Navigate to e.g. http://mirrors.ctan.org/info/lshort/english/lshort.pdf 2. Click on a couple of links, or outline items, such that the history is populated with a few entries. 3. In the console, execute `PDFViewerApplication.pdfHistory.back()` one or more times, thus navigating back to a previous viewer position. 4. In the console, execute `PDFViewerApplication.pdfHistory.forward() one or more times. At the last step above, no (forward) navigation happens with the current `master`; now compare with this patch. --- web/pdf_history.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/pdf_history.js b/web/pdf_history.js index 0edc0933f..7b5e11513 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -109,7 +109,7 @@ class PDFHistory { this._currentHash = getCurrentHash(); this._numPositionUpdates = 0; - this._uid = 0; + this._uid = this._maxUid = 0; this._destination = null; this._position = null; @@ -245,7 +245,7 @@ class PDFHistory { return; } let state = window.history.state; - if (this._isValidState(state) && state.uid < (this._uid - 1)) { + if (this._isValidState(state) && state.uid < this._maxUid) { window.history.forward(); } } @@ -286,6 +286,8 @@ class PDFHistory { window.history.replaceState(newState, '', document.URL); } } else { + this._maxUid = this._uid; + if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('FIREFOX || MOZCENTRAL')) { // Providing the third argument causes a SecurityError for file:// URLs.