From 295716f49646877a023c5f5a39211f2ae29439e4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 3 Oct 2020 18:08:30 +0200 Subject: [PATCH] Support adding pages, in addition to regular destinations, to the browser history and use it with thumbnails (issue 12440) While the referenced issue could very well be seen as an edge-case, this patch adds support for updating of the browser history when interacting with the thumbnails in the sidebar (assuming we want to do this). The main reason for adding the history implementation in the first place, was to simplify navigating back to a previous position in the document when named/explicit destinations are used (e.g. when clicking on "links" or when using the outline in the sidebar). As such, it never really crossed by mind to update the browser history when the thumbnails are used. However, a user clicking on thumbnails could be regarded as a pretty strong indication of user-intent w.r.t. navigation in the document, hence I suppose that updating the browser history in this particular case probably won't hurt. --- web/interfaces.js | 10 ++++++++ web/pdf_history.js | 49 +++++++++++++++++++++++++++++++++++++++ web/pdf_link_service.js | 34 +++++++++++++++++++++++++++ web/pdf_thumbnail_view.js | 2 +- 4 files changed, 94 insertions(+), 1 deletion(-) diff --git a/web/interfaces.js b/web/interfaces.js index 7469c9385..f13b11e5f 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -58,6 +58,11 @@ class IPDFLinkService { */ async goToDestination(dest) {} + /** + * @param {number} pageNumber - The page number. + */ + goToPage(pageNumber) {} + /** * @param dest - The PDF destination object. * @returns {string} The hyperlink to the PDF object. @@ -108,6 +113,11 @@ class IPDFHistory { */ push({ namedDest = null, explicitDest, pageNumber }) {} + /** + * @param {number} pageNumber + */ + pushPage(pageNumber) {} + pushCurrentPosition() {} back() {} diff --git a/web/pdf_history.js b/web/pdf_history.js index 097beb64e..696595a5d 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -271,6 +271,55 @@ class PDFHistory { } } + /** + * Push a page to the browser history; generally the `push` method should be + * used instead. + * @param {number} pageNumber + */ + pushPage(pageNumber) { + if (!this._initialized) { + return; + } + if ( + !( + Number.isInteger(pageNumber) && + pageNumber > 0 && + pageNumber <= this.linkService.pagesCount + ) + ) { + console.error( + `PDFHistory.pushPage: "${pageNumber}" is not a valid page number.` + ); + return; + } + + if (this._destination && this._destination.page === pageNumber) { + // When the new page is identical to the one in `this._destination`, we + // don't want to add a potential duplicate entry in the browser history. + return; + } + if (this._popStateInProgress) { + return; + } + + this._pushOrReplaceState({ + hash: `page=${pageNumber}`, + page: pageNumber, + rotation: this.linkService.rotation, + }); + + if (!this._popStateInProgress) { + // Prevent the browser history from updating while the new page is + // being scrolled into view, to avoid potentially inconsistent state. + this._popStateInProgress = true; + // We defer the resetting of `this._popStateInProgress`, to account for + // e.g. zooming occuring when the new page is being navigated to. + Promise.resolve().then(() => { + this._popStateInProgress = false; + }); + } + } + /** * Push the current position to the browser history. */ diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js index 7aba5dc73..434d6c757 100644 --- a/web/pdf_link_service.js +++ b/web/pdf_link_service.js @@ -200,6 +200,35 @@ class PDFLinkService { this._goToDestinationHelper(dest, namedDest, explicitDest); } + /** + * This method will, when available, also update the browser history. + * + * @param {number} pageNumber - The page number. + */ + goToPage(pageNumber) { + if ( + !( + Number.isInteger(pageNumber) && + pageNumber > 0 && + pageNumber <= this.pagesCount + ) + ) { + console.error( + `PDFLinkService.goToPage: "${pageNumber}" is not a valid page number.` + ); + return; + } + + if (this.pdfHistory) { + // Update the browser history before scrolling the new page into view, + // to be able to accurately capture the current document position. + this.pdfHistory.pushCurrentPosition(); + this.pdfHistory.pushPage(pageNumber); + } + + this.pdfViewer.scrollPageIntoView({ pageNumber }); + } + /** * @param {string|Array} dest - The PDF destination object. * @returns {string} The hyperlink to the PDF object. @@ -524,6 +553,11 @@ class SimpleLinkService { */ async goToDestination(dest) {} + /** + * @param {number} pageNumber - The page number. + */ + goToPage(pageNumber) {} + /** * @param dest - The PDF destination object. * @returns {string} The hyperlink to the PDF object. diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 9e70626df..bf325ec6d 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -138,7 +138,7 @@ class PDFThumbnailView { anchor.title = msg; }); anchor.onclick = function () { - linkService.page = id; + linkService.goToPage(id); return false; }; this.anchor = anchor;