Merge pull request #8868 from Snuffleupagus/save-rotation

Store the rotation in the `ViewHistory`/`PDFHistory` (issue 5927)

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;
 

web/app.js

@@ -15,9 +15,9 @@
 /* globals PDFBug, Stats */
 
 import {
-  animationStarted, DEFAULT_SCALE_VALUE, getPDFFileNameFromURL, MAX_SCALE,
-  MIN_SCALE, noContextMenuHandler, normalizeWheelEventDelta, parseQueryString,
-  ProgressBar, RendererType
+  animationStarted, DEFAULT_SCALE_VALUE, getPDFFileNameFromURL, isValidRotation,
+  MAX_SCALE, MIN_SCALE, noContextMenuHandler, normalizeWheelEventDelta,
+  parseQueryString, ProgressBar, RendererType
 } from './ui_utils';
 import {
   build, createBlob, getDocument, getFilenameFromUrl, InvalidPDFException,
@@ -935,6 +935,8 @@ let PDFViewerApplication = {
 
         if (this.pdfHistory.initialBookmark) {
           this.initialBookmark = this.pdfHistory.initialBookmark;
+
+          this.initialRotation = this.pdfHistory.initialRotation;
         }
       }
 
@@ -948,6 +950,7 @@ let PDFViewerApplication = {
         zoom: DEFAULT_SCALE_VALUE,
         scrollLeft: '0',
         scrollTop: '0',
+        rotation: null,
         sidebarView: SidebarView.NONE,
       }).catch(() => { /* Unable to read from storage; ignoring errors. */ });
 
@@ -956,12 +959,14 @@ let PDFViewerApplication = {
         // Initialize the default values, from user preferences.
         let hash = this.viewerPrefs['defaultZoomValue'] ?
           ('zoom=' + this.viewerPrefs['defaultZoomValue']) : null;
+        let rotation = null;
         let sidebarView = this.viewerPrefs['sidebarViewOnLoad'];
 
         if (values.exists && this.viewerPrefs['showPreviousViewOnLoad']) {
           hash = 'page=' + values.page +
             '&zoom=' + (this.viewerPrefs['defaultZoomValue'] || values.zoom) +
             ',' + values.scrollLeft + ',' + values.scrollTop;
+          rotation = parseInt(values.rotation, 10);
           sidebarView = sidebarView || (values.sidebarView | 0);
         }
         if (pageMode && !this.viewerPrefs['disablePageMode']) {
@@ -970,13 +975,14 @@ let PDFViewerApplication = {
         }
         return {
           hash,
+          rotation,
           sidebarView,
         };
-      }).then(({ hash, sidebarView, }) => {
+      }).then(({ hash, rotation, sidebarView, }) => {
         initialParams.bookmark = this.initialBookmark;
         initialParams.hash = hash;
 
-        this.setInitialView(hash, { sidebarView, });
+        this.setInitialView(hash, { rotation, sidebarView, });
 
         // Make all navigation keys work on document load,
         // unless the viewer is embedded in a web page.
@@ -1131,14 +1137,24 @@ let PDFViewerApplication = {
     });
   },
 
-  setInitialView(storedHash, { sidebarView, } = {}) {
+  setInitialView(storedHash, { rotation, sidebarView, } = {}) {
+    let setRotation = (angle) => {
+      if (isValidRotation(angle)) {
+        this.pdfViewer.pagesRotation = angle;
+      }
+    };
     this.isInitialViewSet = true;
     this.pdfSidebar.setInitialView(sidebarView);
 
     if (this.initialBookmark) {
+      setRotation(this.initialRotation);
+      delete this.initialRotation;
+
       this.pdfLinkService.setHash(this.initialBookmark);
       this.initialBookmark = null;
     } else if (storedHash) {
+      setRotation(rotation);
+
       this.pdfLinkService.setHash(storedHash);
     }
 
@@ -1232,16 +1248,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 +1276,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 +1354,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);
@@ -1769,6 +1781,7 @@ function webViewerUpdateViewarea(evt) {
       'zoom': location.scale,
       'scrollLeft': location.left,
       'scrollTop': location.top,
+      'rotation': location.rotation,
     }).catch(function() { /* unable to write to storage */ });
   }
   let href =
@@ -1926,6 +1939,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;
 

web/interfaces.js

