From 5ef294b3a757e222b5883ea72b886dae0fffb319 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 5 Apr 2022 13:32:09 +0200 Subject: [PATCH 1/2] Remove the `BaseViewer._getCurrentVisiblePage` helper method This method was originally added specifically to work-around bugs/issues related to PresentationMode in Google Chrome. Note that prior to PR 14112 we were using some CSS hacks to only show the current page in PresentationMode, and that could lead to the `getVisibleElements` function not always finding the correct elements. However, after the changes in PR 14112 we're now using the Page-scrolling mode in PresentationMode and consequently there'll only be *a single* page visible at a time. Hence then `BaseViewer._getCurrentVisiblePage` helper method should no longer be needed, and when testing (locally) in Google Chrome everything seems to work correctly now. --- web/base_viewer.js | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 1f3b82fd0..f055122b2 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1418,37 +1418,7 @@ class BaseViewer { : this.container.scrollHeight > this.container.clientHeight; } - /** - * Helper method for `this._getVisiblePages`. Should only ever be used when - * the viewer can only display a single page at a time, for example: - * - When PresentationMode is active. - */ - _getCurrentVisiblePage() { - if (!this.pagesCount) { - return { views: [] }; - } - const pageView = this._pages[this._currentPageNumber - 1]; - // NOTE: Compute the `x` and `y` properties of the current view, - // since `this._updateLocation` depends of them being available. - const element = pageView.div; - - const view = { - id: pageView.id, - x: element.offsetLeft + element.clientLeft, - y: element.offsetTop + element.clientTop, - view: pageView, - }; - const ids = new Set([pageView.id]); - - return { first: view, last: view, views: [view], ids }; - } - _getVisiblePages() { - if (this.isInPresentationMode) { - // The algorithm in `getVisibleElements` doesn't work in all browsers and - // configurations (e.g. Chrome) when PresentationMode is active. - return this._getCurrentVisiblePage(); - } const views = this._scrollMode === ScrollMode.PAGE ? this.#scrollModePageState.pages From 55838303c7f2ee945dfe6e8a2e08579b5960f772 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 5 Apr 2022 13:56:51 +0200 Subject: [PATCH 2/2] Tweak the `pdfOpenParams` parameter, in the "updateviewarea" event, in PresentationMode The `pdfOpenParams` parameter has never really made sense in PresentationMode, since e.g. the zoom-value doesn't (generally) agree with the value chosen by the user prior to entering PresentationMode. This has never mattered all that much, since the `viewBookmark`-button isn't visible in PresentationMode (nor is any other toolbar button for that matter). However, in the `PDFHistory`-implementation we're currently forced to handle this case specifically since we don't want to populate the browser history with nonsensical state. Hence it makes overall sense, as far as I'm concerned, to tweak the "updateviewarea" event to include a *simplified* `pdfOpenParams` parameter when PresentationMode is active. Given that the `viewer components` don't include PresentationMode functionality, this change thus shouldn't matter for third-party users. --- web/base_viewer.js | 8 +++++--- web/pdf_history.js | 19 ++++--------------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index f055122b2..3ba5d3a9d 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1321,8 +1321,6 @@ class BaseViewer { : currentScaleValue; const pageNumber = firstPage.id; - let pdfOpenParams = "#page=" + pageNumber; - pdfOpenParams += "&zoom=" + normalizedScaleValue; const currentPageView = this._pages[pageNumber - 1]; const container = this.container; const topLeft = currentPageView.getPagePoint( @@ -1331,7 +1329,11 @@ class BaseViewer { ); const intLeft = Math.round(topLeft[0]); const intTop = Math.round(topLeft[1]); - pdfOpenParams += "," + intLeft + "," + intTop; + + let pdfOpenParams = `#page=${pageNumber}`; + if (!this.isInPresentationMode) { + pdfOpenParams += `&zoom=${normalizedScaleValue},${intLeft},${intTop}`; + } this._location = { pageNumber, diff --git a/web/pdf_history.js b/web/pdf_history.js index c6ef42cc7..b7dec87af 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -16,11 +16,7 @@ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ -import { - isValidRotation, - parseQueryString, - PresentationModeState, -} from "./ui_utils.js"; +import { isValidRotation, parseQueryString } from "./ui_utils.js"; import { waitOnEventOrTimeout } from "./event_utils.js"; // Heuristic value used when force-resetting `this._blockHashChange`. @@ -69,13 +65,8 @@ class PDFHistory { this.reset(); this._boundEvents = null; - this._isViewerInPresentationMode = false; - // Ensure that we don't miss either a 'presentationmodechanged' or a - // 'pagesinit' event, by registering the listeners immediately. - this.eventBus._on("presentationmodechanged", evt => { - this._isViewerInPresentationMode = - evt.state !== PresentationModeState.NORMAL; - }); + // Ensure that we don't miss a "pagesinit" event, + // by registering the listener immediately. this.eventBus._on("pagesinit", () => { this._isPagesLoaded = false; @@ -566,9 +557,7 @@ class PDFHistory { } this._position = { - hash: this._isViewerInPresentationMode - ? `page=${location.pageNumber}` - : location.pdfOpenParams.substring(1), + hash: location.pdfOpenParams.substring(1), page: this.linkService.page, first: location.pageNumber, rotation: location.rotation,