From 346afd1e1ccdc0fe7574a3df38eadd138963a787 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 31 Jul 2020 16:13:26 +0200 Subject: [PATCH] [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). --- src/display/annotation_layer.js | 5 ++++- src/display/annotation_storage.js | 2 +- src/pdf.js | 3 --- test/driver.js | 1 - web/annotation_layer_builder.js | 6 ++++-- web/interfaces.js | 2 ++ 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index d209c595f..ab070866f 100644 --- 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 index 29e7e20ff..08bf2ea61 100644 --- 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 index 3c156119c..c6a6d1338 100644 --- 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 index 6a9e701e0..ddc89ce76 100644 --- 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 index fb532e1d8..929010512 100644 --- 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 index e254df5f5..7db2de726 100644 --- 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