From 54410d5e4189d1773db4df1d20bbcd93cafae756 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 4 May 2022 11:35:04 +0200 Subject: [PATCH 1/2] Simplify the signature of `BaseViewer._scrollIntoView` and make the method private In PR 14112 usage of this *internal* method was reduced, and it thus can't hurt to clean-up things a little bit more. Note in particular that we can simplify the call-sites by directly passing in the already available `PDFPageView`-instance, since the `id`-property those contain can replace the previous `pageNumber`-parameter[1]. Given that the method name has always been prefixed with an underscore it was thus never intended to be "public", hence we can now enforce that with modern ECMAScript features. --- [1] There's already a bunch of other spots, throughout the viewer-code, where we assume that the `PDFPageView.id`-property contains proper page *numbers* (and not e.g. indices); note how we initialize the `PDFPageView`-instances in the `BaseViewer.setDocument`-method. --- web/base_viewer.js | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 284deb331..042355128 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -943,10 +943,12 @@ class BaseViewer { this.update(); } - _scrollIntoView({ pageDiv, pageNumber, pageSpot = null }) { + #scrollIntoView(pageView, pageSpot = null) { + const { div, id } = pageView; + if (this._scrollMode === ScrollMode.PAGE) { // Ensure that `this._currentPageNumber` is correct. - this._setCurrentPageNumber(pageNumber); + this._setCurrentPageNumber(id); this.#ensurePageViewVisible(); // Ensure that rendering always occurs, to avoid showing a blank page, @@ -955,8 +957,8 @@ class BaseViewer { } if (!pageSpot && !this.isInPresentationMode) { - const left = pageDiv.offsetLeft + pageDiv.clientLeft; - const right = left + pageDiv.clientWidth; + const left = div.offsetLeft + div.clientLeft, + right = left + div.clientWidth; const { scrollLeft, clientWidth } = this.container; if ( this._scrollMode === ScrollMode.HORIZONTAL || @@ -966,7 +968,7 @@ class BaseViewer { pageSpot = { left: 0, top: 0 }; } } - scrollIntoView(pageDiv, pageSpot); + scrollIntoView(div, pageSpot); } /** @@ -1120,15 +1122,13 @@ class BaseViewer { * Refreshes page view: scrolls to the current page and updates the scale. */ #resetCurrentPageView() { - const pageNumber = this._currentPageNumber; + const pageView = this._pages[this._currentPageNumber - 1]; if (this.isInPresentationMode) { // Fixes the case when PDF has different page sizes. this._setScale(this._currentScaleValue, true); } - - const pageView = this._pages[pageNumber - 1]; - this._scrollIntoView({ pageDiv: pageView.div, pageNumber }); + this.#scrollIntoView(pageView); } /** @@ -1272,10 +1272,7 @@ class BaseViewer { } if (scale === "page-fit" && !destArray[4]) { - this._scrollIntoView({ - pageDiv: pageView.div, - pageNumber, - }); + this.#scrollIntoView(pageView); return; } @@ -1293,11 +1290,7 @@ class BaseViewer { left = Math.max(left, 0); top = Math.max(top, 0); } - this._scrollIntoView({ - pageDiv: pageView.div, - pageSpot: { left, top }, - pageNumber, - }); + this.#scrollIntoView(pageView, /* pageSpot = */ { left, top }); } _updateLocation(firstPage) { From 4fffab4ad3221e7aa93c1f7bee1b0c51561d069f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 5 May 2022 11:15:45 +0200 Subject: [PATCH 2/2] Add (basic) support for Spread modes in PresentationMode (issue 14749) After recent changes, adding *basic* Spread mode support in PresentationMode has now become reasonably straightforward. However, documents with *varying* page sizes are non-trivial to handle and would require re-writing (or at least re-factoring) a bunch of the zooming-code. Hence this PR *purposely* only allow Spread modes to be used, in PresentationMode, for documents where all pages have exactly the same size. While this obviously isn't a fully complete solution, it will however cover the vast majority of all documents and should hopefully be deemed good enough for now. --- web/pdf_presentation_mode.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index 912d3a202..7da3719cb 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -78,9 +78,20 @@ class PDFPresentationMode { pageNumber: pdfViewer.currentPageNumber, scaleValue: pdfViewer.currentScaleValue, scrollMode: pdfViewer.scrollMode, - spreadMode: pdfViewer.spreadMode, + spreadMode: null, }; + if ( + pdfViewer.spreadMode !== SpreadMode.NONE && + !(pdfViewer.pageViewsReady && pdfViewer.hasEqualPageSizes) + ) { + console.warn( + "Ignoring Spread modes when entering PresentationMode, " + + "since the document may contain varying page sizes." + ); + this.#args.spreadMode = pdfViewer.spreadMode; + } + try { await promise; return true; @@ -151,7 +162,9 @@ class PDFPresentationMode { // Presentation Mode, by waiting until fullscreen mode in enabled. setTimeout(() => { this.pdfViewer.scrollMode = ScrollMode.PAGE; - this.pdfViewer.spreadMode = SpreadMode.NONE; + if (this.#args.spreadMode !== null) { + this.pdfViewer.spreadMode = SpreadMode.NONE; + } this.pdfViewer.currentPageNumber = this.#args.pageNumber; this.pdfViewer.currentScaleValue = "page-fit"; }, 0); @@ -177,7 +190,9 @@ class PDFPresentationMode { this.#notifyStateChange(PresentationModeState.NORMAL); this.pdfViewer.scrollMode = this.#args.scrollMode; - this.pdfViewer.spreadMode = this.#args.spreadMode; + if (this.#args.spreadMode !== null) { + this.pdfViewer.spreadMode = this.#args.spreadMode; + } this.pdfViewer.currentScaleValue = this.#args.scaleValue; this.pdfViewer.currentPageNumber = pageNumber; this.#args = null;