Merge pull request #9858 from Snuffleupagus/scrollMode-refactor

Additional Scroll/Spread mode clean-up (PR 9832 follow-up)

web/app.js

@@ -973,8 +973,7 @@ let PDFViewerApplication = {
     this.toolbar.setPagesCount(pdfDocument.numPages, false);
     this.secondaryToolbar.setPagesCount(pdfDocument.numPages);
 
-    let id = this.documentFingerprint = pdfDocument.fingerprint;
+    const store = this.store = new ViewHistory(pdfDocument.fingerprint);
-    let store = this.store = new ViewHistory(id);
 
     let baseDocumentUrl;
     if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) {
@@ -1003,7 +1002,7 @@ let PDFViewerApplication = {
         // The browsing history is only enabled when the viewer is standalone,
         // i.e. not when it is embedded in a web page.
         let resetHistory = !AppOptions.get('showPreviousViewOnLoad');
-        this.pdfHistory.initialize(id, resetHistory);
+        this.pdfHistory.initialize(pdfDocument.fingerprint, resetHistory);
 
         if (this.pdfHistory.initialBookmark) {
           this.initialBookmark = this.pdfHistory.initialBookmark;
@@ -1044,12 +1043,8 @@ let PDFViewerApplication = {
 
           rotation = parseInt(values.rotation, 10);
           sidebarView = sidebarView || (values.sidebarView | 0);
-          if (values.scrollMode !== null) {
+          scrollMode = scrollMode || (values.scrollMode | 0);
-            scrollMode = values.scrollMode;
+          spreadMode = spreadMode || (values.spreadMode | 0);
-          }
-          if (values.spreadMode !== null) {
-            spreadMode = values.spreadMode;
-          }
         }
         if (pageMode && !AppOptions.get('disablePageMode')) {
           // Always let the user preference/history take precedence.
@@ -1246,23 +1241,26 @@ let PDFViewerApplication = {
     });
   },
 
-  setInitialView(storedHash, values = {}) {
+  setInitialView(storedHash, { rotation, sidebarView,
-    let { rotation, sidebarView, scrollMode, spreadMode, } = values;
+                               scrollMode, spreadMode, } = {}) {
     let setRotation = (angle) => {
       if (isValidRotation(angle)) {
         this.pdfViewer.pagesRotation = angle;
       }
     };
+    let setViewerModes = (scroll, spread) => {
+      if (Number.isInteger(scroll)) {
+        this.pdfViewer.scrollMode = scroll;
+      }
+      if (Number.isInteger(spread)) {
+        this.pdfViewer.spreadMode = spread;
+      }
+    };
 
     // Putting these before isInitialViewSet = true prevents these values from
     // being stored in the document history (and overriding any future changes
     // made to the corresponding global preferences), just this once.
-    if (Number.isInteger(scrollMode)) {
+    setViewerModes(scrollMode, spreadMode);
-      this.pdfViewer.setScrollMode(scrollMode);
-    }
-    if (Number.isInteger(spreadMode)) {
-      this.pdfViewer.setSpreadMode(spreadMode);
-    }
 
     this.isInitialViewSet = true;
     this.pdfSidebar.setInitialView(sidebarView);
@@ -2021,10 +2019,10 @@ function webViewerRotateCcw() {
   PDFViewerApplication.rotatePages(-90);
 }
 function webViewerSwitchScrollMode(evt) {
-  PDFViewerApplication.pdfViewer.setScrollMode(evt.mode);
+  PDFViewerApplication.pdfViewer.scrollMode = evt.mode;
 }
 function webViewerSwitchSpreadMode(evt) {
-  PDFViewerApplication.pdfViewer.setSpreadMode(evt.mode);
+  PDFViewerApplication.pdfViewer.spreadMode = evt.mode;
 }
 function webViewerDocumentProperties() {
   PDFViewerApplication.pdfDocumentProperties.open();

web/app_options.js

@@ -116,6 +116,16 @@ const defaultOptions = {
     value: 0,
     kind: OptionKind.VIEWER,
   },
+  scrollModeOnLoad: {
+    /** @type {number} */
+    value: 0,
+    kind: OptionKind.VIEWER,
+  },
+  spreadModeOnLoad: {
+    /** @type {number} */
+    value: 0,
+    kind: OptionKind.VIEWER,
+  },
   textLayerMode: {
     /** @type {number} */
     value: 1,

web/base_viewer.js

@@ -73,14 +73,6 @@ const SpreadMode = {
  *   size in total pixels, i.e. width * height. Use -1 for no limit.
  *   The default value is 4096 * 4096 (16 mega-pixels).
  * @property {IL10n} l10n - Localization service.
- * @property {number} scrollMode - (optional) The direction in which the
- *   document pages should be laid out within the scrolling container. The
- *   constants from {ScrollMode} should be used. The default value is
- *   `ScrollMode.VERTICAL`.
- * @property {number} spreadMode - (optional) If not `SpreadMode.NONE`, groups
- *   pages into spreads, starting with odd- or even-numbered pages. The
- *   constants from {SpreadMode} should be used. The default value is
- *   `SpreadMode.NONE`.
  */
 
 function PDFPageViewBuffer(size) {
@@ -162,8 +154,6 @@ class BaseViewer {
     this.useOnlyCssZoom = options.useOnlyCssZoom || false;
     this.maxCanvasPixels = options.maxCanvasPixels;
     this.l10n = options.l10n || NullL10n;
-    this.scrollMode = options.scrollMode || ScrollMode.VERTICAL;
-    this.spreadMode = options.spreadMode || SpreadMode.NONE;
 
     this.defaultRenderingQueue = !options.renderingQueue;
     if (this.defaultRenderingQueue) {
@@ -181,8 +171,17 @@ class BaseViewer {
     if (this.removePageBorders) {
       this.viewer.classList.add('removePageBorders');
     }
-    if (this.scrollMode !== ScrollMode.VERTICAL) {
+
-      this._updateScrollModeClasses();
+    if ((typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) &&
+        ('scrollMode' in options || 'spreadMode' in options)) {
+      console.error(`The ${this._name} constructor options ` +
+        '`scrollMode`/`spreadMode` are deprecated, use the setters instead.');
+      if (options.scrollMode !== undefined) {
+        this.scrollMode = options.scrollMode;
+      }
+      if (options.spreadMode !== undefined) {
+        this.spreadMode = options.spreadMode;
+      }
     }
   }
 
@@ -441,8 +440,8 @@ class BaseViewer {
         bindOnAfterAndBeforeDraw(pageView);
         this._pages.push(pageView);
       }
-      if (this.spreadMode !== SpreadMode.NONE) {
+      if (this._spreadMode !== SpreadMode.NONE) {
-        this._regroupSpreads();
+        this._updateSpreadMode();
       }
 
       // Fetch all the pages since the viewport is needed before printing
@@ -524,9 +523,13 @@ class BaseViewer {
     this._pagesRotation = 0;
     this._pagesRequests = [];
     this._pageViewsReady = false;
+    this._scrollMode = ScrollMode.VERTICAL;
+    this._spreadMode = SpreadMode.NONE;
 
-    // Remove the pages from the DOM.
+    // Remove the pages from the DOM...
     this.viewer.textContent = '';
+    // ... and reset the Scroll mode CSS class(es) afterwards.
+    this._updateScrollMode();
   }
 
   _scrollUpdate() {
@@ -1023,26 +1026,132 @@ class BaseViewer {
     });
   }
 
-  setScrollMode(mode) {
+  /**
+   * @return {number} One of the values in {ScrollMode}.
+   */
+  get scrollMode() {
+    return this._scrollMode;
+  }
+
+  /**
+   * @param {number} mode - The direction in which the document pages should be
+   *   laid out within the scrolling container.
+   *   The constants from {ScrollMode} should be used.
+   */
+  set scrollMode(mode) {
+    if (this._scrollMode === mode) {
+      return; // The Scroll mode didn't change.
+    }
     if (!Number.isInteger(mode) || !Object.values(ScrollMode).includes(mode)) {
       throw new Error(`Invalid scroll mode: ${mode}`);
     }
-    this.scrollMode = mode;
+    this._scrollMode = mode;
+    this.eventBus.dispatch('scrollmodechanged', { source: this, mode, });
+
+    this._updateScrollMode(/* pageNumber = */ this._currentPageNumber);
   }
 
-  _updateScrollModeClasses() {
+  _updateScrollMode(pageNumber = null) {
-    // No-op in the base class.
+    const scrollMode = this._scrollMode, viewer = this.viewer;
+
+    if (scrollMode === ScrollMode.HORIZONTAL) {
+      viewer.classList.add('scrollHorizontal');
+    } else {
+      viewer.classList.remove('scrollHorizontal');
+    }
+    if (scrollMode === ScrollMode.WRAPPED) {
+      viewer.classList.add('scrollWrapped');
+    } else {
+      viewer.classList.remove('scrollWrapped');
+    }
+
+    if (!this.pdfDocument || !pageNumber) {
+      return;
+    }
+    // Non-numeric scale values can be sensitive to the scroll orientation.
+    // Call this before re-scrolling to the current page, to ensure that any
+    // changes in scale don't move the current page.
+    if (this._currentScaleValue && isNaN(this._currentScaleValue)) {
+      this._setScale(this._currentScaleValue, true);
+    }
+    this.scrollPageIntoView({ pageNumber, });
+    this.update();
   }
 
-  setSpreadMode(mode) {
+  setScrollMode(mode) {
+    if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) {
+      console.error(`${this._name}.setScrollMode() is deprecated, ` +
+                    `use the ${this._name}.scrollMode setter instead.`);
+      this.scrollMode = mode;
+    }
+  }
+
+  /**
+   * @return {number} One of the values in {SpreadMode}.
+   */
+  get spreadMode() {
+    return this._spreadMode;
+  }
+
+  /**
+   * @param {number} mode - Group the pages in spreads, starting with odd- or
+   *   even-number pages (unless `SpreadMode.NONE` is used).
+   *   The constants from {SpreadMode} should be used.
+   */
+  set spreadMode(mode) {
+    if (this._spreadMode === mode) {
+      return; // The Spread mode didn't change.
+    }
     if (!Number.isInteger(mode) || !Object.values(SpreadMode).includes(mode)) {
       throw new Error(`Invalid spread mode: ${mode}`);
     }
-    this.spreadMode = mode;
+    this._spreadMode = mode;
+    this.eventBus.dispatch('spreadmodechanged', { source: this, mode, });
+
+    this._updateSpreadMode(/* pageNumber = */ this._currentPageNumber);
   }
 
-  _regroupSpreads() {
+  _updateSpreadMode(pageNumber = null) {
-    // No-op in the base class.
+    if (!this.pdfDocument) {
+      return;
+    }
+    const viewer = this.viewer, pages = this._pages;
+    // Temporarily remove all the pages from the DOM.
+    viewer.textContent = '';
+
+    if (this._spreadMode === SpreadMode.NONE) {
+      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
+        viewer.appendChild(pages[i].div);
+      }
+    } else {
+      const parity = this._spreadMode - 1;
+      let spread = null;
+      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
+        if (spread === null) {
+          spread = document.createElement('div');
+          spread.className = 'spread';
+          viewer.appendChild(spread);
+        } else if (i % 2 === parity) {
+          spread = spread.cloneNode(false);
+          viewer.appendChild(spread);
+        }
+        spread.appendChild(pages[i].div);
+      }
+    }
+
+    if (!pageNumber) {
+      return;
+    }
+    this.scrollPageIntoView({ pageNumber, });
+    this.update();
+  }
+
+  setSpreadMode(mode) {
+    if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) {
+      console.error(`${this._name}.setSpreadMode() is deprecated, ` +
+                    `use the ${this._name}.spreadMode setter instead.`);
+      this.spreadMode = mode;
+    }
   }
 }
 

web/pdf_single_page_viewer.js

@@ -147,6 +147,10 @@ class PDFSinglePageViewer extends BaseViewer {
     // The Scroll/Spread modes are never used in `PDFSinglePageViewer`.
     return shadow(this, '_isScrollModeHorizontal', false);
   }
+
+  _updateScrollMode() { }
+
+  _updateSpreadMode() { }
 }
 
 export {

web/pdf_viewer.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { BaseViewer, ScrollMode, SpreadMode } from './base_viewer';
+import { BaseViewer, ScrollMode } from './base_viewer';
 import { getVisibleElements, scrollIntoView } from './ui_utils';
 import { shadow } from 'pdfjs-lib';
 
@@ -27,7 +27,7 @@ class PDFViewer extends BaseViewer {
       const left = pageDiv.offsetLeft + pageDiv.clientLeft;
       const right = left + pageDiv.clientWidth;
       const { scrollLeft, clientWidth, } = this.container;
-      if (this.scrollMode === ScrollMode.HORIZONTAL ||
+      if (this._scrollMode === ScrollMode.HORIZONTAL ||
           left < scrollLeft || right > scrollLeft + clientWidth) {
         pageSpot = { left: 0, top: 0, };
       }
@@ -38,7 +38,7 @@ class PDFViewer extends BaseViewer {
   _getVisiblePages() {
     if (!this.isInPresentationMode) {
       return getVisibleElements(this.container, this._pages, true,
-                                this.scrollMode === ScrollMode.HORIZONTAL);
+                                this._scrollMode === ScrollMode.HORIZONTAL);
     }
     // The algorithm in getVisibleElements doesn't work in all browsers and
     // configurations when presentation mode is active.
@@ -91,86 +91,7 @@ class PDFViewer extends BaseViewer {
     // Used to ensure that pre-rendering of the next/previous page works
     // correctly, since Scroll/Spread modes are ignored in Presentation Mode.
     return (this.isInPresentationMode ?
-            false : this.scrollMode === ScrollMode.HORIZONTAL);
+            false : this._scrollMode === ScrollMode.HORIZONTAL);
-  }
-
-  setScrollMode(mode) {
-    if (mode === this.scrollMode) {
-      return;
-    }
-    super.setScrollMode(mode);
-
-    this.eventBus.dispatch('scrollmodechanged', { mode, });
-    this._updateScrollModeClasses();
-
-    if (!this.pdfDocument) {
-      return;
-    }
-    const pageNumber = this._currentPageNumber;
-    // Non-numeric scale modes can be sensitive to the scroll orientation.
-    // Call this before re-scrolling to the current page, to ensure that any
-    // changes in scale don't move the current page.
-    if (isNaN(this._currentScaleValue)) {
-      this._setScale(this._currentScaleValue, true);
-    }
-    this.scrollPageIntoView({ pageNumber, });
-    this.update();
-  }
-
-  _updateScrollModeClasses() {
-    const { scrollMode, viewer, } = this;
-
-    if (scrollMode === ScrollMode.HORIZONTAL) {
-      viewer.classList.add('scrollHorizontal');
-    } else {
-      viewer.classList.remove('scrollHorizontal');
-    }
-    if (scrollMode === ScrollMode.WRAPPED) {
-      viewer.classList.add('scrollWrapped');
-    } else {
-      viewer.classList.remove('scrollWrapped');
-    }
-  }
-
-  setSpreadMode(mode) {
-    if (mode === this.spreadMode) {
-      return;
-    }
-    super.setSpreadMode(mode);
-
-    this.eventBus.dispatch('spreadmodechanged', { mode, });
-    this._regroupSpreads();
-  }
-
-  _regroupSpreads() {
-    if (!this.pdfDocument) {
-      return;
-    }
-    const viewer = this.viewer, pages = this._pages;
-    // Temporarily remove all the pages from the DOM.
-    viewer.textContent = '';
-
-    if (this.spreadMode === SpreadMode.NONE) {
-      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
-        viewer.appendChild(pages[i].div);
-      }
-    } else {
-      const parity = this.spreadMode - 1;
-      let spread = null;
-      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
-        if (spread === null) {
-          spread = document.createElement('div');
-          spread.className = 'spread';
-          viewer.appendChild(spread);
-        } else if (i % 2 === parity) {
-          spread = spread.cloneNode(false);
-          viewer.appendChild(spread);
-        }
-        spread.appendChild(pages[i].div);
-      }
-    }
-    this.scrollPageIntoView({ pageNumber: this._currentPageNumber, });
-    this.update();
   }
 }
 

web/secondary_toolbar.js

@@ -140,6 +140,10 @@ class SecondaryToolbar {
     this.pageNumber = 0;
     this.pagesCount = 0;
     this._updateUIState();
+
+    // Reset the Scroll/Spread buttons too, since they're document specific.
+    this.eventBus.dispatch('resetscrollmode', { source: this, });
+    this.eventBus.dispatch('resetspreadmode', { source: this, });
   }
 
   _updateUIState() {
@@ -189,7 +193,7 @@ class SecondaryToolbar {
   }
 
   _bindScrollModeListener(buttons) {
-    this.eventBus.on('scrollmodechanged', function(evt) {
+    function scrollModeChanged(evt) {
       buttons.scrollVerticalButton.classList.remove('toggled');
       buttons.scrollHorizontalButton.classList.remove('toggled');
       buttons.scrollWrappedButton.classList.remove('toggled');
@@ -205,11 +209,18 @@ class SecondaryToolbar {
           buttons.scrollWrappedButton.classList.add('toggled');
           break;
       }
+    }
+    this.eventBus.on('scrollmodechanged', scrollModeChanged);
+
+    this.eventBus.on('resetscrollmode', (evt) => {
+      if (evt.source === this) {
+        scrollModeChanged({ mode: ScrollMode.VERTICAL, });
+      }
     });
   }
 
   _bindSpreadModeListener(buttons) {
-    this.eventBus.on('spreadmodechanged', function(evt) {
+    function spreadModeChanged(evt) {
       buttons.spreadNoneButton.classList.remove('toggled');
       buttons.spreadOddButton.classList.remove('toggled');
       buttons.spreadEvenButton.classList.remove('toggled');
@@ -225,6 +236,13 @@ class SecondaryToolbar {
           buttons.spreadEvenButton.classList.add('toggled');
           break;
       }
+    }
+    this.eventBus.on('spreadmodechanged', spreadModeChanged);
+
+    this.eventBus.on('resetspreadmode', (evt) => {
+      if (evt.source === this) {
+        spreadModeChanged({ mode: SpreadMode.NONE, });
+      }
     });
   }