From 0daf51c340718429774ef93a2be47dcf5242c297 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Dec 2020 11:13:43 +0100 Subject: [PATCH 1/2] [Scripting] Try to ensure that the `WillPrint`/`DidPrint` respectively `DidSave` events are always dispatched Note that currently the `DidSave` event is not *guaranteed* to actually be dispatched if there's any errors during saving, which is easily fixed by simply moving it to occur in the `finally`-handler in `PDFViewerApplication.save` method. For the `WillPrint`/`DidPrint` events, things are unfortunately more complicated. Currently these events will *only* be dispatched iff the printing request comes from within the viewer itself (e.g. by the user clicking on the "Print" toolbar button), however printing can be triggered in a few additional ways: - In the GENERIC viewer: - By the Ctrl+P keyboard shortcut. - In the MOZCENTRAL viewer, i.e. the Firefox built-in viewer: - By the Ctrl+P keyboard shortcut. - By the "Print" item, as found in either the Firefox "Hamburger menu" or in the browser-window menu. In either of the cases described above, no `WillPrint`/`DidPrint` events will be dispatched. In order to *guarantee* that things work in the general case, we thus have to move the `dispatchEventInSandbox` calls to the "beforeprint"/"afterprint" event handlers instead. --- web/app.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/web/app.js b/web/app.js index c2ddd2b48d604..6040dc854f178 100644 --- a/web/app.js +++ b/web/app.js @@ -1047,16 +1047,16 @@ const PDFViewerApplication = { .then(data => { const blob = new Blob([data], { type: "application/pdf" }); downloadManager.download(blob, url, filename, sourceEventType); - - this._scriptingInstance?.scripting.dispatchEventInSandbox({ - id: "doc", - name: "DidSave", - }); }) .catch(() => { this.download({ sourceEventType }); }) .finally(() => { + this._scriptingInstance?.scripting.dispatchEventInSandbox({ + id: "doc", + name: "DidSave", + }); + this._saveInProgress = false; }); }, @@ -1967,6 +1967,11 @@ const PDFViewerApplication = { }, beforePrint() { + this._scriptingInstance?.scripting.dispatchEventInSandbox({ + id: "doc", + name: "WillPrint", + }); + if (this.printService) { // There is no way to suppress beforePrint/afterPrint events, // but PDFPrintService may generate double events -- this will ignore @@ -2028,6 +2033,11 @@ const PDFViewerApplication = { }, afterPrint() { + this._scriptingInstance?.scripting.dispatchEventInSandbox({ + id: "doc", + name: "DidPrint", + }); + if (this.printService) { this.printService.destroy(); this.printService = null; @@ -2060,17 +2070,7 @@ const PDFViewerApplication = { if (!this.supportsPrinting) { return; } - this._scriptingInstance?.scripting.dispatchEventInSandbox({ - id: "doc", - name: "WillPrint", - }); - window.print(); - - this._scriptingInstance?.scripting.dispatchEventInSandbox({ - id: "doc", - name: "DidPrint", - }); }, bindEvents() { From a4786c96898de09cb6174f4abe6b7d19009d1cb6 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Dec 2020 11:39:54 +0100 Subject: [PATCH 2/2] [Scripting] Await manually triggered `dispatchEventInSandbox` calls in the viewer Given that the `dispatchEventInSandbox` method (on the scripting-classes) is asynchronous, there's a very real risk that the events won't be dispatched/handled until *after* their associated functionality has actually run (with the "Will..." events being particularily susceptible to this issue). To reduce the likelihood of that happening, we can simply `await` the `dispatchEventInSandbox` calls as necessary. A couple of methods are now marked as `async` to support these changes, however that shouldn't be a problem as far as I can tell. *Please note:* Given that the browser "beforeprint"/"afterprint" events are *synchronous*, we unfortunately cannot await the `WillPrint`/`DidPrint` event dispatching. To fix this properly the web-platform would need support for asynchronous printing, and we'll thus have to hope that things work correctly anyway. --- web/app.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/web/app.js b/web/app.js index 6040dc854f178..21266514bd00a 100644 --- a/web/app.js +++ b/web/app.js @@ -1013,7 +1013,7 @@ const PDFViewerApplication = { .catch(downloadByUrl); // Error occurred, try downloading with the URL. }, - save({ sourceEventType = "download" } = {}) { + async save({ sourceEventType = "download" } = {}) { if (this._saveInProgress) { return; } @@ -1036,12 +1036,13 @@ const PDFViewerApplication = { this.download({ sourceEventType }); return; } - this._scriptingInstance?.scripting.dispatchEventInSandbox({ + this._saveInProgress = true; + + await this._scriptingInstance?.scripting.dispatchEventInSandbox({ id: "doc", name: "WillSave", }); - this._saveInProgress = true; this.pdfDocument .saveDocument(this.pdfDocument.annotationStorage) .then(data => { @@ -1051,8 +1052,8 @@ const PDFViewerApplication = { .catch(() => { this.download({ sourceEventType }); }) - .finally(() => { - this._scriptingInstance?.scripting.dispatchEventInSandbox({ + .finally(async () => { + await this._scriptingInstance?.scripting.dispatchEventInSandbox({ id: "doc", name: "DidSave", }); @@ -1614,7 +1615,7 @@ const PDFViewerApplication = { return; } - scripting.dispatchEventInSandbox({ + await scripting.dispatchEventInSandbox({ id: "doc", name: "Open", }); @@ -1967,6 +1968,8 @@ const PDFViewerApplication = { }, beforePrint() { + // Given that the "beforeprint" browser event is synchronous, we + // unfortunately cannot await the scripting event dispatching here. this._scriptingInstance?.scripting.dispatchEventInSandbox({ id: "doc", name: "WillPrint", @@ -2033,6 +2036,8 @@ const PDFViewerApplication = { }, afterPrint() { + // Given that the "afterprint" browser event is synchronous, we + // unfortunately cannot await the scripting event dispatching here. this._scriptingInstance?.scripting.dispatchEventInSandbox({ id: "doc", name: "DidPrint",