From 5b28a0bf974218c0fd5c524a5d7bd8a3ce9ce832 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 3 Apr 2021 15:33:14 +0200 Subject: [PATCH 1/2] Re-factor the `download`/`save`-methods, on `PDFViewerApplication`, to make full use of async/await In the next patch we'll need to be able to actually wait for saving to complete, hence it's necessary to slightly re-factor the `save`-method. As part of these changes, we can reduce some duplication in the `save`-method and slightly improve the overall code. For consistency, the `download`-method is updated similarily to improve the code (this functionality is *very* old, even pre-dating the introduction of Promises in the code-base). --- web/app.js | 85 ++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/web/app.js b/web/app.js index 3a66de48a..99a943d8f 100644 --- a/web/app.js +++ b/web/app.js @@ -949,62 +949,59 @@ const PDFViewerApplication = { ); }, - download({ sourceEventType = "download" } = {}) { - function downloadByUrl() { - downloadManager.downloadUrl(url, filename); - } - - const downloadManager = this.downloadManager, - url = this.baseUrl, - filename = this._docFilename; - - // When the PDF document isn't ready, or the PDF file is still downloading, - // simply download using the URL. - if (!this.pdfDocument || !this.downloadComplete) { - downloadByUrl(); + /** + * @private + */ + _ensureDownloadComplete() { + if (this.pdfDocument && this.downloadComplete) { return; } + throw new Error("PDF document not downloaded."); + }, - this.pdfDocument - .getData() - .then(function (data) { - const blob = new Blob([data], { type: "application/pdf" }); - downloadManager.download(blob, url, filename, sourceEventType); - }) - .catch(downloadByUrl); // Error occurred, try downloading with the URL. + async download({ sourceEventType = "download" } = {}) { + const url = this.baseUrl, + filename = this._docFilename; + try { + this._ensureDownloadComplete(); + + const data = await this.pdfDocument.getData(); + const blob = new Blob([data], { type: "application/pdf" }); + + await this.downloadManager.download(blob, url, filename, sourceEventType); + } catch (reason) { + // When the PDF document isn't ready, or the PDF file is still + // downloading, simply download using the URL. + await this.downloadManager.downloadUrl(url, filename); + } }, async save({ sourceEventType = "download" } = {}) { if (this._saveInProgress) { return; } - - const downloadManager = this.downloadManager, - url = this.baseUrl, - filename = this._docFilename; - - // When the PDF document isn't ready, or the PDF file is still downloading, - // simply download using the URL. - if (!this.pdfDocument || !this.downloadComplete) { - this.download({ sourceEventType }); - return; - } this._saveInProgress = true; await this.pdfScriptingManager.dispatchWillSave(); - this.pdfDocument - .saveDocument(this.pdfDocument.annotationStorage) - .then(data => { - const blob = new Blob([data], { type: "application/pdf" }); - downloadManager.download(blob, url, filename, sourceEventType); - }) - .catch(() => { - this.download({ sourceEventType }); - }) - .finally(async () => { - await this.pdfScriptingManager.dispatchDidSave(); - this._saveInProgress = false; - }); + const url = this.baseUrl, + filename = this._docFilename; + try { + this._ensureDownloadComplete(); + + const data = await this.pdfDocument.saveDocument( + this.pdfDocument.annotationStorage + ); + const blob = new Blob([data], { type: "application/pdf" }); + + await this.downloadManager.download(blob, url, filename, sourceEventType); + } catch (reason) { + // When the PDF document isn't ready, or the PDF file is still + // downloading, simply fallback to a "regular" download. + await this.download({ sourceEventType }); + } finally { + await this.pdfScriptingManager.dispatchDidSave(); + this._saveInProgress = false; + } }, downloadOrSave(options) { From 3f59d4201a1b95b83e2fadb8488270b059412d35 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 3 Apr 2021 15:36:34 +0200 Subject: [PATCH 2/2] [GENERIC viewer] Avoid data loss in forms, by triggering saving when the document is closed (issue 12257) As discussed in the issue, this is a small/simple patch that should help to prevent *outright* data loss in forms when a new document is opened in the GENERIC viewer. While the implementation is perhaps a bit "simplistic", it does seem to work and should be fine given that this is an edge-case only relevant for the GENERIC viewer. --- web/app.js | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/web/app.js b/web/app.js index 99a943d8f..a7df2b857 100644 --- a/web/app.js +++ b/web/app.js @@ -802,7 +802,19 @@ const PDFViewerApplication = { } if (!this.pdfLoadingTask) { - return undefined; + return; + } + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) && + this.pdfDocument?.annotationStorage.size > 0 && + this._annotationStorageModified + ) { + try { + // Trigger saving, to prevent data loss in forms; see issue 12257. + await this.save({ sourceEventType: "save" }); + } catch (reason) { + // Ignoring errors, to ensure that document closing won't break. + } } const promises = []; @@ -851,8 +863,6 @@ const PDFViewerApplication = { PDFBug.cleanup(); } await Promise.all(promises); - - return undefined; }, /** @@ -1707,11 +1717,19 @@ const PDFViewerApplication = { } const { annotationStorage } = pdfDocument; - annotationStorage.onSetModified = function () { + annotationStorage.onSetModified = () => { window.addEventListener("beforeunload", beforeUnload); + + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { + this._annotationStorageModified = true; + } }; - annotationStorage.onResetModified = function () { + annotationStorage.onResetModified = () => { window.removeEventListener("beforeunload", beforeUnload); + + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { + delete this._annotationStorageModified; + } }; },