Merge pull request #12493 from Snuffleupagus/PDFHistory.pushPage

timvandermeij · web-flow · commit e389ed6201df · 2020-10-20T22:20:11.000+02:00
Support adding pages, in addition to regular destinations, to the browser history and use it with thumbnails (issue 12440)
diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
@@ -344,7 +344,7 @@ class LinkAnnotationElement extends AnnotationElement {
     link.href = this.linkService.getDestinationHash(destination);
     link.onclick = () => {
       if (destination) {
-        this.linkService.navigateTo(destination);
+        this.linkService.goToDestination(destination);
       }
       return false;
     };
diff --git a/web/interfaces.js b/web/interfaces.js
@@ -54,9 +54,14 @@ class IPDFLinkService {
   set externalLinkEnabled(value) {}
 
   /**
-   * @param dest - The PDF destination object.
+   * @param {string|Array} dest - The named, or explicit, PDF destination.
+   */
+  async goToDestination(dest) {}
+
+  /**
+   * @param {number} pageNumber - The page number.
    */
-  navigateTo(dest) {}
+  goToPage(pageNumber) {}
 
   /**
    * @param dest - The PDF destination object.
@@ -108,6 +113,11 @@ class IPDFHistory {
    */
   push({ namedDest = null, explicitDest, pageNumber }) {}
 
+  /**
+   * @param {number} pageNumber
+   */
+  pushPage(pageNumber) {}
+
   pushCurrentPosition() {}
 
   back() {}
diff --git a/web/pdf_history.js b/web/pdf_history.js
@@ -143,9 +143,6 @@ class PDFHistory {
       state.uid,
       /* removeTemporary = */ true
     );
-    if (this._uid > this._maxUid) {
-      this._maxUid = this._uid;
-    }
 
     if (destination.rotation !== undefined) {
       this._initialRotation = destination.rotation;
@@ -271,6 +268,55 @@ class PDFHistory {
     }
   }
 
+  /**
+   * Push a page to the browser history; generally the `push` method should be
+   * used instead.
+   * @param {number} pageNumber
+   */
+  pushPage(pageNumber) {
+    if (!this._initialized) {
+      return;
+    }
+    if (
+      !(
+        Number.isInteger(pageNumber) &&
+        pageNumber > 0 &&
+        pageNumber <= this.linkService.pagesCount
+      )
+    ) {
+      console.error(
+        `PDFHistory.pushPage: "${pageNumber}" is not a valid page number.`
+      );
+      return;
+    }
+
+    if (this._destination && this._destination.page === pageNumber) {
+      // When the new page is identical to the one in `this._destination`, we
+      // don't want to add a potential duplicate entry in the browser history.
+      return;
+    }
+    if (this._popStateInProgress) {
+      return;
+    }
+
+    this._pushOrReplaceState({
+      hash: `page=${pageNumber}`,
+      page: pageNumber,
+      rotation: this.linkService.rotation,
+    });
+
+    if (!this._popStateInProgress) {
+      // Prevent the browser history from updating while the new page is
+      // being scrolled into view, to avoid potentially inconsistent state.
+      this._popStateInProgress = true;
+      // We defer the resetting of `this._popStateInProgress`, to account for
+      // e.g. zooming occuring when the new page is being navigated to.
+      Promise.resolve().then(() => {
+        this._popStateInProgress = false;
+      });
+    }
+  }
+
   /**
    * Push the current position to the browser history.
    */
@@ -361,7 +407,6 @@ class PDFHistory {
     if (shouldReplace) {
       window.history.replaceState(newState, "", newUrl);
     } else {
-      this._maxUid = this._uid;
       window.history.pushState(newState, "", newUrl);
     }
 
@@ -485,6 +530,7 @@ class PDFHistory {
     }
     this._destination = destination;
     this._uid = uid;
+    this._maxUid = Math.max(this._maxUid, uid);
     // This should always be reset when `this._destination` is updated.
     this._numPositionUpdates = 0;
   }
@@ -639,23 +685,20 @@ class PDFHistory {
       state.uid,
       /* removeTemporary = */ true
     );
-    if (this._uid > this._maxUid) {
-      this._maxUid = this._uid;
-    }
 
     if (isValidRotation(destination.rotation)) {
       this.linkService.rotation = destination.rotation;
     }
     if (destination.dest) {
-      this.linkService.navigateTo(destination.dest);
+      this.linkService.goToDestination(destination.dest);
     } else if (destination.hash) {
       this.linkService.setHash(destination.hash);
     } else if (destination.page) {
       // Fallback case; shouldn't be necessary, but better safe than sorry.
       this.linkService.page = destination.page;
     }
 
-    // Since `PDFLinkService.navigateTo` is asynchronous, we thus defer the
+    // Since `PDFLinkService.goToDestination` is asynchronous, we thus defer the
     // resetting of `this._popStateInProgress` slightly.
     Promise.resolve().then(() => {
       this._popStateInProgress = false;
diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js
@@ -108,91 +108,127 @@ class PDFLinkService {
   }
 
   /**
-   * @param {string|Array} dest - The named, or explicit, PDF destination.
+   * @deprecated
    */
   navigateTo(dest) {
-    const goToDestination = ({ namedDest, explicitDest }) => {
-      // Dest array looks like that: <page-ref> </XYZ|/FitXXX> <args..>
-      const destRef = explicitDest[0];
-      let pageNumber;
-
-      if (destRef instanceof Object) {
-        pageNumber = this._cachedPageNumber(destRef);
-
-        if (pageNumber === null) {
-          // Fetch the page reference if it's not yet available. This could
-          // only occur during loading, before all pages have been resolved.
-          this.pdfDocument
-            .getPageIndex(destRef)
-            .then(pageIndex => {
-              this.cachePageRef(pageIndex + 1, destRef);
-              goToDestination({ namedDest, explicitDest });
-            })
-            .catch(() => {
-              console.error(
-                `PDFLinkService.navigateTo: "${destRef}" is not ` +
-                  `a valid page reference, for dest="${dest}".`
-              );
-            });
-          return;
-        }
-      } else if (Number.isInteger(destRef)) {
-        pageNumber = destRef + 1;
-      } else {
-        console.error(
-          `PDFLinkService.navigateTo: "${destRef}" is not ` +
-            `a valid destination reference, for dest="${dest}".`
-        );
-        return;
-      }
-      if (!pageNumber || pageNumber < 1 || pageNumber > this.pagesCount) {
-        console.error(
-          `PDFLinkService.navigateTo: "${pageNumber}" is not ` +
-            `a valid page number, for dest="${dest}".`
-        );
-        return;
-      }
-
-      if (this.pdfHistory) {
-        // Update the browser history before scrolling the new destination into
-        // view, to be able to accurately capture the current document position.
-        this.pdfHistory.pushCurrentPosition();
-        this.pdfHistory.push({ namedDest, explicitDest, pageNumber });
-      }
+    console.error(
+      "Deprecated method: `navigateTo`, use `goToDestination` instead."
+    );
+    this.goToDestination(dest);
+  }
 
-      this.pdfViewer.scrollPageIntoView({
-        pageNumber,
-        destArray: explicitDest,
-        ignoreDestinationZoom: this._ignoreDestinationZoom,
-      });
-    };
-
-    new Promise((resolve, reject) => {
-      if (typeof dest === "string") {
-        this.pdfDocument.getDestination(dest).then(destArray => {
-          resolve({
-            namedDest: dest,
-            explicitDest: destArray,
+  /**
+   * @private
+   */
+  _goToDestinationHelper(rawDest, namedDest = null, explicitDest) {
+    // Dest array looks like that: <page-ref> </XYZ|/FitXXX> <args..>
+    const destRef = explicitDest[0];
+    let pageNumber;
+
+    if (destRef instanceof Object) {
+      pageNumber = this._cachedPageNumber(destRef);
+
+      if (pageNumber === null) {
+        // Fetch the page reference if it's not yet available. This could
+        // only occur during loading, before all pages have been resolved.
+        this.pdfDocument
+          .getPageIndex(destRef)
+          .then(pageIndex => {
+            this.cachePageRef(pageIndex + 1, destRef);
+            this._goToDestinationHelper(rawDest, namedDest, explicitDest);
+          })
+          .catch(() => {
+            console.error(
+              `PDFLinkService._goToDestinationHelper: "${destRef}" is not ` +
+                `a valid page reference, for dest="${rawDest}".`
+            );
           });
-        });
-        return;
-      }
-      resolve({
-        namedDest: "",
-        explicitDest: dest,
-      });
-    }).then(data => {
-      if (!Array.isArray(data.explicitDest)) {
-        console.error(
-          `PDFLinkService.navigateTo: "${data.explicitDest}" is` +
-            ` not a valid destination array, for dest="${dest}".`
-        );
         return;
       }
-      goToDestination(data);
+    } else if (Number.isInteger(destRef)) {
+      pageNumber = destRef + 1;
+    } else {
+      console.error(
+        `PDFLinkService._goToDestinationHelper: "${destRef}" is not ` +
+          `a valid destination reference, for dest="${rawDest}".`
+      );
+      return;
+    }
+    if (!pageNumber || pageNumber < 1 || pageNumber > this.pagesCount) {
+      console.error(
+        `PDFLinkService._goToDestinationHelper: "${pageNumber}" is not ` +
+          `a valid page number, for dest="${rawDest}".`
+      );
+      return;
+    }
+
+    if (this.pdfHistory) {
+      // Update the browser history before scrolling the new destination into
+      // view, to be able to accurately capture the current document position.
+      this.pdfHistory.pushCurrentPosition();
+      this.pdfHistory.push({ namedDest, explicitDest, pageNumber });
+    }
+
+    this.pdfViewer.scrollPageIntoView({
+      pageNumber,
+      destArray: explicitDest,
+      ignoreDestinationZoom: this._ignoreDestinationZoom,
     });
   }
 
+  /**
+   * This method will, when available, also update the browser history.
+   *
+   * @param {string|Array} dest - The named, or explicit, PDF destination.
+   */
+  async goToDestination(dest) {
+    let namedDest, explicitDest;
+    if (typeof dest === "string") {
+      namedDest = dest;
+      explicitDest = await this.pdfDocument.getDestination(dest);
+    } else {
+      namedDest = null;
+      explicitDest = await dest;
+    }
+    if (!Array.isArray(explicitDest)) {
+      console.error(
+        `PDFLinkService.goToDestination: "${explicitDest}" is not ` +
+          `a valid destination array, for dest="${dest}".`
+      );
+      return;
+    }
+    this._goToDestinationHelper(dest, namedDest, explicitDest);
+  }
+
+  /**
+   * This method will, when available, also update the browser history.
+   *
+   * @param {number} pageNumber - The page number.
+   */
+  goToPage(pageNumber) {
+    if (
+      !(
+        Number.isInteger(pageNumber) &&
+        pageNumber > 0 &&
+        pageNumber <= this.pagesCount
+      )
+    ) {
+      console.error(
+        `PDFLinkService.goToPage: "${pageNumber}" is not a valid page number.`
+      );
+      return;
+    }
+
+    if (this.pdfHistory) {
+      // Update the browser history before scrolling the new page into view,
+      // to be able to accurately capture the current document position.
+      this.pdfHistory.pushCurrentPosition();
+      this.pdfHistory.pushPage(pageNumber);
+    }
+
+    this.pdfViewer.scrollPageIntoView({ pageNumber });
+  }
+
   /**
    * @param {string|Array} dest - The PDF destination object.
    * @returns {string} The hyperlink to the PDF object.
@@ -307,7 +343,7 @@ class PDFLinkService {
       // Ensure that this parameter is *always* handled last, in order to
       // guarantee that it won't be overridden (e.g. by the "page" parameter).
       if ("nameddest" in params) {
-        this.navigateTo(params.nameddest);
+        this.goToDestination(params.nameddest);
       }
     } else {
       // Named (or explicit) destination.
@@ -323,7 +359,7 @@ class PDFLinkService {
       } catch (ex) {}
 
       if (typeof dest === "string" || isValidExplicitDestination(dest)) {
-        this.navigateTo(dest);
+        this.goToDestination(dest);
         return;
       }
       console.error(
@@ -394,6 +430,9 @@ class PDFLinkService {
     this._pagesRefCache[refStr] = pageNum;
   }
 
+  /**
+   * @private
+   */
   _cachedPageNumber(pageRef) {
     const refStr =
       pageRef.gen === 0 ? `${pageRef.num}R` : `${pageRef.num}R${pageRef.gen}`;
@@ -510,9 +549,14 @@ class SimpleLinkService {
   set rotation(value) {}
 
   /**
-   * @param dest - The PDF destination object.
+   * @param {string|Array} dest - The named, or explicit, PDF destination.
+   */
+  async goToDestination(dest) {}
+
+  /**
+   * @param {number} pageNumber - The page number.
    */
-  navigateTo(dest) {}
+  goToPage(pageNumber) {}
 
   /**
    * @param dest - The PDF destination object.
diff --git a/web/pdf_outline_viewer.js b/web/pdf_outline_viewer.js
diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js

Original file line number	Diff line number	Diff line change
`@@ -344,7 +344,7 @@ class LinkAnnotationElement extends AnnotationElement {`
`344`	`344`	`link.href = this.linkService.getDestinationHash(destination);`
`345`	`345`	`link.onclick = () => {`
`346`	`346`	`if (destination) {`
`347`		`- this.linkService.navigateTo(destination);`
	`347`	`+ this.linkService.goToDestination(destination);`
`348`	`348`	`}`
`349`	`349`	`return false;`
`350`	`350`	`};`