[api-minor] Fix the AnnotationStorage usage properly in the viewer/…

…tests (PR 12107 and 12143 follow-up) *The [api-minor] label probably ought to have been added to the original PR, given the changes to the `createAnnotationLayerBuilder` signature (if nothing else).* This patch fixes the following things: - Let the `AnnotationLayer.render` method create an `AnnotationStorage`-instance if none was provided, thus making the parameter *properly* optional. This not only fixes the reference tests, it also prevents issues when the viewer components are used. - Stop exporting `AnnotationStorage` in the official API, i.e. the `src/pdf.js` file, since it's no longer necessary given the change above. Generally speaking, unless absolutely necessary we probably shouldn't export unused things in the API. - Fix a number of JSDocs `typedef`s, in `src/display/` and `web/` code, to actually account for the new `annotationStorage` parameter. - Update `web/interfaces.js` to account for the changes in `createAnnotationLayerBuilder`. - Initialize the storage, in `AnnotationStorage`, using `Object.create(null)` rather than `{}` (which is the PDF.js default).
mozilla · Jul 31, 2020 · 346afd1 · 346afd1
1 parent c5663f2
commit 346afd1
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 8 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,
@@ -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