From 958ea2be8be36b08eb4672031335235d1512645d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 19 Dec 2020 12:38:10 +0100 Subject: [PATCH 1/3] Move the functionality of the `webViewerDownloadOrSave` function into a new `PDFViewerApplication` method instead Given that this relies on accessing properties on the `PDFDocumentProxy`-instance, it seems more appropriate for this code to live in `PDFViewerApplication`. --- web/app.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/web/app.js b/web/app.js index df37f0c669778..7eb431c7d6f90 100644 --- a/web/app.js +++ b/web/app.js @@ -1060,6 +1060,14 @@ const PDFViewerApplication = { }); }, + downloadOrSave(options) { + if (this.pdfDocument?.annotationStorage.size > 0) { + this.save(options); + } else { + this.download(options); + } + }, + /** * For PDF documents that contain e.g. forms and javaScript, we should only * trigger the fallback bar once the user has interacted with the page. @@ -2712,21 +2720,11 @@ function webViewerPresentationMode() { function webViewerPrint() { PDFViewerApplication.triggerPrinting(); } -function webViewerDownloadOrSave(sourceEventType) { - if ( - PDFViewerApplication.pdfDocument && - PDFViewerApplication.pdfDocument.annotationStorage.size > 0 - ) { - PDFViewerApplication.save({ sourceEventType }); - } else { - PDFViewerApplication.download({ sourceEventType }); - } -} function webViewerDownload() { - webViewerDownloadOrSave("download"); + PDFViewerApplication.downloadOrSave({ sourceEventType: "download" }); } function webViewerSave() { - webViewerDownloadOrSave("save"); + PDFViewerApplication.downloadOrSave({ sourceEventType: "save" }); } function webViewerFirstPage() { if (PDFViewerApplication.pdfDocument) { From 517af6b6ab164d92f188524e80c56e3e129e3f8f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 19 Dec 2020 12:49:12 +0100 Subject: [PATCH 2/3] Delay initialization of the `AnnotationStorage` callbacks slightly in the default viewer These callbacks should not be necessary *before* the document has been initialized. Furthermore, move the functionality to a new helper-method since `PDFViewerApplication.load` is already quite large. --- web/app.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/web/app.js b/web/app.js index 7eb431c7d6f90..80a67da559787 100644 --- a/web/app.js +++ b/web/app.js @@ -1287,14 +1287,6 @@ const PDFViewerApplication = { this.pdfLinkService.setDocument(pdfDocument, baseDocumentUrl); this.pdfDocumentProperties.setDocument(pdfDocument, this.url); - const annotationStorage = pdfDocument.annotationStorage; - annotationStorage.onSetModified = function () { - window.addEventListener("beforeunload", beforeUnload); - }; - annotationStorage.onResetModified = function () { - window.removeEventListener("beforeunload", beforeUnload); - }; - const pdfViewer = this.pdfViewer; pdfViewer.setDocument(pdfDocument); const { firstPagePromise, onePageRendered, pagesPromise } = pdfViewer; @@ -1322,6 +1314,7 @@ const PDFViewerApplication = { firstPagePromise.then(pdfPage => { this.loadingBar.setWidth(this.appConfig.viewerContainer); + this._initializeAnnotationStorageCallbacks(pdfDocument); Promise.all([ animationStarted, @@ -1878,6 +1871,23 @@ const PDFViewerApplication = { } }, + /** + * @private + */ + _initializeAnnotationStorageCallbacks(pdfDocument) { + if (pdfDocument !== this.pdfDocument) { + return; + } + const { annotationStorage } = pdfDocument; + + annotationStorage.onSetModified = function () { + window.addEventListener("beforeunload", beforeUnload); + }; + annotationStorage.onResetModified = function () { + window.removeEventListener("beforeunload", beforeUnload); + }; + }, + setInitialView( storedHash, { rotation, sidebarView, scrollMode, spreadMode } = {} From f9530e56dabf63c5daa4c8ef5f787271bb853580 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 19 Dec 2020 13:21:40 +0100 Subject: [PATCH 3/3] Run `AnnotationStorage.resetModified` when destroying the `PDFDocumentLoadingTask`/`PDFDocumentProxy` This will, in a very simple way using the existing events, thus allow the viewer to remove the "beforeunload" `window` event listener when the document is closed. Generally speaking we want to avoid having *global* event listeners for the PDF document instance, which is why the `EventBus` exists, and instead reserve global events for the viewer itself. However, the `AnnotationStorage` "beforeunload" event unfortunately needs to be document-specific and we should thus ensure that it's correctly removed when the document is destroyed. --- src/display/api.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/display/api.js b/src/display/api.js index ba5dd87cf8ee0..48de0601c4f48 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2127,6 +2127,10 @@ class WorkerTransport { this.setupMessageHandler(); } + get loadingTaskSettled() { + return this.loadingTask._capability.settled; + } + destroy() { if (this.destroyCapability) { return this.destroyCapability.promise; @@ -2154,6 +2158,18 @@ class WorkerTransport { // 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();