Merge pull request mozilla#8775 from Snuffleupagus/rewrite-PDFHistory-2

Re-write `PDFHistory` from scratch

test/unit/clitests.json

@@ -17,6 +17,7 @@
     "murmurhash3_spec.js",
     "node_stream_spec.js",
     "parser_spec.js",
+    "pdf_history.js",
     "primitives_spec.js",
     "stream_spec.js",
     "type1_parser_spec.js",

test/unit/jasmine-boot.js

@@ -64,6 +64,7 @@ function initializePDFJS(callback) {
     'pdfjs-test/unit/murmurhash3_spec',
     'pdfjs-test/unit/network_spec',
     'pdfjs-test/unit/parser_spec',
+    'pdfjs-test/unit/pdf_history_spec',
     'pdfjs-test/unit/primitives_spec',
     'pdfjs-test/unit/stream_spec',
     'pdfjs-test/unit/type1_parser_spec',

test/unit/pdf_history_spec.js

@@ -0,0 +1,45 @@
+/* Copyright 2017 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { isDestsEqual } from '../../web/pdf_history';
+
+describe('pdf_history', function() {
+  describe('isDestsEqual', function() {
+    let firstDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 0, 375, null];
+    let secondDest = [{ num: 5, gen: 0, }, { name: 'XYZ', }, 0, 375, null];
+    let thirdDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 750, 0, null];
+    let fourthDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 0, 375, 1.0];
+    let fifthDest = [{ gen: 0, num: 1, }, { name: 'XYZ', }, 0, 375, null];
+
+    it('should reject non-equal destination arrays', function() {
+      expect(isDestsEqual(firstDest, undefined)).toEqual(false);
+      expect(isDestsEqual(firstDest, [1, 2, 3, 4, 5])).toEqual(false);
+
+      expect(isDestsEqual(firstDest, secondDest)).toEqual(false);
+      expect(isDestsEqual(firstDest, thirdDest)).toEqual(false);
+      expect(isDestsEqual(firstDest, fourthDest)).toEqual(false);
+    });
+
+    it('should accept equal destination arrays', function() {
+      expect(isDestsEqual(firstDest, firstDest)).toEqual(true);
+      expect(isDestsEqual(firstDest, fifthDest)).toEqual(true);
+
+      let firstDestCopy = firstDest.slice();
+      expect(firstDest).not.toBe(firstDestCopy);
+
+      expect(isDestsEqual(firstDest, firstDestCopy)).toEqual(true);
+    });
+  });
+});

test/unit/ui_utils_spec.js

@@ -14,7 +14,8 @@
  */
 
 import {
-  binarySearchFirstItem, EventBus, getPDFFileNameFromURL
+  binarySearchFirstItem, EventBus, getPDFFileNameFromURL, waitOnEventOrTimeout,
+  WaitOnType
 } from '../../web/ui_utils';
 import { createObjectURL, isNodeJS } from '../../src/shared/util';
 
@@ -259,4 +260,118 @@ describe('ui_utils', function() {
       expect(count).toEqual(2);
     });
   });
