Merge pull request #12147 from Snuffleupagus/fix-AnnotationStorage-usage

[api-minor] Fix the `AnnotationStorage` usage properly in the viewer/tests (PR 12107 and 12143 follow-up)
mozilla · Jul 31, 2020 · b037e59 · b037e59
2 parents c5663f2 + 95bfc43
commit b037e59
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 11 deletions.
diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
@@ -29,6 +29,7 @@ import {
   Util,
   warn,
 } from "../shared/util.js";
+import { AnnotationStorage } from "./annotation_storage.js";
 
 /**
  * @typedef {Object} AnnotationElementParameters
@@ -38,6 +39,7 @@ import {
  * @property {PageViewport} viewport
  * @property {IPDFLinkService} linkService
  * @property {DownloadManager} downloadManager
+ * @property {AnnotationStorage} [annotationStorage]
  * @property {string} [imageResourcesPath] - Path for image resources, mainly
  *   for annotation icons. Include trailing slash.
  * @property {boolean} renderInteractiveForms
@@ -1463,7 +1465,8 @@ class AnnotationLayer {
         imageResourcesPath: parameters.imageResourcesPath || "",
         renderInteractiveForms: parameters.renderInteractiveForms || false,
         svgFactory: new DOMSVGFactory(),
-        annotationStorage: parameters.annotationStorage,
+        annotationStorage:
+          parameters.annotationStorage || new AnnotationStorage(),
       });
       if (element.isRenderable) {
         parameters.div.appendChild(element.render());

diff --git a/src/display/annotation_storage.js b/src/display/annotation_storage.js
@@ -15,7 +15,7 @@
 
 class AnnotationStorage {
   constructor() {
-    this._storage = {};
+    this._storage = Object.create(null);
   }
 
   /**

diff --git a/src/pdf.js b/src/pdf.js
@@ -50,7 +50,6 @@ import {
   VerbosityLevel,
 } from "./shared/util.js";
 import { AnnotationLayer } from "./display/annotation_layer.js";
-import { AnnotationStorage } from "./display/annotation_storage.js";
 import { apiCompatibilityParams } from "./display/api_compatibility.js";
 import { GlobalWorkerOptions } from "./display/worker_options.js";
 import { renderTextLayer } from "./display/text_layer.js";
@@ -157,8 +156,6 @@ export {
   VerbosityLevel,
   // From "./display/annotation_layer.js":
   AnnotationLayer,
-  // From "./display/annotation_storage.js":
-  AnnotationStorage,
   // From "./display/api_compatibility.js":
   apiCompatibilityParams,
   // From "./display/worker_options.js":

diff --git a/test/driver.js b/test/driver.js
@@ -220,7 +220,6 @@ var rasterizeAnnotationLayer = (function rasterizeAnnotationLayerClosure() {
           linkService: new pdfjsViewer.SimpleLinkService(),
           imageResourcesPath,
           renderInteractiveForms,
-          annotationStorage: new pdfjsLib.AnnotationStorage(),
         };
         pdfjsLib.AnnotationLayer.render(parameters);
 

diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js
@@ -21,6 +21,7 @@ import { SimpleLinkService } from "./pdf_link_service.js";
  * @typedef {Object} AnnotationLayerBuilderOptions
  * @property {HTMLDivElement} pageDiv
  * @property {PDFPage} pdfPage
+ * @property {AnnotationStorage} [annotationStorage]
  * @property {string} [imageResourcesPath] - Path for image resources, mainly
  *   for annotation icons. Include trailing slash.
  * @property {boolean} renderInteractiveForms
@@ -38,7 +39,7 @@ class AnnotationLayerBuilder {
     pdfPage,
     linkService,
     downloadManager,
-    annotationStorage,
+    annotationStorage = null,
     imageResourcesPath = "",
     renderInteractiveForms = false,
     l10n = NullL10n,
@@ -65,6 +66,9 @@ class AnnotationLayerBuilder {
       if (this._cancelled) {
         return;
       }
+      if (annotations.length === 0) {
+        return;
+      }
 
       const parameters = {
         viewport: viewport.clone({ dontFlip: true }),
@@ -85,9 +89,6 @@ class AnnotationLayerBuilder {
       } else {
         // Create an annotation layer div and render the annotations
         // if there is at least one annotation.
-        if (annotations.length === 0) {
-          return;
-        }
         this.div = document.createElement("div");
         this.div.className = "annotationLayer";
         this.pageDiv.appendChild(this.div);
@@ -118,6 +119,7 @@ class DefaultAnnotationLayerFactory {
   /**
    * @param {HTMLDivElement} pageDiv
    * @param {PDFPage} pdfPage
+   * @param {AnnotationStorage} [annotationStorage]
    * @param {string} [imageResourcesPath] - Path for image resources, mainly
    *   for annotation icons. Include trailing slash.
    * @param {boolean} renderInteractiveForms
@@ -127,7 +129,7 @@ class DefaultAnnotationLayerFactory {
   createAnnotationLayerBuilder(
     pageDiv,
     pdfPage,
-    annotationStorage,
+    annotationStorage = null,
     imageResourcesPath = "",
     renderInteractiveForms = false,
     l10n = NullL10n

diff --git a/web/interfaces.js b/web/interfaces.js
@@ -165,6 +165,7 @@ class IPDFAnnotationLayerFactory {
   /**
    * @param {HTMLDivElement} pageDiv
    * @param {PDFPage} pdfPage
+   * @param {AnnotationStorage} [annotationStorage]
    * @param {string} [imageResourcesPath] - Path for image resources, mainly
    *   for annotation icons. Include trailing slash.
    * @param {boolean} renderInteractiveForms
@@ -174,6 +175,7 @@ class IPDFAnnotationLayerFactory {
   createAnnotationLayerBuilder(
     pageDiv,
     pdfPage,
+    annotationStorage = null,
     imageResourcesPath = "",
     renderInteractiveForms = false,
     l10n = undefined