From cadc4d2f615b20d1392602e1cfe164aab901c068 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 25 Feb 2022 14:49:12 +0100 Subject: [PATCH 1/2] Replace all "private" methods in `PDFPresentationMode` with proper ones Now that the there's ECMAScript support for properly private methods on `class`es, we can use that instead and thus remove all of the `@private` JSDocs comments. --- web/pdf_presentation_mode.js | 133 +++++++++++------------------------ 1 file changed, 42 insertions(+), 91 deletions(-) diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index 937ee50f6..9b590ad81 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -66,9 +66,9 @@ class PDFPresentationMode { if (this.switchInProgress || this.active || !this.pdfViewer.pagesCount) { return false; } - this._addFullscreenChangeListeners(); - this._setSwitchInProgress(); - this._notifyStateChange(); + this.#addFullscreenChangeListeners(); + this.#setSwitchInProgress(); + this.#notifyStateChange(); if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { if (this.container.requestFullscreen) { @@ -96,10 +96,7 @@ class PDFPresentationMode { return true; } - /** - * @private - */ - _mouseWheel(evt) { + #mouseWheel(evt) { if (!this.active) { return; } @@ -122,13 +119,13 @@ class PDFPresentationMode { (this.mouseScrollDelta > 0 && delta < 0) || (this.mouseScrollDelta < 0 && delta > 0) ) { - this._resetMouseScrollState(); + this.#resetMouseScrollState(); } this.mouseScrollDelta += delta; if (Math.abs(this.mouseScrollDelta) >= PAGE_SWITCH_THRESHOLD) { const totalDelta = this.mouseScrollDelta; - this._resetMouseScrollState(); + this.#resetMouseScrollState(); const success = totalDelta > 0 ? this.pdfViewer.previousPage() @@ -146,10 +143,7 @@ class PDFPresentationMode { return !!(document.fullscreenElement || document.webkitIsFullScreen); } - /** - * @private - */ - _notifyStateChange() { + #notifyStateChange() { let state = PresentationModeState.NORMAL; if (this.switchInProgress) { state = PresentationModeState.CHANGING; @@ -168,37 +162,29 @@ class PDFPresentationMode { * 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). - * - * @private */ - _setSwitchInProgress() { + #setSwitchInProgress() { if (this.switchInProgress) { clearTimeout(this.switchInProgress); } this.switchInProgress = setTimeout(() => { - this._removeFullscreenChangeListeners(); + this.#removeFullscreenChangeListeners(); delete this.switchInProgress; - this._notifyStateChange(); + this.#notifyStateChange(); }, DELAY_BEFORE_RESETTING_SWITCH_IN_PROGRESS); } - /** - * @private - */ - _resetSwitchInProgress() { + #resetSwitchInProgress() { if (this.switchInProgress) { clearTimeout(this.switchInProgress); delete this.switchInProgress; } } - /** - * @private - */ - _enter() { + #enter() { this.active = true; - this._resetSwitchInProgress(); - this._notifyStateChange(); + this.#resetSwitchInProgress(); + this.#notifyStateChange(); this.container.classList.add(ACTIVE_SELECTOR); // Ensure that the correct page is scrolled into view when entering @@ -210,8 +196,8 @@ class PDFPresentationMode { this.pdfViewer.currentScaleValue = "page-fit"; }, 0); - this._addWindowListeners(); - this._showControls(); + this.#addWindowListeners(); + this.#showControls(); this.contextMenuOpen = false; // Text selection is disabled in Presentation Mode, thus it's not possible @@ -220,10 +206,7 @@ class PDFPresentationMode { window.getSelection().removeAllRanges(); } - /** - * @private - */ - _exit() { + #exit() { const pageNumber = this.pdfViewer.currentPageNumber; this.container.classList.remove(ACTIVE_SELECTOR); @@ -231,8 +214,8 @@ class PDFPresentationMode { // Presentation Mode, by waiting until fullscreen mode is disabled. setTimeout(() => { this.active = false; - this._removeFullscreenChangeListeners(); - this._notifyStateChange(); + this.#removeFullscreenChangeListeners(); + this.#notifyStateChange(); this.pdfViewer.scrollMode = this.args.scrollMode; this.pdfViewer.spreadMode = this.args.spreadMode; @@ -241,16 +224,13 @@ class PDFPresentationMode { this.args = null; }, 0); - this._removeWindowListeners(); - this._hideControls(); - this._resetMouseScrollState(); + this.#removeWindowListeners(); + this.#hideControls(); + this.#resetMouseScrollState(); this.contextMenuOpen = false; } - /** - * @private - */ - _mouseDown(evt) { + #mouseDown(evt) { if (this.contextMenuOpen) { this.contextMenuOpen = false; evt.preventDefault(); @@ -274,17 +254,11 @@ class PDFPresentationMode { } } - /** - * @private - */ - _contextMenu() { + #contextMenu() { this.contextMenuOpen = true; } - /** - * @private - */ - _showControls() { + #showControls() { if (this.controlsTimeout) { clearTimeout(this.controlsTimeout); } else { @@ -296,10 +270,7 @@ class PDFPresentationMode { }, DELAY_BEFORE_HIDING_CONTROLS); } - /** - * @private - */ - _hideControls() { + #hideControls() { if (!this.controlsTimeout) { return; } @@ -310,18 +281,13 @@ class PDFPresentationMode { /** * Resets the properties used for tracking mouse scrolling events. - * - * @private */ - _resetMouseScrollState() { + #resetMouseScrollState() { this.mouseScrollTimeStamp = 0; this.mouseScrollDelta = 0; } - /** - * @private - */ - _touchSwipe(evt) { + #touchSwipe(evt) { if (!this.active) { return; } @@ -381,16 +347,13 @@ class PDFPresentationMode { } } - /** - * @private - */ - _addWindowListeners() { - this.showControlsBind = this._showControls.bind(this); - this.mouseDownBind = this._mouseDown.bind(this); - this.mouseWheelBind = this._mouseWheel.bind(this); - this.resetMouseScrollStateBind = this._resetMouseScrollState.bind(this); - this.contextMenuBind = this._contextMenu.bind(this); - this.touchSwipeBind = this._touchSwipe.bind(this); + #addWindowListeners() { + this.showControlsBind = this.#showControls.bind(this); + this.mouseDownBind = this.#mouseDown.bind(this); + this.mouseWheelBind = this.#mouseWheel.bind(this); + this.resetMouseScrollStateBind = this.#resetMouseScrollState.bind(this); + this.contextMenuBind = this.#contextMenu.bind(this); + this.touchSwipeBind = this.#touchSwipe.bind(this); window.addEventListener("mousemove", this.showControlsBind); window.addEventListener("mousedown", this.mouseDownBind); @@ -402,10 +365,7 @@ class PDFPresentationMode { window.addEventListener("touchend", this.touchSwipeBind); } - /** - * @private - */ - _removeWindowListeners() { + #removeWindowListeners() { window.removeEventListener("mousemove", this.showControlsBind); window.removeEventListener("mousedown", this.mouseDownBind); window.removeEventListener("wheel", this.mouseWheelBind, { @@ -425,22 +385,16 @@ class PDFPresentationMode { delete this.touchSwipeBind; } - /** - * @private - */ - _fullscreenChange() { + #fullscreenChange() { if (this.isFullscreen) { - this._enter(); + this.#enter(); } else { - this._exit(); + this.#exit(); } } - /** - * @private - */ - _addFullscreenChangeListeners() { - this.fullscreenChangeBind = this._fullscreenChange.bind(this); + #addFullscreenChangeListeners() { + this.fullscreenChangeBind = this.#fullscreenChange.bind(this); window.addEventListener("fullscreenchange", this.fullscreenChangeBind); if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { @@ -451,10 +405,7 @@ class PDFPresentationMode { } } - /** - * @private - */ - _removeFullscreenChangeListeners() { + #removeFullscreenChangeListeners() { window.removeEventListener("fullscreenchange", this.fullscreenChangeBind); if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { window.removeEventListener( From 9d773c1499dc1dbe90996d16d4f8c24b59d5288b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 25 Feb 2022 15:36:45 +0100 Subject: [PATCH 2/2] Only support the standard, unprefixed, Fullscreen API in the default viewer At this point in time, after recent rounds of clean-up, the `webkit`-prefixed Fullscreen API is the only remaining *browser-specific* compatibility hack in the `web/`-folder JavaScript code. The standard, and thus unprefixed, Fullscreen API has been supported for *over three years* in both Mozilla Firefox and Google Chrome. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API#browser_compatibility), the unprefixed Fullscreen API has been available since: - Mozilla Firefox 64, released on 2018-12-11; see https://wiki.mozilla.org/Release_Management/Calendar#Past_branch_dates - Google Chrome 71, released on 2018-12-04; see https://en.wikipedia.org/wiki/Google_Chrome_version_history Hence *only* Safari now requires using a prefixed Fullscreen API, and it's thus (significantly) lagging behind other browsers in this regard. Considering that the default viewer is written *specifically* to be the UI for the Firefox PDF Viewer, and that we ask users to not just use it as-is[1], I think that we should only support the standard Fullscreen API now. Furthermore, note also that the FAQ already lists Safari as "Mostly" supported; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support --- [1] Note e.g. http://mozilla.github.io/pdf.js/getting_started/#introduction > The viewer is built on the display layer and is the UI for PDF viewer in Firefox and the other browser extensions within the project. It can be a good starting point for building your own viewer. *However, we do ask if you plan to embed the viewer in your own site, that it not just be an unmodified version. Please re-skin it or build upon it.* --- web/app.js | 9 +------ web/pdf_presentation_mode.js | 47 ++++++------------------------------ 2 files changed, 9 insertions(+), 47 deletions(-) diff --git a/web/app.js b/web/app.js index f60bf0296..6f6897f54 100644 --- a/web/app.js +++ b/web/app.js @@ -671,14 +671,7 @@ const PDFViewerApplication = { }, get supportsFullscreen() { - if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { - return shadow(this, "supportsFullscreen", document.fullscreenEnabled); - } - return shadow( - this, - "supportsFullscreen", - document.fullscreenEnabled || document.webkitFullscreenEnabled - ); + return shadow(this, "supportsFullscreen", document.fullscreenEnabled); }, get supportsIntegratedFind() { diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index 9b590ad81..f94bea021 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -63,28 +63,19 @@ class PDFPresentationMode { * @returns {boolean} Indicating if the request was successful. */ request() { - if (this.switchInProgress || this.active || !this.pdfViewer.pagesCount) { + if ( + this.switchInProgress || + this.active || + !this.pdfViewer.pagesCount || + !this.container.requestFullscreen + ) { return false; } this.#addFullscreenChangeListeners(); this.#setSwitchInProgress(); this.#notifyStateChange(); - if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { - if (this.container.requestFullscreen) { - this.container.requestFullscreen(); - } else { - return false; - } - } else { - if (this.container.requestFullscreen) { - this.container.requestFullscreen(); - } else if (this.container.webkitRequestFullscreen) { - this.container.webkitRequestFullscreen(Element.ALLOW_KEYBOARD_INPUT); - } else { - return false; - } - } + this.container.requestFullscreen(); this.args = { pageNumber: this.pdfViewer.currentPageNumber, @@ -92,7 +83,6 @@ class PDFPresentationMode { scrollMode: this.pdfViewer.scrollMode, spreadMode: this.pdfViewer.spreadMode, }; - return true; } @@ -136,13 +126,6 @@ class PDFPresentationMode { } } - get isFullscreen() { - if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { - return !!document.fullscreenElement; - } - return !!(document.fullscreenElement || document.webkitIsFullScreen); - } - #notifyStateChange() { let state = PresentationModeState.NORMAL; if (this.switchInProgress) { @@ -386,7 +369,7 @@ class PDFPresentationMode { } #fullscreenChange() { - if (this.isFullscreen) { + if (/* isFullscreen = */ document.fullscreenElement) { this.#enter(); } else { this.#exit(); @@ -395,25 +378,11 @@ class PDFPresentationMode { #addFullscreenChangeListeners() { this.fullscreenChangeBind = this.#fullscreenChange.bind(this); - window.addEventListener("fullscreenchange", this.fullscreenChangeBind); - if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { - window.addEventListener( - "webkitfullscreenchange", - this.fullscreenChangeBind - ); - } } #removeFullscreenChangeListeners() { window.removeEventListener("fullscreenchange", this.fullscreenChangeBind); - if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { - window.removeEventListener( - "webkitfullscreenchange", - this.fullscreenChangeBind - ); - } - delete this.fullscreenChangeBind; } }