Skip to content

Commit 346afd1

Browse files
committed
[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).
1 parent c5663f2 commit 346afd1

File tree

6 files changed

+11
-8
lines changed

6 files changed

+11
-8
lines changed

src/display/annotation_layer.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
Util,
3030
warn,
3131
} from "../shared/util.js";
32+
import { AnnotationStorage } from "./annotation_storage.js";
3233

3334
/**
3435
* @typedef {Object} AnnotationElementParameters
@@ -38,6 +39,7 @@ import {
3839
* @property {PageViewport} viewport
3940
* @property {IPDFLinkService} linkService
4041
* @property {DownloadManager} downloadManager
42+
* @property {AnnotationStorage} [annotationStorage]
4143
* @property {string} [imageResourcesPath] - Path for image resources, mainly
4244
* for annotation icons. Include trailing slash.
4345
* @property {boolean} renderInteractiveForms
@@ -1463,7 +1465,8 @@ class AnnotationLayer {
14631465
imageResourcesPath: parameters.imageResourcesPath || "",
14641466
renderInteractiveForms: parameters.renderInteractiveForms || false,
14651467
svgFactory: new DOMSVGFactory(),
1466-
annotationStorage: parameters.annotationStorage,
1468+
annotationStorage:
1469+
parameters.annotationStorage || new AnnotationStorage(),
14671470
});
14681471
if (element.isRenderable) {
14691472
parameters.div.appendChild(element.render());

src/display/annotation_storage.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
class AnnotationStorage {
1717
constructor() {
18-
this._storage = {};
18+
this._storage = Object.create(null);
1919
}
2020

2121
/**

src/pdf.js

-3
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import {
5050
VerbosityLevel,
5151
} from "./shared/util.js";
5252
import { AnnotationLayer } from "./display/annotation_layer.js";
53-
import { AnnotationStorage } from "./display/annotation_storage.js";
5453
import { apiCompatibilityParams } from "./display/api_compatibility.js";
5554
import { GlobalWorkerOptions } from "./display/worker_options.js";
5655
import { renderTextLayer } from "./display/text_layer.js";
@@ -157,8 +156,6 @@ export {
157156
VerbosityLevel,
158157
// From "./display/annotation_layer.js":
159158
AnnotationLayer,
160-
// From "./display/annotation_storage.js":
161-
AnnotationStorage,
162159
// From "./display/api_compatibility.js":
163160
apiCompatibilityParams,
164161
// From "./display/worker_options.js":

test/driver.js

-1
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ var rasterizeAnnotationLayer = (function rasterizeAnnotationLayerClosure() {
220220
linkService: new pdfjsViewer.SimpleLinkService(),
221221
imageResourcesPath,
222222
renderInteractiveForms,
223-
annotationStorage: new pdfjsLib.AnnotationStorage(),
224223
};
225224
pdfjsLib.AnnotationLayer.render(parameters);
226225

web/annotation_layer_builder.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { SimpleLinkService } from "./pdf_link_service.js";
2121
* @typedef {Object} AnnotationLayerBuilderOptions
2222
* @property {HTMLDivElement} pageDiv
2323
* @property {PDFPage} pdfPage
24+
* @property {AnnotationStorage} [annotationStorage]
2425
* @property {string} [imageResourcesPath] - Path for image resources, mainly
2526
* for annotation icons. Include trailing slash.
2627
* @property {boolean} renderInteractiveForms
@@ -38,7 +39,7 @@ class AnnotationLayerBuilder {
3839
pdfPage,
3940
linkService,
4041
downloadManager,
41-
annotationStorage,
42+
annotationStorage = null,
4243
imageResourcesPath = "",
4344
renderInteractiveForms = false,
4445
l10n = NullL10n,
@@ -118,6 +119,7 @@ class DefaultAnnotationLayerFactory {
118119
/**
119120
* @param {HTMLDivElement} pageDiv
120121
* @param {PDFPage} pdfPage
122+
* @param {AnnotationStorage} [annotationStorage]
121123
* @param {string} [imageResourcesPath] - Path for image resources, mainly
122124
* for annotation icons. Include trailing slash.
123125
* @param {boolean} renderInteractiveForms
@@ -127,7 +129,7 @@ class DefaultAnnotationLayerFactory {
127129
createAnnotationLayerBuilder(
128130
pageDiv,
129131
pdfPage,
130-
annotationStorage,
132+
annotationStorage = null,
131133
imageResourcesPath = "",
132134
renderInteractiveForms = false,
133135
l10n = NullL10n

web/interfaces.js

+2
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ class IPDFAnnotationLayerFactory {
165165
/**
166166
* @param {HTMLDivElement} pageDiv
167167
* @param {PDFPage} pdfPage
168+
* @param {AnnotationStorage} [annotationStorage]
168169
* @param {string} [imageResourcesPath] - Path for image resources, mainly
169170
* for annotation icons. Include trailing slash.
170171
* @param {boolean} renderInteractiveForms
@@ -174,6 +175,7 @@ class IPDFAnnotationLayerFactory {
174175
createAnnotationLayerBuilder(
175176
pageDiv,
176177
pdfPage,
178+
annotationStorage = null,
177179
imageResourcesPath = "",
178180
renderInteractiveForms = false,
179181
l10n = undefined

0 commit comments

Comments
 (0)