From a8a22283c3f5caa775767d7ebf729500ccdb2b45 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 20 Apr 2020 15:48:34 +0200 Subject: [PATCH 1/2] Change the "download" keyboard shortcut (Ctrl+S) handling, in GENERIC/CHROME builds, to utilize the `EventBus` (issue 11657) This improves the consistency of the "download" handling, in the default viewer, such that the `Toolbar`/`SecondaryToolbar` buttons *and* the keyboard shortcut are now handled in the same way (using the `EventBus`). Given that the "download" keyboard shortcut handling is limited to GENERIC/CHROME builds and that the issue does raise a valid point about only being able to observe *some* downloads, these changes seem acceptable in this particular case. Finally the pre-processor condition is adjusted to *explicitly*, rather than implicitly, list the affected build targets. *Please note:* This patch should NOT be construed as carte blanche to simply convert all of the code in `webViewerKeyDown`, or elsewhere, to make use of the `EventBus` instead of direct function calls. Any further changes, along the lines in this patch, would need to be evaluated on a case-by-case basis to determine if they are actually wanted, given that many/most existing cases in `webViewerKeyDown` should already be *indirectly* observable through the `EventBus` instance. --- web/app.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/app.js b/web/app.js index 78aacbc98..82f283157 100644 --- a/web/app.js +++ b/web/app.js @@ -2571,12 +2571,14 @@ function webViewerKeyDown(evt) { } } - if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC || CHROME")) { + const { eventBus } = PDFViewerApplication; + // CTRL or META without shift if (cmd === 1 || cmd === 8) { switch (evt.keyCode) { case 83: // s - PDFViewerApplication.download(); + eventBus.dispatch("download", { source: window }); handled = true; break; } From 5733d9dd248e73581d3e6595f8b3509df796f5f2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 20 Apr 2020 15:55:02 +0200 Subject: [PATCH 2/2] Add a new "openfile" keyboard shortcut (Ctrl+O), in GENERIC builds Somewhat surprisingly, despite the GENERIC viewer implementing "openfile" support, there's never been a keyboard shortcut available. Similar to the previous patch, this utilizes the `EventBus` for consistency with the `Toolbar`/`SecondaryToolbar` buttons. *Please note:* This patch should NOT be construed as carte blanche to simply convert all of the code in `webViewerKeyDown`, or elsewhere, to make use of the `EventBus` instead of direct function calls. Any further changes, along the lines in this patch, would need to be evaluated on a case-by-case basis to determine if they are actually wanted, given that many/most existing cases in `webViewerKeyDown` should already be *indirectly* observable through the `EventBus` instance. --- web/app.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/app.js b/web/app.js index 82f283157..8261040c0 100644 --- a/web/app.js +++ b/web/app.js @@ -2581,6 +2581,13 @@ function webViewerKeyDown(evt) { eventBus.dispatch("download", { source: window }); handled = true; break; + + case 79: // o + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { + eventBus.dispatch("openfile", { source: window }); + handled = true; + } + break; } } }