From a7e70a50f5e59a8673731d66825a77210542c486 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Thu, 6 Dec 2018 18:32:41 +0100
Subject: [PATCH] Add OpenAction destination support, off by default, to the
 viewer

Given that it's really not clear to me if this is actually desired functionality in the default viewer, and considering that it doesn't fit in *great* with the way that `PDFHistory` is initialized, this feature is currently off by default[1].

---
[1] It's controlled with the `disableOpenActionDestination` Preference/AppOption.
---
 extensions/chromium/preferences_schema.json |  4 ++++
 web/app.js                                  | 17 ++++++++++++---
 web/app_options.js                          |  5 +++++
 web/default_preferences.json                |  1 +
 web/interfaces.js                           |  2 +-
 web/pdf_history.js                          | 23 +++++++++++++++------
 6 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json
index 696090120..019577cfd 100644
--- a/extensions/chromium/preferences_schema.json
+++ b/extensions/chromium/preferences_schema.json
@@ -117,6 +117,10 @@
       ],
       "default": 0
     },
+    "disableOpenActionDestination": {
+      "type": "boolean",
+      "default": true
+    },
     "disablePageLabels": {
       "type": "boolean",
       "default": false
diff --git a/web/app.js b/web/app.js
index a4229a81d..420125a87 100644
--- a/web/app.js
+++ b/web/app.js
@@ -867,7 +867,9 @@ let PDFViewerApplication = {
 
     // Since the `setInitialView` call below depends on this being resolved,
     // fetch it early to avoid delaying initial rendering of the PDF document.
-    let pageModePromise = pdfDocument.getPageMode().catch(
+    const pageModePromise = pdfDocument.getPageMode().catch(
+      function() { /* Avoid breaking initial rendering; ignoring errors. */ });
+    const openActionDestPromise = pdfDocument.getOpenActionDestination().catch(
       function() { /* Avoid breaking initial rendering; ignoring errors. */ });
 
     this.toolbar.setPagesCount(pdfDocument.numPages, false);
@@ -923,8 +925,17 @@ let PDFViewerApplication = {
       }).catch(() => { /* Unable to read from storage; ignoring errors. */ });
 
       Promise.all([
-        storePromise, pageModePromise,
-      ]).then(async ([values = {}, pageMode]) => {
+        storePromise, pageModePromise, openActionDestPromise,
+      ]).then(async ([values = {}, pageMode, openActionDest]) => {
+        if (openActionDest && !this.initialBookmark &&
+            !AppOptions.get('disableOpenActionDestination')) {
+          // Always let the browser history/document hash take precedence.
+          this.initialBookmark = JSON.stringify(openActionDest);
+          // TODO: Re-factor the `PDFHistory` initialization to remove this hack
+          // that's currently necessary to prevent weird initial history state.
+          this.pdfHistory.push({ explicitDest: openActionDest,
+                                 pageNumber: null, });
+        }
         const initialBookmark = this.initialBookmark;
         // Initialize the default values, from user preferences.
         const zoom = AppOptions.get('defaultZoomValue');
diff --git a/web/app_options.js b/web/app_options.js
index 220b28455..bbd8e6c82 100644
--- a/web/app_options.js
+++ b/web/app_options.js
@@ -48,6 +48,11 @@ const defaultOptions = {
     value: false,
     kind: OptionKind.VIEWER,
   },
+  disableOpenActionDestination: {
+    /** @type {boolean} */
+    value: true,
+    kind: OptionKind.VIEWER,
+  },
   disablePageLabels: {
     /** @type {boolean} */
     value: false,
diff --git a/web/default_preferences.json b/web/default_preferences.json
index 3389a5cf8..f41aa1487 100644
--- a/web/default_preferences.json
+++ b/web/default_preferences.json
@@ -16,6 +16,7 @@
   "renderer": "canvas",
   "renderInteractiveForms": false,
   "enablePrintAutoRotate": false,
+  "disableOpenActionDestination": true,
   "disablePageMode": false,
   "disablePageLabels": false,
   "scrollModeOnLoad": 0,
diff --git a/web/interfaces.js b/web/interfaces.js
index 98fe4bd2d..a7f58a2c8 100644
--- a/web/interfaces.js
+++ b/web/interfaces.js
@@ -94,7 +94,7 @@ class IPDFHistory {
   /**
    * @param {Object} params
    */
-  push({ namedDest, explicitDest, pageNumber, }) {}
+  push({ namedDest = null, explicitDest, pageNumber, }) {}
 
   pushCurrentPosition() {}
 
diff --git a/web/pdf_history.js b/web/pdf_history.js
index 7c02dbe34..cbf8f3869 100644
--- a/web/pdf_history.js
+++ b/web/pdf_history.js
@@ -158,16 +158,27 @@ class PDFHistory {
    * Push an internal destination to the browser history.
    * @param {PushParameters}
    */
-  push({ namedDest, explicitDest, pageNumber, }) {
+  push({ namedDest = null, explicitDest, pageNumber, }) {
     if (!this.initialized) {
       return;
     }
-    if ((namedDest && typeof namedDest !== 'string') ||
-        !Array.isArray(explicitDest) ||
-        !(Number.isInteger(pageNumber) &&
-          pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) {
-      console.error('PDFHistory.push: Invalid parameters.');
+    if (namedDest && typeof namedDest !== 'string') {
+      console.error('PDFHistory.push: ' +
+                    `"${namedDest}" is not a valid namedDest parameter.`);
       return;
+    } else if (!Array.isArray(explicitDest)) {
+      console.error('PDFHistory.push: ' +
+                    `"${explicitDest}" is not a valid explicitDest parameter.`);
+      return;
+    } else if (!(Number.isInteger(pageNumber) &&
+                 pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) {
+      // Allow an unset `pageNumber` if and only if the history is still empty;
+      // please refer to the `this._destination.page = null;` comment above.
+      if (pageNumber !== null || this._destination) {
+        console.error('PDFHistory.push: ' +
+                      `"${pageNumber}" is not a valid pageNumber parameter.`);
+        return;
+      }
     }
 
     let hash = namedDest || JSON.stringify(explicitDest);