From 5565a6f8bf612f29f36f4bc53744628818fbbfed Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 19 Aug 2017 14:23:40 +0200 Subject: [PATCH] Slightly refactor the pages rotation handling code in the viewer This changes both `PDFViewer` and `PDFThumbnailViewer` to return early in the `pagesRotation` setters if the rotation doesn't change. It also fixes an existing issue, in `PDFViewer`, that would cause errors if the rotation changes *before* the scale has been set to a non-default value. Finally, in preparation for subsequent patches, it also refactors the rotation code in `web/app.js` to update the thumbnails and trigger rendering with the new `rotationchanging` event. --- test/unit/ui_utils_spec.js | 27 +++++++++++++++++++++++++-- web/app.js | 24 ++++++++++++++---------- web/pdf_thumbnail_viewer.js | 7 +++++-- web/pdf_viewer.js | 22 ++++++++++++++++++---- web/ui_utils.js | 5 +++++ 5 files changed, 67 insertions(+), 18 deletions(-) diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index 58f97e9d1..bae36b51d 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -14,8 +14,8 @@ */ import { - binarySearchFirstItem, EventBus, getPDFFileNameFromURL, waitOnEventOrTimeout, - WaitOnType + binarySearchFirstItem, EventBus, getPDFFileNameFromURL, isValidRotation, + waitOnEventOrTimeout, WaitOnType } from '../../web/ui_utils'; import { createObjectURL, isNodeJS } from '../../src/shared/util'; @@ -261,6 +261,29 @@ describe('ui_utils', function() { }); }); + describe('isValidRotation', function() { + it('should reject non-integer angles', function() { + expect(isValidRotation()).toEqual(false); + expect(isValidRotation(null)).toEqual(false); + expect(isValidRotation(NaN)).toEqual(false); + expect(isValidRotation([90])).toEqual(false); + expect(isValidRotation('90')).toEqual(false); + expect(isValidRotation(90.5)).toEqual(false); + }); + + it('should reject non-multiple of 90 degree angles', function() { + expect(isValidRotation(45)).toEqual(false); + expect(isValidRotation(-123)).toEqual(false); + }); + + it('should accept valid angles', function() { + expect(isValidRotation(0)).toEqual(true); + expect(isValidRotation(90)).toEqual(true); + expect(isValidRotation(-270)).toEqual(true); + expect(isValidRotation(540)).toEqual(true); + }); + }); + describe('waitOnEventOrTimeout', function() { let eventBus; diff --git a/web/app.js b/web/app.js index caeeb37da..1dc37c0ac 100644 --- a/web/app.js +++ b/web/app.js @@ -1232,16 +1232,10 @@ let PDFViewerApplication = { if (!this.pdfDocument) { return; } - let { pdfViewer, pdfThumbnailViewer, } = this; - let pageNumber = pdfViewer.currentPageNumber; - let newRotation = (pdfViewer.pagesRotation + 360 + delta) % 360; - - pdfViewer.pagesRotation = newRotation; - pdfThumbnailViewer.pagesRotation = newRotation; - - this.forceRendering(); - // Ensure that the active page doesn't change during rotation. - pdfViewer.currentPageNumber = pageNumber; + let newRotation = (this.pdfViewer.pagesRotation + 360 + delta) % 360; + this.pdfViewer.pagesRotation = newRotation; + // Note that the thumbnail viewer is updated, and rendering is triggered, + // in the 'rotationchanging' event handler. }, requestPresentationMode() { @@ -1266,6 +1260,7 @@ let PDFViewerApplication = { eventBus.on('updateviewarea', webViewerUpdateViewarea); eventBus.on('pagechanging', webViewerPageChanging); eventBus.on('scalechanging', webViewerScaleChanging); + eventBus.on('rotationchanging', webViewerRotationChanging); eventBus.on('sidebarviewchanged', webViewerSidebarViewChanged); eventBus.on('pagemode', webViewerPageMode); eventBus.on('namedaction', webViewerNamedAction); @@ -1343,6 +1338,7 @@ let PDFViewerApplication = { eventBus.off('updateviewarea', webViewerUpdateViewarea); eventBus.off('pagechanging', webViewerPageChanging); eventBus.off('scalechanging', webViewerScaleChanging); + eventBus.off('rotationchanging', webViewerRotationChanging); eventBus.off('sidebarviewchanged', webViewerSidebarViewChanged); eventBus.off('pagemode', webViewerPageMode); eventBus.off('namedaction', webViewerNamedAction); @@ -1926,6 +1922,14 @@ function webViewerScaleChanging(evt) { PDFViewerApplication.pdfViewer.update(); } +function webViewerRotationChanging(evt) { + PDFViewerApplication.pdfThumbnailViewer.pagesRotation = evt.pagesRotation; + + PDFViewerApplication.forceRendering(); + // Ensure that the active page doesn't change during rotation. + PDFViewerApplication.pdfViewer.currentPageNumber = evt.pageNumber; +} + function webViewerPageChanging(evt) { let page = evt.pageNumber; diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index d0ecb469a..ce23d6556 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -14,7 +14,7 @@ */ import { - getVisibleElements, NullL10n, scrollIntoView, watchScroll + getVisibleElements, isValidRotation, NullL10n, scrollIntoView, watchScroll } from './ui_utils'; import { PDFThumbnailView } from './pdf_thumbnail_view'; @@ -95,12 +95,15 @@ class PDFThumbnailViewer { } set pagesRotation(rotation) { - if (!(typeof rotation === 'number' && rotation % 90 === 0)) { + if (!isValidRotation(rotation)) { throw new Error('Invalid thumbnails rotation angle.'); } if (!this.pdfDocument) { return; } + if (this._pagesRotation === rotation) { + return; // The rotation didn't change. + } this._pagesRotation = rotation; for (let i = 0, ii = this._thumbnails.length; i < ii; i++) { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index f58652726..7e65818c1 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -16,8 +16,8 @@ import { createPromiseCapability, PDFJS } from 'pdfjs-lib'; import { CSS_UNITS, DEFAULT_SCALE, DEFAULT_SCALE_VALUE, getVisibleElements, - MAX_AUTO_SCALE, NullL10n, RendererType, SCROLLBAR_PADDING, scrollIntoView, - UNKNOWN_SCALE, VERTICAL_PADDING, watchScroll + isValidRotation, MAX_AUTO_SCALE, NullL10n, RendererType, SCROLLBAR_PADDING, + scrollIntoView, UNKNOWN_SCALE, VERTICAL_PADDING, watchScroll } from './ui_utils'; import { PDFRenderingQueue, RenderingStates } from './pdf_rendering_queue'; import { AnnotationLayerBuilder } from './annotation_layer_builder'; @@ -271,20 +271,34 @@ class PDFViewer { * @param {number} rotation - The rotation of the pages (0, 90, 180, 270). */ set pagesRotation(rotation) { - if (!(typeof rotation === 'number' && rotation % 90 === 0)) { + if (!isValidRotation(rotation)) { throw new Error('Invalid pages rotation angle.'); } if (!this.pdfDocument) { return; } + if (this._pagesRotation === rotation) { + return; // The rotation didn't change. + } this._pagesRotation = rotation; + let pageNumber = this._currentPageNumber; + for (let i = 0, ii = this._pages.length; i < ii; i++) { let pageView = this._pages[i]; pageView.update(pageView.scale, rotation); } + // Prevent errors in case the rotation changes *before* the scale has been + // set to a non-default value. + if (this._currentScaleValue) { + this._setScale(this._currentScaleValue, true); + } - this._setScale(this._currentScaleValue, true); + this.eventBus.dispatch('rotationchanging', { + source: this, + pagesRotation: rotation, + pageNumber, + }); if (this.defaultRenderingQueue) { this.update(); diff --git a/web/ui_utils.js b/web/ui_utils.js index a670d6702..aec8a5793 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -443,6 +443,10 @@ function normalizeWheelEventDelta(evt) { return delta; } +function isValidRotation(angle) { + return Number.isInteger(angle) && angle % 90 === 0; +} + function cloneObj(obj) { let result = Object.create(null); for (let i in obj) { @@ -655,6 +659,7 @@ export { MAX_AUTO_SCALE, SCROLLBAR_PADDING, VERTICAL_PADDING, + isValidRotation, cloneObj, RendererType, mozL10n,