+
+  describe('waitOnEventOrTimeout', function() {
+    let eventBus;
+
+    beforeAll(function(done) {
+      eventBus = new EventBus();
+      done();
+    });
+
+    afterAll(function() {
+      eventBus = null;
+    });
+
+    it('should reject invalid parameters', function(done) {
+      let invalidTarget = waitOnEventOrTimeout({
+        target: 'window',
+        name: 'DOMContentLoaded',
+      }).then(function() {
+        throw new Error('Should reject invalid parameters.');
+      }, function(reason) {
+        expect(reason instanceof Error).toEqual(true);
+      });
+
+      let invalidName = waitOnEventOrTimeout({
+        target: eventBus,
+        name: '',
+      }).then(function() {
+        throw new Error('Should reject invalid parameters.');
+      }, function(reason) {
+        expect(reason instanceof Error).toEqual(true);
+      });
+
+      let invalidDelay = waitOnEventOrTimeout({
+        target: eventBus,
+        name: 'pagerendered',
+        delay: -1000,
+      }).then(function() {
+        throw new Error('Should reject invalid parameters.');
+      }, function(reason) {
+        expect(reason instanceof Error).toEqual(true);
+      });
+
+      Promise.all([invalidTarget, invalidName, invalidDelay]).then(done,
+                                                                   done.fail);
+    });
+
+    it('should resolve on event, using the DOM', function(done) {
+      if (isNodeJS()) {
+        pending('Document in not supported in Node.js.');
+      }
+      let button = document.createElement('button');
+
+      let buttonClicked = waitOnEventOrTimeout({
+        target: button,
+        name: 'click',
+        delay: 10000,
+      });
+      // Immediately dispatch the expected event.
+      button.click();
+
+      buttonClicked.then(function(type) {
+        expect(type).toEqual(WaitOnType.EVENT);
+        done();
+      }, done.fail);
+    });
+
+    it('should resolve on timeout, using the DOM', function(done) {
+      if (isNodeJS()) {
+        pending('Document in not supported in Node.js.');
+      }
+      let button = document.createElement('button');
+
+      let buttonClicked = waitOnEventOrTimeout({
+        target: button,
+        name: 'click',
+        delay: 10,
+      });
+      // Do *not* dispatch the event, and wait for the timeout.
+
+      buttonClicked.then(function(type) {
+        expect(type).toEqual(WaitOnType.TIMEOUT);
+        done();
+      }, done.fail);
+    });
+
+    it('should resolve on event, using the EventBus', function(done) {
+      let pageRendered = waitOnEventOrTimeout({
+        target: eventBus,
+        name: 'pagerendered',
+        delay: 10000,
+      });
+      // Immediately dispatch the expected event.
+      eventBus.dispatch('pagerendered');
+
+      pageRendered.then(function(type) {
+        expect(type).toEqual(WaitOnType.EVENT);
+        done();
+      }, done.fail);
+    });
+
+    it('should resolve on timeout, using the EventBus', function(done) {
+      let pageRendered = waitOnEventOrTimeout({
+        target: eventBus,
+        name: 'pagerendered',
+        delay: 10,
+      });
+      // Do *not* dispatch the event, and wait for the timeout.
+
+      pageRendered.then(function(type) {
+        expect(type).toEqual(WaitOnType.TIMEOUT);
+        done();
+      }, done.fail);
+    });
+  });
 });

web/app.js