@@ -30,6 +30,16 @@ class IPDFLinkService {
    */
   set page(value) {}
 
+  /**
+   * @returns {number}
+   */
+  get rotation() {}
+
+  /**
+   * @param {number} value
+   */
+  set rotation(value) {}
+
   /**
    * @param dest - The PDF destination object.
    */

web/pdf_history.js

@@ -13,7 +13,9 @@
  * limitations under the License.
  */
 
-import { cloneObj, parseQueryString, waitOnEventOrTimeout } from './ui_utils';
+import {
+  cloneObj, isValidRotation, parseQueryString, waitOnEventOrTimeout
+} from './ui_utils';
 import { getGlobalEventBus } from './dom_events';
 
 // Heuristic value used when force-resetting `this._blockHashChange`.
@@ -49,7 +51,7 @@ function parseCurrentHash(linkService) {
   if (!(Number.isInteger(page) && page > 0 && page <= linkService.pagesCount)) {
     page = null;
   }
-  return { hash, page, };
+  return { hash, page, rotation: linkService.rotation, };
 }
 
 class PDFHistory {
@@ -62,6 +64,7 @@ class PDFHistory {
 
     this.initialized = false;
     this.initialBookmark = null;
+    this.initialRotation = null;
 
     this._boundEvents = Object.create(null);
     this._isViewerInPresentationMode = false;
@@ -99,6 +102,7 @@ class PDFHistory {
 
     this.initialized = true;
     this.initialBookmark = null;
+    this.initialRotation = null;
 
     this._popStateInProgress = false;
     this._blockHashChange = 0;
@@ -110,7 +114,7 @@ class PDFHistory {
     this._position = null;
 
     if (!this._isValidState(state) || resetHistory) {
-      let { hash, page, } = parseCurrentHash(this.linkService);
+      let { hash, page, rotation, } = parseCurrentHash(this.linkService);
 
       if (!hash || reInitialized || resetHistory) {
         // Ensure that the browser history is reset on PDF document load.
@@ -119,7 +123,8 @@ class PDFHistory {
       }
       // Ensure that the browser history is initialized correctly when
       // the document hash is present on PDF document load.
-      this._pushOrReplaceState({ hash, page, }, /* forceReplace = */ true);
+      this._pushOrReplaceState({ hash, page, rotation, },
+                               /* forceReplace = */ true);
       return;
     }
 
@@ -128,6 +133,10 @@ class PDFHistory {
     let destination = state.destination;
     this._updateInternalState(destination, state.uid,
                               /* removeTemporary = */ true);
+
+    if (destination.rotation !== undefined) {
+      this.initialRotation = destination.rotation;
+    }
     if (destination.dest) {
       this.initialBookmark = JSON.stringify(destination.dest);
 
@@ -188,6 +197,7 @@ class PDFHistory {
       dest: explicitDest,
       hash,
       page: pageNumber,
+      rotation: this.linkService.rotation,
     }, forceReplace);
 
     if (!this._popStateInProgress) {
@@ -402,6 +412,7 @@ class PDFHistory {
         `page=${location.pageNumber}` : location.pdfOpenParams.substring(1),
       page: this.linkService.page,
       first: location.pageNumber,
+      rotation: location.rotation,
     };
 
     if (this._popStateInProgress) {
@@ -459,8 +470,9 @@ class PDFHistory {
       // This case corresponds to the user changing the hash of the document.
       this._currentUid = this._uid;
 
-      let { hash, page, } = parseCurrentHash(this.linkService);
-      this._pushOrReplaceState({ hash, page, }, /* forceReplace */ true);
+      let { hash, page, rotation, } = parseCurrentHash(this.linkService);
+      this._pushOrReplaceState({ hash, page, rotation, },
+                               /* forceReplace = */ true);
       return;
     }
     if (!this._isValidState(state)) {
@@ -497,6 +509,10 @@ class PDFHistory {
     let destination = state.destination;
     this._updateInternalState(destination, state.uid,
                               /* removeTemporary = */ true);
+
+    if (isValidRotation(destination.rotation)) {
+      this.linkService.rotation = destination.rotation;
+    }
     if (destination.dest) {
       this.linkService.navigateTo(destination.dest);
     } else if (destination.hash) {

web/pdf_link_service.js

@@ -75,6 +75,20 @@ class PDFLinkService {
     this.pdfViewer.currentPageNumber = value;
   }
 
+  /**
+   * @returns {number}
+   */
+  get rotation() {
+    return this.pdfViewer.pagesRotation;
+  }
+
+  /**
+   * @param {number} value
+   */
+  set rotation(value) {
+    this.pdfViewer.pagesRotation = value;
+  }
+
   /**
    * @param {string|Array} dest - The named, or explicit, PDF destination.
    */
@@ -413,6 +427,16 @@ class SimpleLinkService {
    * @param {number} value
    */
   set page(value) {}
+  /**
+   * @returns {number}
+   */
+  get rotation() {
+    return 0;
+  }
+  /**
+   * @param {number} value
+   */
+  set rotation(value) {}
   /**
    * @param dest - The PDF destination object.
    */

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++) {

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.eventBus.dispatch('rotationchanging', {
+      source: this,
+      pagesRotation: rotation,
+      pageNumber,
+    });
 
     if (this.defaultRenderingQueue) {
       this.update();
@@ -718,6 +732,7 @@ class PDFViewer {
       scale: normalizedScaleValue,
       top: intTop,
       left: intLeft,
+      rotation: this._pagesRotation,
       pdfOpenParams,
     };
   }

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,