From af1bb04662ca54f6ef98618bafeaeb0bedce02e7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 7 May 2020 13:53:07 +0200 Subject: [PATCH] Attempt to respect the "zoom" hash parameter, even when the "nameddest" parameter is present (issue 11875) Given that the `PDFLinkService.setHash` method itself if completely synchronous, moving the handling of "nameddest" to occur last *shouldn't* cause any problems (famous last words). This way the destination will still override any previous parameter, such as e.g. the "page", as expected. Furthermore, given that the `PDFLinkService.navigateTo` method is asynchronous that should provide additional guarantees that the "nameddest" parameter is always respected. As sort-of expected, this fairly innocent looking change also required some tweaks in the `PDFHistory` to prevent dummy history entires upon document load (only an issue when both "page" *and* "nameddest" parameters are provided in the hash). --- web/pdf_history.js | 14 ++++++++++---- web/pdf_link_service.js | 9 +++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/web/pdf_history.js b/web/pdf_history.js index 96f240bfc..8e2b148ad 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -117,7 +117,9 @@ class PDFHistory { this._position = null; if (!this._isValidState(state, /* checkReload = */ true) || resetHistory) { - const { hash, page, rotation } = this._parseCurrentHash(); + const { hash, page, rotation } = this._parseCurrentHash( + /* checkNameddest = */ true + ); if (!hash || reInitialized || resetHistory) { // Ensure that the browser history is reset on PDF document load. @@ -490,16 +492,20 @@ class PDFHistory { /** * @private */ - _parseCurrentHash() { + _parseCurrentHash(checkNameddest = false) { const hash = unescape(getCurrentHash()).substring(1); - let page = parseQueryString(hash).page | 0; + const params = parseQueryString(hash); + + const nameddest = params.nameddest || ""; + let page = params.page | 0; if ( !( Number.isInteger(page) && page > 0 && page <= this.linkService.pagesCount - ) + ) || + (checkNameddest && nameddest.length > 0) ) { page = null; } diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js index c20343780..4b96fbafd 100644 --- a/web/pdf_link_service.js +++ b/web/pdf_link_service.js @@ -233,10 +233,6 @@ class PDFLinkService { }); } // borrowing syntax from "Parameters for Opening PDF Files" - if ("nameddest" in params) { - this.navigateTo(params.nameddest); - return; - } if ("page" in params) { pageNumber = params.page | 0 || 1; } @@ -308,6 +304,11 @@ class PDFLinkService { mode: params.pagemode, }); } + // Ensure that this parameter is *always* handled last, in order to + // guarantee that it won't be overridden (e.g. by the "page" parameter). + if ("nameddest" in params) { + this.navigateTo(params.nameddest); + } } else { // Named (or explicit) destination. dest = unescape(hash);