@@ -89,7 +89,6 @@ const DefaultExternalServices = {
 
 let PDFViewerApplication = {
   initialBookmark: document.location.hash.substring(1),
-  initialDestination: null,
   initialized: false,
   fellback: false,
   appConfig: null,
@@ -931,21 +930,16 @@ let PDFViewerApplication = {
       if (!PDFJS.disableHistory && !this.isViewerEmbedded) {
         // The browsing history is only enabled when the viewer is standalone,
         // i.e. not when it is embedded in a web page.
-        if (!this.viewerPrefs['showPreviousViewOnLoad']) {
-          this.pdfHistory.clearHistoryState();
-        }
-        this.pdfHistory.initialize(this.documentFingerprint);
+        let resetHistory = !this.viewerPrefs['showPreviousViewOnLoad'];
+        this.pdfHistory.initialize(id, resetHistory);
 
-        if (this.pdfHistory.initialDestination) {
-          this.initialDestination = this.pdfHistory.initialDestination;
-        } else if (this.pdfHistory.initialBookmark) {
+        if (this.pdfHistory.initialBookmark) {
           this.initialBookmark = this.pdfHistory.initialBookmark;
         }
       }
 
       let initialParams = {
-        destination: this.initialDestination,
-        bookmark: this.initialBookmark,
+        bookmark: null,
         hash: null,
       };
       let storePromise = store.getMultiple({
@@ -979,9 +973,11 @@ let PDFViewerApplication = {
           sidebarView,
         };
       }).then(({ hash, sidebarView, }) => {
-        this.setInitialView(hash, { sidebarView, });
+        initialParams.bookmark = this.initialBookmark;
         initialParams.hash = hash;
 
+        this.setInitialView(hash, { sidebarView, });
+
         // Make all navigation keys work on document load,
         // unless the viewer is embedded in a web page.
         if (!this.isViewerEmbedded) {
@@ -991,14 +987,12 @@ let PDFViewerApplication = {
       }).then(() => {
         // For documents with different page sizes, once all pages are resolved,
         // ensure that the correct location becomes visible on load.
-        if (!initialParams.destination && !initialParams.bookmark &&
-            !initialParams.hash) {
+        if (!initialParams.bookmark && !initialParams.hash) {
           return;
         }
         if (pdfViewer.hasEqualPageSizes) {
           return;
         }
-        this.initialDestination = initialParams.destination;
         this.initialBookmark = initialParams.bookmark;
 
         pdfViewer.currentScaleValue = pdfViewer.currentScaleValue;
@@ -1141,12 +1135,8 @@ let PDFViewerApplication = {
     this.isInitialViewSet = true;
     this.pdfSidebar.setInitialView(sidebarView);
 
-    if (this.initialDestination) {
-      this.pdfLinkService.navigateTo(this.initialDestination);
-      this.initialDestination = null;
-    } else if (this.initialBookmark) {
+    if (this.initialBookmark) {
       this.pdfLinkService.setHash(this.initialBookmark);
-      this.pdfHistory.push({ hash: this.initialBookmark, }, true);
       this.initialBookmark = null;
     } else if (storedHash) {
       this.pdfLinkService.setHash(storedHash);
@@ -1787,10 +1777,6 @@ function webViewerUpdateViewarea(evt) {
   PDFViewerApplication.appConfig.secondaryToolbar.viewBookmarkButton.href =
     href;
 
-  // Update the current bookmark in the browsing history.
-  PDFViewerApplication.pdfHistory.updateCurrentBookmark(location.pdfOpenParams,
-                                                        location.pageNumber);
-
   // Show/hide the loading indicator in the page number input element.
   let currentPage =
     PDFViewerApplication.pdfViewer.getPageView(PDFViewerApplication.page - 1);
@@ -1814,16 +1800,14 @@ function webViewerResize() {
 }
 
 function webViewerHashchange(evt) {
-  if (PDFViewerApplication.pdfHistory.isHashChangeUnlocked) {
-    let hash = evt.hash;
-    if (!hash) {
-      return;
-    }
-    if (!PDFViewerApplication.isInitialViewSet) {
-      PDFViewerApplication.initialBookmark = hash;
-    } else {
-      PDFViewerApplication.pdfLinkService.setHash(hash);
-    }
+  let hash = evt.hash;
+  if (!hash) {
+    return;
+  }
+  if (!PDFViewerApplication.isInitialViewSet) {
+    PDFViewerApplication.initialBookmark = hash;
+  } else if (!PDFViewerApplication.pdfHistory.popStateInProgress) {
+    PDFViewerApplication.pdfLinkService.setHash(hash);
   }
 }
 
@@ -2277,23 +2261,6 @@ function webViewerKeyDown(evt) {
     }
   }
 
-  if (cmd === 2) { // alt-key
-    switch (evt.keyCode) {
-      case 37: // left arrow
-        if (isViewerInPresentationMode) {
-          PDFViewerApplication.pdfHistory.back();
-          handled = true;
-        }
-        break;
-      case 39: // right arrow
-        if (isViewerInPresentationMode) {
-          PDFViewerApplication.pdfHistory.forward();
-          handled = true;
-        }
-        break;
-    }
-  }
-
   if (ensureViewerFocused && !pdfViewer.containsElement(curElement)) {
     // The page container is not focused, but a page navigation key has been
     // pressed. Change the focus to the viewer container to make sure that

web/interfaces.js

@@ -73,10 +73,22 @@ class IPDFLinkService {
  * @interface
  */
 class IPDFHistory {
-  forward() {}
+  /**
+   * @param {string} fingerprint - The PDF document's unique fingerprint.
+   * @param {boolean} resetHistory - (optional) Reset the browsing history.
+   */
+  initialize(fingerprint, resetHistory = false) {}
+
+  /**
+   * @param {Object} params
+   */
+  push({ namedDest, explicitDest, pageNumber, }) {}
+
+  pushCurrentPosition() {}
+
   back() {}
-  push(params) {}
-  updateNextHashParam(hash) {}
+
+  forward() {}
 }
 
 /**