From 2e38b7d00b780dcab1a15012f3f7614997de8e42 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 9 Nov 2018 09:45:09 +0100
Subject: [PATCH 1/2] Update `BaseViewer.scrollPageIntoView` to always validate
 the `pageNumber` parameter

Note that when e.g. presentation mode is active, we fail[1] to ensure that the `pageNumber` parameter is actually an integer before calling `_setCurrentPageNumber` (that method expects the argument be an integer).
Also changes the method signature, of `scrollPageIntoView`, to use object destructuring instead.

---
[1] Most likely, this is actually *my* oversight :-)
---
 web/base_viewer.js | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/web/base_viewer.js b/web/base_viewer.js
index 19b3c344a..730e9ff94 100644
--- a/web/base_viewer.js
+++ b/web/base_viewer.js
@@ -660,25 +660,23 @@ class BaseViewer {
    * Scrolls page into view.
    * @param {ScrollPageIntoViewParameters} params
    */
-  scrollPageIntoView(params) {
+  scrollPageIntoView({ pageNumber, destArray = null,
+                       allowNegativeOffset = false, }) {
     if (!this.pdfDocument) {
       return;
     }
-    let pageNumber = params.pageNumber || 0;
-    let dest = params.destArray || null;
-    let allowNegativeOffset = params.allowNegativeOffset || false;
-
-    if (this.isInPresentationMode || !dest) {
-      this._setCurrentPageNumber(pageNumber, /* resetCurrentPageView = */ true);
-      return;
-    }
-
-    let pageView = this._pages[pageNumber - 1];
+    const pageView = (Number.isInteger(pageNumber) &&
+                      this._pages[pageNumber - 1]);
     if (!pageView) {
       console.error(
         `${this._name}.scrollPageIntoView: Invalid "pageNumber" parameter.`);
       return;
     }
+
+    if (this.isInPresentationMode || !destArray) {
+      this._setCurrentPageNumber(pageNumber, /* resetCurrentPageView = */ true);
+      return;
+    }
     let x = 0, y = 0;
     let width = 0, height = 0, widthScale, heightScale;
     let changeOrientation = (pageView.rotation % 180 === 0 ? false : true);
@@ -687,11 +685,11 @@ class BaseViewer {
     let pageHeight = (changeOrientation ? pageView.width : pageView.height) /
       pageView.scale / CSS_UNITS;
     let scale = 0;
-    switch (dest[1].name) {
+    switch (destArray[1].name) {
       case 'XYZ':
-        x = dest[2];
-        y = dest[3];
-        scale = dest[4];
+        x = destArray[2];
+        y = destArray[3];
+        scale = destArray[4];
         // If x and/or y coordinates are not supplied, default to
         // _top_ left of the page (not the obvious bottom left,
         // since aligning the bottom of the intended page with the
@@ -705,7 +703,7 @@ class BaseViewer {
         break;
       case 'FitH':
       case 'FitBH':
-        y = dest[2];
+        y = destArray[2];
         scale = 'page-width';
         // According to the PDF spec, section 12.3.2.2, a `null` value in the
         // parameter should maintain the position relative to the new page.
@@ -716,16 +714,16 @@ class BaseViewer {
         break;
       case 'FitV':
       case 'FitBV':
-        x = dest[2];
+        x = destArray[2];
         width = pageWidth;
         height = pageHeight;
         scale = 'page-height';
         break;
       case 'FitR':
-        x = dest[2];
-        y = dest[3];
-        width = dest[4] - x;
-        height = dest[5] - y;
+        x = destArray[2];
+        y = destArray[3];
+        width = destArray[4] - x;
+        height = destArray[5] - y;
         let hPadding = this.removePageBorders ? 0 : SCROLLBAR_PADDING;
         let vPadding = this.removePageBorders ? 0 : VERTICAL_PADDING;
 
@@ -736,8 +734,8 @@ class BaseViewer {
         scale = Math.min(Math.abs(widthScale), Math.abs(heightScale));
         break;
       default:
-        console.error(`${this._name}.scrollPageIntoView: "${dest[1].name}" ` +
-                      'is not a valid destination type.');
+        console.error(`${this._name}.scrollPageIntoView: ` +
+          `"${destArray[1].name}" is not a valid destination type.`);
         return;
     }
 
@@ -747,7 +745,7 @@ class BaseViewer {
       this.currentScaleValue = DEFAULT_SCALE_VALUE;
     }
 
-    if (scale === 'page-fit' && !dest[4]) {
+    if (scale === 'page-fit' && !destArray[4]) {
       this._scrollIntoView({
         pageDiv: pageView.div,
         pageNumber,

From e16e072cb3caea1f5d5045e8a59198644120cd34 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 9 Nov 2018 10:16:40 +0100
Subject: [PATCH 2/2] Directly call `_setCurrentPageNumber` when updating the
 Scroll/Spread modes in `BaseViewer`

Given that the `_updateScrollMode`/`_updateSpreadMode` methods are "private", there's no particular reason to not just directly call `_setCurrentPageNumber`.
---
 web/base_viewer.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/web/base_viewer.js b/web/base_viewer.js
index 730e9ff94..352fe1c81 100644
--- a/web/base_viewer.js
+++ b/web/base_viewer.js
@@ -1086,7 +1086,7 @@ class BaseViewer {
     if (this._currentScaleValue && isNaN(this._currentScaleValue)) {
       this._setScale(this._currentScaleValue, true);
     }
-    this.scrollPageIntoView({ pageNumber, });
+    this._setCurrentPageNumber(pageNumber, /* resetCurrentPageView = */ true);
     this.update();
   }
 
@@ -1146,7 +1146,7 @@ class BaseViewer {
     if (!pageNumber) {
       return;
     }
-    this.scrollPageIntoView({ pageNumber, });
+    this._setCurrentPageNumber(pageNumber, /* resetCurrentPageView = */ true);
     this.update();
   }
 }