Skip to content

[api-minor] Remove the manual passing of an AnnotationStorage-instance when calling various API-method #13207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 47 additions & 29 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ class PDFDocumentProxy {
* @type {AnnotationStorage} Storage for annotation data in forms.
*/
get annotationStorage() {
return shadow(this, "annotationStorage", new AnnotationStorage());
return this._transport.annotationStorage;
}

/**
Expand Down Expand Up @@ -954,13 +954,26 @@ class PDFDocumentProxy {
}

/**
* @param {AnnotationStorage} annotationStorage - Storage for annotation
* data in forms.
* @returns {Promise<Uint8Array>} A promise that is resolved with a
* {Uint8Array} containing the full data of the saved document.
*/
saveDocument(annotationStorage) {
return this._transport.saveDocument(annotationStorage);
saveDocument() {
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
arguments.length > 0
) {
deprecated("saveDocument no longer accepts any options.");
}
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
this._transport.annotationStorage.size <= 0
) {
deprecated(
"saveDocument called while `annotationStorage` is empty, " +
"please use the getData-method instead."
);
}
return this._transport.saveDocument();
}

/**
Expand Down Expand Up @@ -1068,7 +1081,7 @@ class PDFDocumentProxy {
* some operations. The default value is `false`.
* @property {boolean} [renderInteractiveForms] - Whether or not interactive
* form elements are rendered in the display layer. If so, we do not render
* them on the canvas as well.
* them on the canvas as well. The default value is `false`.
* @property {Array<any>} [transform] - Additional transform, applied just
* before viewport transform.
* @property {Object} [imageLayer] - An object that has `beginLayout`,
Expand All @@ -1080,8 +1093,9 @@ class PDFDocumentProxy {
* <color> value, a `CanvasGradient` object (a linear or radial gradient) or
* a `CanvasPattern` object (a repetitive image). The default value is
* 'rgb(255,255,255)'.
* @property {AnnotationStorage} [annotationStorage] - Storage for annotation
* data in forms.
* @property {boolean} [includeAnnotationStorage] - Render stored interactive
* form element data, from the {@link AnnotationStorage}-instance, onto the
* canvas itself; useful e.g. for printing. The default value is `false`.
* @property {Promise<OptionalContentConfig>} [optionalContentConfigPromise] -
* A promise that should resolve with an {@link OptionalContentConfig}
* created from `PDFDocumentProxy.getOptionalContentConfig`. If `null`,
Expand Down Expand Up @@ -1230,9 +1244,19 @@ class PDFPageProxy {
imageLayer = null,
canvasFactory = null,
background = null,
annotationStorage = null,
includeAnnotationStorage = false,
optionalContentConfigPromise = null,
}) {
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
arguments[0]?.annotationStorage !== undefined
) {
deprecated(
"render no longer accepts an `annotationStorage` option, " +
"please use the `includeAnnotationStorage`-boolean instead."
);
includeAnnotationStorage ||= !!arguments[0].annotationStorage;
}
if (this._stats) {
this._stats.time("Overall");
}
Expand Down Expand Up @@ -1264,6 +1288,9 @@ class PDFPageProxy {
const webGLContext = new WebGLContext({
enable: enableWebGL,
});
const annotationStorage = includeAnnotationStorage
? this._transport.annotationStorage.serializable
: null;

// If there's no displayReadyCapability yet, then the operatorList
// was never requested before. Make the request and create the promise.
Expand All @@ -1282,7 +1309,7 @@ class PDFPageProxy {
pageIndex: this._pageIndex,
intent: renderingIntent,
renderInteractiveForms: renderInteractiveForms === true,
annotationStorage: annotationStorage?.serializable || null,
annotationStorage,
});
}

Expand Down Expand Up @@ -2192,8 +2219,8 @@ class WorkerTransport {
this.setupMessageHandler();
}

get loadingTaskSettled() {
return this.loadingTask._capability.settled;
get annotationStorage() {
return shadow(this, "annotationStorage", new AnnotationStorage());
}

destroy() {
Expand All @@ -2220,21 +2247,14 @@ class WorkerTransport {
});
this.pageCache.length = 0;
this.pagePromises.length = 0;
// Allow `AnnotationStorage`-related clean-up when destroying the document.
if (this.hasOwnProperty("annotationStorage")) {
this.annotationStorage.resetModified();
}
// We also need to wait for the worker to finish its long running tasks.
const terminated = this.messageHandler.sendWithPromise("Terminate", null);
waitOn.push(terminated);
// Allow `AnnotationStorage`-related clean-up when destroying the document.
if (this.loadingTaskSettled) {
const annotationStorageResetModified = this.loadingTask.promise
.then(pdfDocument => {
// Avoid initializing the `annotationStorage` if it doesn't exist.
if (pdfDocument.hasOwnProperty("annotationStorage")) {
pdfDocument.annotationStorage.resetModified();
}
})
.catch(() => {});
waitOn.push(annotationStorageResetModified);
}

Promise.all(waitOn).then(() => {
this.commonObjs.clear();
this.fontLoader.clear();
Expand Down Expand Up @@ -2669,17 +2689,15 @@ class WorkerTransport {
});
}

saveDocument(annotationStorage) {
saveDocument() {
return this.messageHandler
.sendWithPromise("SaveDocument", {
numPages: this._numPages,
annotationStorage: annotationStorage?.serializable || null,
annotationStorage: this.annotationStorage.serializable,
filename: this._fullReader?.filename ?? null,
})
.finally(() => {
if (annotationStorage) {
annotationStorage.resetModified();
}
this.annotationStorage.resetModified();
});
}

Expand Down
9 changes: 4 additions & 5 deletions test/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -635,14 +635,13 @@ var Driver = (function DriverClosure() {
optionalContentConfigPromise: task.optionalContentConfigPromise,
};
if (renderPrint) {
const annotationStorage = task.annotationStorage;
if (annotationStorage) {
const docAnnotationStorage = task.pdfDoc.annotationStorage;
const entries = Object.entries(annotationStorage);
if (task.annotationStorage) {
const entries = Object.entries(task.annotationStorage),
docAnnotationStorage = task.pdfDoc.annotationStorage;
for (const [key, value] of entries) {
docAnnotationStorage.setValue(key, value);
}
renderContext.annotationStorage = docAnnotationStorage;
renderContext.includeAnnotationStorage = true;
}
renderContext.intent = "print";
}
Expand Down
4 changes: 1 addition & 3 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -998,9 +998,7 @@ const PDFViewerApplication = {
try {
this._ensureDownloadComplete();

const data = await this.pdfDocument.saveDocument(
this.pdfDocument.annotationStorage
);
const data = await this.pdfDocument.saveDocument();
const blob = new Blob([data], { type: "application/pdf" });

await this.downloadManager.download(blob, url, filename, sourceEventType);
Expand Down
2 changes: 1 addition & 1 deletion web/firefox_print_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function composePage(
transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0],
viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation }),
intent: "print",
annotationStorage: pdfDocument.annotationStorage,
includeAnnotationStorage: true,
optionalContentConfigPromise,
};
currentRenderTask = thisRenderTask = pdfPage.render(renderContext);
Expand Down
2 changes: 1 addition & 1 deletion web/pdf_print_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function renderPage(
transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0],
viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation }),
intent: "print",
annotationStorage: pdfDocument.annotationStorage,
includeAnnotationStorage: true,
optionalContentConfigPromise,
};
return pdfPage.render(renderContext).promise;
Expand Down