From c1cfe2881b643f3b4e2017fe5db349936075a4b8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 3 Oct 2019 11:55:13 +0200 Subject: [PATCH 1/3] [PDFSidebarResizer] Skip the `CSS.supports` checks for MOZCENTRAL builds Since CSS variable support cannot be disabled any more in Firefox, the run-time checks are of no using for MOZCENTRAL builds. --- web/pdf_sidebar_resizer.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/pdf_sidebar_resizer.js b/web/pdf_sidebar_resizer.js index 384be23f74c70..3883310546b4c 100644 --- a/web/pdf_sidebar_resizer.js +++ b/web/pdf_sidebar_resizer.js @@ -47,8 +47,10 @@ class PDFSidebarResizer { this.eventBus = eventBus; this.l10n = l10n; - if (typeof CSS === 'undefined' || typeof CSS.supports !== 'function' || - !CSS.supports(SIDEBAR_WIDTH_VAR, `calc(-1 * ${SIDEBAR_MIN_WIDTH}px)`)) { + if ((typeof PDFJSDev === 'undefined' || !PDFJSDev.test('MOZCENTRAL')) && + (typeof CSS === 'undefined' || typeof CSS.supports !== 'function' || + !CSS.supports(SIDEBAR_WIDTH_VAR, + `calc(-1 * ${SIDEBAR_MIN_WIDTH}px)`))) { console.warn('PDFSidebarResizer: ' + 'The browser does not support resizing of the sidebar.'); return; From 9e4552d7920fcba13b3d74e1e180fc49180f4436 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 3 Oct 2019 12:03:45 +0200 Subject: [PATCH 2/3] [PDFSidebarResizer] Refactor the clamping in `_updateWidth` Rather than manually clamping the `width` here, we can just `export` an already existing helper function from `ui_utils.js` instead. (Hopefully it will eventually be possible to replace this helper function with a native `Math.clamp` function, given that there exists a "Stage 1 Proposal" for adding such a thing to the ECMAScript specification.) --- web/pdf_sidebar_resizer.js | 17 ++++++----------- web/ui_utils.js | 1 + 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/web/pdf_sidebar_resizer.js b/web/pdf_sidebar_resizer.js index 3883310546b4c..f975c0587efaa 100644 --- a/web/pdf_sidebar_resizer.js +++ b/web/pdf_sidebar_resizer.js @@ -13,7 +13,7 @@ * limitations under the License. */ -import { NullL10n } from './ui_utils'; +import { clamp, NullL10n } from './ui_utils'; const SIDEBAR_WIDTH_VAR = '--sidebar-width'; const SIDEBAR_MIN_WIDTH = 200; // pixels @@ -84,19 +84,14 @@ class PDFSidebarResizer { } // Prevent the sidebar from becoming too narrow, or from occupying more // than half of the available viewer width. - const maxWidth = Math.floor(this.outerContainerWidth / 2); - if (width > maxWidth) { - width = maxWidth; - } - if (width < SIDEBAR_MIN_WIDTH) { - width = SIDEBAR_MIN_WIDTH; - } + const newWidth = clamp(width, SIDEBAR_MIN_WIDTH, + Math.floor(this.outerContainerWidth / 2)); // Only update the UI when the sidebar width did in fact change. - if (width === this._width) { + if (newWidth === this._width) { return false; } - this._width = width; - this.doc.style.setProperty(SIDEBAR_WIDTH_VAR, `${width}px`); + this._width = newWidth; + this.doc.style.setProperty(SIDEBAR_WIDTH_VAR, `${newWidth}px`); return true; } diff --git a/web/ui_utils.js b/web/ui_utils.js index e1235759ae5ba..cb7108a7520e8 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -898,6 +898,7 @@ export { NullL10n, EventBus, getGlobalEventBus, + clamp, ProgressBar, getPDFFileNameFromURL, noContextMenuHandler, From 32d16ab5f6b95f0cd50a0ce0f42b7df44130d253 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 3 Oct 2019 12:24:19 +0200 Subject: [PATCH 3/3] [PDFSidebarResizer] Re-factor the `resize` event listener to improve readability I've absolutely no idea why I wrote the code that way originally, since the nested `if`s are not really helping readability one bit. Hence this patch which changes things to use early `return`s instead, to help readability. --- web/pdf_sidebar_resizer.js | 49 ++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/web/pdf_sidebar_resizer.js b/web/pdf_sidebar_resizer.js index f975c0587efaa..71d557ffede81 100644 --- a/web/pdf_sidebar_resizer.js +++ b/web/pdf_sidebar_resizer.js @@ -151,30 +151,33 @@ class PDFSidebarResizer { this.eventBus.on('resize', (evt) => { // When the *entire* viewer is resized, such that it becomes narrower, // ensure that the sidebar doesn't end up being too wide. - if (evt && evt.source === window) { - // Always reset the cached width when the viewer is resized. - this._outerContainerWidth = null; - - if (this._width) { - // NOTE: If the sidebar is closed, we don't need to worry about - // visual glitches nor ensure that rendering is triggered. - if (this.sidebarOpen) { - this.outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); - let updated = this._updateWidth(this._width); - - Promise.resolve().then(() => { - this.outerContainer.classList.remove(SIDEBAR_RESIZING_CLASS); - // Trigger rendering if the sidebar width changed, to avoid - // depending on the order in which 'resize' events are handled. - if (updated) { - this.eventBus.dispatch('resize', { source: this, }); - } - }); - } else { - this._updateWidth(this._width); - } - } + if (!evt || evt.source !== window) { + return; + } + // Always reset the cached width when the viewer is resized. + this._outerContainerWidth = null; + + if (!this._width) { + // The sidebar hasn't been resized, hence no need to adjust its width. + return; } + // NOTE: If the sidebar is closed, we don't need to worry about + // visual glitches nor ensure that rendering is triggered. + if (!this.sidebarOpen) { + this._updateWidth(this._width); + return; + } + this.outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); + let updated = this._updateWidth(this._width); + + Promise.resolve().then(() => { + this.outerContainer.classList.remove(SIDEBAR_RESIZING_CLASS); + // Trigger rendering if the sidebar width changed, to avoid + // depending on the order in which 'resize' events are handled. + if (updated) { + this.eventBus.dispatch('resize', { source: this, }); + } + }); }); } }