From 8d61b7c088ced2e16a93bc93058090659b0181b2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 9 Apr 2022 11:25:53 +0200 Subject: [PATCH 1/2] Simplify handling of `requestFullscreen` errors in `PDFPresentationMode` Since quite some time the `Element.requestFullscreen()` method has been returning a Promise, see https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen#return_value Hence we can utilize that to detect failures to enter fullscreen-mode, and remove our old `setTimeout`-based hacks that were used for this purpose. According to the MDN compatibility data, see https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen#browser_compatibility, all browsers that we support have implemented this functionality. (Note that after PR 14606, we no longer support PresentationMode in Safari.) --- web/pdf_presentation_mode.js | 55 +++++++++++++++--------------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index f94bea021..ba67cf69c 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -20,7 +20,6 @@ import { SpreadMode, } from "./ui_utils.js"; -const DELAY_BEFORE_RESETTING_SWITCH_IN_PROGRESS = 1500; // in ms const DELAY_BEFORE_HIDING_CONTROLS = 3000; // in ms const ACTIVE_SELECTOR = "pdfPresentationMode"; const CONTROLS_SELECTOR = "pdfPresentationModeControls"; @@ -42,6 +41,8 @@ const SWIPE_ANGLE_THRESHOLD = Math.PI / 6; */ class PDFPresentationMode { + #args = null; + /** * @param {PDFPresentationModeOptions} options */ @@ -51,7 +52,6 @@ class PDFPresentationMode { this.eventBus = eventBus; this.active = false; - this.args = null; this.contextMenuOpen = false; this.mouseScrollTimeStamp = 0; this.mouseScrollDelta = 0; @@ -60,9 +60,9 @@ class PDFPresentationMode { /** * Request the browser to enter fullscreen mode. - * @returns {boolean} Indicating if the request was successful. + * @returns {Promise} Indicating if the request was successful. */ - request() { + async request() { if ( this.switchInProgress || this.active || @@ -75,22 +75,30 @@ class PDFPresentationMode { this.#setSwitchInProgress(); this.#notifyStateChange(); - this.container.requestFullscreen(); + const promise = this.container.requestFullscreen(); - this.args = { + this.#args = { pageNumber: this.pdfViewer.currentPageNumber, scaleValue: this.pdfViewer.currentScaleValue, scrollMode: this.pdfViewer.scrollMode, spreadMode: this.pdfViewer.spreadMode, }; - return true; + + try { + await promise; + return true; + } catch (reason) { + this.#removeFullscreenChangeListeners(); + this.#resetSwitchInProgress(); + this.#notifyStateChange(); + } + return false; } #mouseWheel(evt) { if (!this.active) { return; } - evt.preventDefault(); const delta = normalizeWheelEventDelta(evt); @@ -139,29 +147,12 @@ class PDFPresentationMode { }); } - /** - * Used to initialize a timeout when requesting Presentation Mode, - * i.e. when the browser is requested to enter fullscreen mode. - * This timeout is used to prevent the current page from being scrolled - * partially, or completely, out of view when entering Presentation Mode. - * NOTE: This issue seems limited to certain zoom levels (e.g. page-width). - */ #setSwitchInProgress() { - if (this.switchInProgress) { - clearTimeout(this.switchInProgress); - } - this.switchInProgress = setTimeout(() => { - this.#removeFullscreenChangeListeners(); - delete this.switchInProgress; - this.#notifyStateChange(); - }, DELAY_BEFORE_RESETTING_SWITCH_IN_PROGRESS); + this.switchInProgress = true; } #resetSwitchInProgress() { - if (this.switchInProgress) { - clearTimeout(this.switchInProgress); - delete this.switchInProgress; - } + this.switchInProgress = false; } #enter() { @@ -175,7 +166,7 @@ class PDFPresentationMode { setTimeout(() => { this.pdfViewer.scrollMode = ScrollMode.PAGE; this.pdfViewer.spreadMode = SpreadMode.NONE; - this.pdfViewer.currentPageNumber = this.args.pageNumber; + this.pdfViewer.currentPageNumber = this.#args.pageNumber; this.pdfViewer.currentScaleValue = "page-fit"; }, 0); @@ -200,11 +191,11 @@ class PDFPresentationMode { this.#removeFullscreenChangeListeners(); this.#notifyStateChange(); - this.pdfViewer.scrollMode = this.args.scrollMode; - this.pdfViewer.spreadMode = this.args.spreadMode; - this.pdfViewer.currentScaleValue = this.args.scaleValue; + this.pdfViewer.scrollMode = this.#args.scrollMode; + this.pdfViewer.spreadMode = this.#args.spreadMode; + this.pdfViewer.currentScaleValue = this.#args.scaleValue; this.pdfViewer.currentPageNumber = pageNumber; - this.args = null; + this.#args = null; }, 0); this.#removeWindowListeners(); From bde6d9ffbaa52c8dbb38c01a40c323529ddb150f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 9 Apr 2022 11:38:05 +0200 Subject: [PATCH 2/2] Re-factor how `PDFPresentationMode`, internally, tracks the current `PresentationModeState` With the changes in the previous patch, we can simplify the state-tracking by using the `PresentationModeState`-values directly in the `PDFPresentationMode` class. --- web/pdf_presentation_mode.js | 63 ++++++++++++++---------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index ba67cf69c..912d3a202 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -41,6 +41,8 @@ const SWIPE_ANGLE_THRESHOLD = Math.PI / 6; */ class PDFPresentationMode { + #state = PresentationModeState.UNKNOWN; + #args = null; /** @@ -51,7 +53,6 @@ class PDFPresentationMode { this.pdfViewer = pdfViewer; this.eventBus = eventBus; - this.active = false; this.contextMenuOpen = false; this.mouseScrollTimeStamp = 0; this.mouseScrollDelta = 0; @@ -63,25 +64,21 @@ class PDFPresentationMode { * @returns {Promise} Indicating if the request was successful. */ async request() { - if ( - this.switchInProgress || - this.active || - !this.pdfViewer.pagesCount || - !this.container.requestFullscreen - ) { + const { container, pdfViewer } = this; + + if (this.active || !pdfViewer.pagesCount || !container.requestFullscreen) { return false; } this.#addFullscreenChangeListeners(); - this.#setSwitchInProgress(); - this.#notifyStateChange(); + this.#notifyStateChange(PresentationModeState.CHANGING); - const promise = this.container.requestFullscreen(); + const promise = container.requestFullscreen(); this.#args = { - pageNumber: this.pdfViewer.currentPageNumber, - scaleValue: this.pdfViewer.currentScaleValue, - scrollMode: this.pdfViewer.scrollMode, - spreadMode: this.pdfViewer.spreadMode, + pageNumber: pdfViewer.currentPageNumber, + scaleValue: pdfViewer.currentScaleValue, + scrollMode: pdfViewer.scrollMode, + spreadMode: pdfViewer.spreadMode, }; try { @@ -89,12 +86,18 @@ class PDFPresentationMode { return true; } catch (reason) { this.#removeFullscreenChangeListeners(); - this.#resetSwitchInProgress(); - this.#notifyStateChange(); + this.#notifyStateChange(PresentationModeState.NORMAL); } return false; } + get active() { + return ( + this.#state === PresentationModeState.CHANGING || + this.#state === PresentationModeState.FULLSCREEN + ); + } + #mouseWheel(evt) { if (!this.active) { return; @@ -134,31 +137,14 @@ class PDFPresentationMode { } } - #notifyStateChange() { - let state = PresentationModeState.NORMAL; - if (this.switchInProgress) { - state = PresentationModeState.CHANGING; - } else if (this.active) { - state = PresentationModeState.FULLSCREEN; - } - this.eventBus.dispatch("presentationmodechanged", { - source: this, - state, - }); - } + #notifyStateChange(state) { + this.#state = state; - #setSwitchInProgress() { - this.switchInProgress = true; - } - - #resetSwitchInProgress() { - this.switchInProgress = false; + this.eventBus.dispatch("presentationmodechanged", { source: this, state }); } #enter() { - this.active = true; - this.#resetSwitchInProgress(); - this.#notifyStateChange(); + this.#notifyStateChange(PresentationModeState.FULLSCREEN); this.container.classList.add(ACTIVE_SELECTOR); // Ensure that the correct page is scrolled into view when entering @@ -187,9 +173,8 @@ class PDFPresentationMode { // Ensure that the correct page is scrolled into view when exiting // Presentation Mode, by waiting until fullscreen mode is disabled. setTimeout(() => { - this.active = false; this.#removeFullscreenChangeListeners(); - this.#notifyStateChange(); + this.#notifyStateChange(PresentationModeState.NORMAL); this.pdfViewer.scrollMode = this.#args.scrollMode; this.pdfViewer.spreadMode = this.#args.spreadMode;