From df931ef685b4d12e761168f8123cb7b15d510291 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 23 Feb 2021 12:58:17 +0100 Subject: [PATCH] Move the opening of PDF file attachments into the `DownloadManager`-implementations Note how the `PDFAttachmentViewer` handles PDF file attachments specially, by opening them in a new window/tab, rather than forcing them to be downloaded. This is done to improve the overall UX, since browsers in general are able to handle PDF files internally. However, for file *annotations* we're currently not attempting to do the same thing and are instead just downloading them directly. In order to unify the behaviour, without having to duplicate a lot of code, the opening of PDF file attachments is thus moved into a new `DownloadManager.openOrDownloadData` method. --- src/display/annotation_layer.js | 10 +++--- web/download_manager.js | 48 +++++++++++++++++++++++++++ web/firefoxcom.js | 38 ++++++++++++++++++++- web/pdf_attachment_viewer.js | 58 ++------------------------------- web/ui_utils.js | 4 +++ 5 files changed, 97 insertions(+), 61 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 1e0eb863a..06ca8e9db 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -1982,11 +1982,11 @@ class FileAttachmentAnnotationElement extends AnnotationElement { * @memberof FileAttachmentAnnotationElement */ _download() { - if (!this.downloadManager) { - warn("Download cannot be started due to unavailable download manager"); - return; - } - this.downloadManager.downloadData(this.content, this.filename, ""); + this.downloadManager?.openOrDownloadData( + this.container, + this.content, + this.filename + ); } } diff --git a/web/download_manager.js b/web/download_manager.js index 0261e4580..399b34663 100644 --- a/web/download_manager.js +++ b/web/download_manager.js @@ -14,6 +14,7 @@ */ import { createObjectURL, createValidAbsoluteUrl } from "pdfjs-lib"; +import { PdfFileRegExp } from "./ui_utils.js"; import { viewerCompatibilityParams } from "./viewer_compatibility.js"; if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("CHROME || GENERIC")) { @@ -43,6 +44,10 @@ function download(blobUrl, filename) { } class DownloadManager { + constructor() { + this._openBlobUrls = new WeakMap(); + } + downloadUrl(url, filename) { if (!createValidAbsoluteUrl(url, "http://example.com")) { return; // restricted/invalid URL @@ -59,6 +64,49 @@ class DownloadManager { download(blobUrl, filename); } + /** + * @returns {boolean} Indicating if the data was opened. + */ + openOrDownloadData(element, data, filename) { + const isPdfFile = PdfFileRegExp.test(filename); + const contentType = isPdfFile ? "application/pdf" : ""; + + if (isPdfFile && !viewerCompatibilityParams.disableCreateObjectURL) { + let blobUrl = this._openBlobUrls.get(element); + if (!blobUrl) { + blobUrl = URL.createObjectURL(new Blob([data], { type: contentType })); + this._openBlobUrls.set(element, blobUrl); + } + let viewerUrl; + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { + // The current URL is the viewer, let's use it and append the file. + viewerUrl = "?file=" + encodeURIComponent(blobUrl + "#" + filename); + } else if (PDFJSDev.test("CHROME")) { + // In the Chrome extension, the URL is rewritten using the history API + // in viewer.js, so an absolute URL must be generated. + viewerUrl = + // eslint-disable-next-line no-undef + chrome.runtime.getURL("/content/web/viewer.html") + + "?file=" + + encodeURIComponent(blobUrl + "#" + filename); + } + + try { + window.open(viewerUrl); + return true; + } catch (ex) { + console.error(`openOrDownloadData: ${ex}`); + // Release the `blobUrl`, since opening it failed, and fallback to + // downloading the PDF file. + URL.revokeObjectURL(blobUrl); + this._openBlobUrls.delete(element); + } + } + + this.downloadData(data, filename, contentType); + return false; + } + /** * @param sourceEventType {string} Used to signal what triggered the download. * The version of PDF.js integrated with Firefox uses this to to determine diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 5ea019c61..7c86f9f73 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -14,10 +14,10 @@ */ import "../extensions/firefox/tools/l10n.js"; +import { DEFAULT_SCALE_VALUE, PdfFileRegExp } from "./ui_utils.js"; import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; import { PDFDataRangeTransport, shadow } from "pdfjs-lib"; import { BasePreferences } from "./preferences.js"; -import { DEFAULT_SCALE_VALUE } from "./ui_utils.js"; if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { throw new Error( @@ -99,6 +99,10 @@ class FirefoxCom { } class DownloadManager { + constructor() { + this._openBlobUrls = new WeakMap(); + } + downloadUrl(url, filename) { FirefoxCom.request("download", { originalUrl: url, @@ -121,6 +125,38 @@ class DownloadManager { }); } + /** + * @returns {boolean} Indicating if the data was opened. + */ + openOrDownloadData(element, data, filename) { + const isPdfFile = PdfFileRegExp.test(filename); + const contentType = isPdfFile ? "application/pdf" : ""; + + if (isPdfFile) { + let blobUrl = this._openBlobUrls.get(element); + if (!blobUrl) { + blobUrl = URL.createObjectURL(new Blob([data], { type: contentType })); + this._openBlobUrls.set(element, blobUrl); + } + // Let Firefox's content handler catch the URL and display the PDF. + const viewerUrl = blobUrl + "#filename=" + encodeURIComponent(filename); + + try { + window.open(viewerUrl); + return true; + } catch (ex) { + console.error(`openOrDownloadData: ${ex}`); + // Release the `blobUrl`, since opening it failed, and fallback to + // downloading the PDF file. + URL.revokeObjectURL(blobUrl); + this._openBlobUrls.delete(element); + } + } + + this.downloadData(data, filename, contentType); + return false; + } + download(blob, url, filename, sourceEventType = "download") { const blobUrl = URL.createObjectURL(blob); diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index 0f15811ba..9826c2442 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -15,9 +15,6 @@ import { createPromiseCapability, getFilenameFromUrl } from "pdfjs-lib"; import { BaseTreeViewer } from "./base_tree_viewer.js"; -import { viewerCompatibilityParams } from "./viewer_compatibility.js"; - -const PdfFileRegExp = /\.pdf$/i; /** * @typedef {Object} PDFAttachmentViewerOptions @@ -91,55 +88,12 @@ class PDFAttachmentViewer extends BaseTreeViewer { }); } - /** - * NOTE: Should only be used when `URL.createObjectURL` is natively supported. - * @private - */ - _bindPdfLink(element, { content, filename }) { - let blobUrl; - element.onclick = () => { - if (!blobUrl) { - blobUrl = URL.createObjectURL( - new Blob([content], { type: "application/pdf" }) - ); - } - let viewerUrl; - if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { - // The current URL is the viewer, let's use it and append the file. - viewerUrl = "?file=" + encodeURIComponent(blobUrl + "#" + filename); - } else if (PDFJSDev.test("MOZCENTRAL")) { - // Let Firefox's content handler catch the URL and display the PDF. - viewerUrl = blobUrl + "#filename=" + encodeURIComponent(filename); - } else if (PDFJSDev.test("CHROME")) { - // In the Chrome extension, the URL is rewritten using the history API - // in viewer.js, so an absolute URL must be generated. - viewerUrl = - // eslint-disable-next-line no-undef - chrome.runtime.getURL("/content/web/viewer.html") + - "?file=" + - encodeURIComponent(blobUrl + "#" + filename); - } - try { - window.open(viewerUrl); - } catch (ex) { - console.error(`_bindPdfLink: ${ex}`); - // Release the `blobUrl`, since opening it failed... - URL.revokeObjectURL(blobUrl); - blobUrl = null; - // ... and fallback to downloading the PDF file. - this.downloadManager.downloadData(content, filename, "application/pdf"); - } - return false; - }; - } - /** * @private */ _bindLink(element, { content, filename }) { element.onclick = () => { - const contentType = PdfFileRegExp.test(filename) ? "application/pdf" : ""; - this.downloadManager.downloadData(content, filename, contentType); + this.downloadManager.openOrDownloadData(element, content, filename); return false; }; } @@ -165,20 +119,14 @@ class PDFAttachmentViewer extends BaseTreeViewer { let attachmentsCount = 0; for (const name of names) { const item = attachments[name]; + const content = item.content; const filename = getFilenameFromUrl(item.filename); const div = document.createElement("div"); div.className = "treeItem"; const element = document.createElement("a"); - if ( - PdfFileRegExp.test(filename) && - !viewerCompatibilityParams.disableCreateObjectURL - ) { - this._bindPdfLink(element, { content: item.content, filename }); - } else { - this._bindLink(element, { content: item.content, filename }); - } + this._bindLink(element, { content, filename }); element.textContent = this._normalizeTextContent(filename); div.appendChild(element); diff --git a/web/ui_utils.js b/web/ui_utils.js index 0c5eda291..1353b0c05 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -69,6 +69,9 @@ const SpreadMode = { // Used by `PDFViewerApplication`, and by the API unit-tests. const AutoPrintRegExp = /\bprint\s*\(/; +// Used by the (various) `DownloadManager`-implementations. +const PdfFileRegExp = /\.pdf$/i; + // Replaces {{arguments}} with their values. function formatL10nValue(text, args) { if (!args) { @@ -1059,6 +1062,7 @@ export { normalizeWheelEventDirection, NullL10n, parseQueryString, + PdfFileRegExp, PresentationModeState, ProgressBar, RendererType,