From 6fd899dc443425747098935207096328e7b55eb2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 24 Feb 2021 13:02:58 +0100 Subject: [PATCH] [api-minor] Support the Content-Disposition filename in the Firefox PDF Viewer (bug 1694556, PR 9379 follow-up) As can be seen [in the mozilla-central code](https://searchfox.org/mozilla-central/rev/a6db3bd67367aa9ddd9505690cab09b47e65a762/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#1222-1225), we're already getting the Content-Disposition filename. However, that data isn't passed through to the viewer nor to the `PDFDataTransportStream`-implementation, which explains why it's currently being ignored. *Please note:* This will also require a small mozilla-central patch, see https://bugzilla.mozilla.org/show_bug.cgi?id=1694556, to forward the necessary data to the viewer. --- src/display/api.js | 12 +++++++++++- src/display/display_utils.js | 5 +++++ src/display/network_utils.js | 9 +++------ src/display/transport_stream.js | 17 ++++++++++++++--- src/pdf.js | 2 ++ web/app.js | 8 ++++++-- web/download_manager.js | 9 ++++----- web/firefoxcom.js | 15 ++++++++------- web/pdf_attachment_viewer.js | 4 ++-- web/ui_utils.js | 4 ---- 10 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 5bdcc7c87..2aa097521 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -334,6 +334,7 @@ function getDocument(src) { length: params.length, initialData: params.initialData, progressiveDone: params.progressiveDone, + contentDispositionFilename: params.contentDispositionFilename, disableRange: params.disableRange, disableStream: params.disableStream, }, @@ -401,6 +402,8 @@ function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { source.length = pdfDataRangeTransport.length; source.initialData = pdfDataRangeTransport.initialData; source.progressiveDone = pdfDataRangeTransport.progressiveDone; + source.contentDispositionFilename = + pdfDataRangeTransport.contentDispositionFilename; } return worker.messageHandler .sendWithPromise("GetDocRequest", { @@ -554,11 +557,18 @@ class PDFDataRangeTransport { * @param {number} length * @param {Uint8Array} initialData * @param {boolean} [progressiveDone] + * @param {string} [contentDispositionFilename] */ - constructor(length, initialData, progressiveDone = false) { + constructor( + length, + initialData, + progressiveDone = false, + contentDispositionFilename = null + ) { this.length = length; this.initialData = initialData; this.progressiveDone = progressiveDone; + this.contentDispositionFilename = contentDispositionFilename; this._rangeListeners = []; this._progressListeners = []; diff --git a/src/display/display_utils.js b/src/display/display_utils.js index 58ff3de3f..c118b04a4 100644 --- a/src/display/display_utils.js +++ b/src/display/display_utils.js @@ -451,6 +451,10 @@ function addLinkAttributes(link, { url, target, rel, enabled = true } = {}) { link.rel = typeof rel === "string" ? rel : DEFAULT_LINK_REL; } +function isPdfFile(filename) { + return typeof filename === "string" && /\.pdf$/i.test(filename); +} + /** * Gets the file name from a given URL. * @param {string} url @@ -652,6 +656,7 @@ export { DOMSVGFactory, getFilenameFromUrl, isFetchSupported, + isPdfFile, isValidFetchUrl, LinkTarget, loadScript, diff --git a/src/display/network_utils.js b/src/display/network_utils.js index 919e58afa..f52f32c3c 100644 --- a/src/display/network_utils.js +++ b/src/display/network_utils.js @@ -19,6 +19,7 @@ import { UnexpectedResponseException, } from "../shared/util.js"; import { getFilenameFromContentDispositionHeader } from "./content_disposition.js"; +import { isPdfFile } from "./display_utils.js"; function validateRangeRequestCapabilities({ getResponseHeader, @@ -70,7 +71,7 @@ function extractFilenameFromHeader(getResponseHeader) { filename = decodeURIComponent(filename); } catch (ex) {} } - if (/\.pdf$/i.test(filename)) { + if (isPdfFile(filename)) { return filename; } } @@ -82,11 +83,7 @@ function createResponseStatusError(status, url) { return new MissingPDFException('Missing PDF "' + url + '".'); } return new UnexpectedResponseException( - "Unexpected server response (" + - status + - ') while retrieving PDF "' + - url + - '".', + `Unexpected server response (${status}) while retrieving PDF "${url}".`, status ); } diff --git a/src/display/transport_stream.js b/src/display/transport_stream.js index e4cb4a878..d0d6044df 100644 --- a/src/display/transport_stream.js +++ b/src/display/transport_stream.js @@ -14,6 +14,7 @@ */ import { assert, createPromiseCapability } from "../shared/util.js"; +import { isPdfFile } from "./display_utils.js"; /** @implements {IPDFStream} */ class PDFDataTransportStream { @@ -25,6 +26,8 @@ class PDFDataTransportStream { this._queuedChunks = []; this._progressiveDone = params.progressiveDone || false; + this._contentDispositionFilename = + params.contentDispositionFilename || null; const initialData = params.initialData; if (initialData?.length > 0) { @@ -125,7 +128,8 @@ class PDFDataTransportStream { return new PDFDataTransportStreamReader( this, queuedChunks, - this._progressiveDone + this._progressiveDone, + this._contentDispositionFilename ); } @@ -153,10 +157,17 @@ class PDFDataTransportStream { /** @implements {IPDFStreamReader} */ class PDFDataTransportStreamReader { - constructor(stream, queuedChunks, progressiveDone = false) { + constructor( + stream, + queuedChunks, + progressiveDone = false, + contentDispositionFilename = null + ) { this._stream = stream; this._done = progressiveDone || false; - this._filename = null; + this._filename = isPdfFile(contentDispositionFilename) + ? contentDispositionFilename + : null; this._queuedChunks = queuedChunks || []; this._loaded = 0; for (const chunk of this._queuedChunks) { diff --git a/src/pdf.js b/src/pdf.js index 51bf5d87d..af6bfc96d 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -18,6 +18,7 @@ import { addLinkAttributes, getFilenameFromUrl, isFetchSupported, + isPdfFile, isValidFetchUrl, LinkTarget, loadScript, @@ -128,6 +129,7 @@ export { // From "./display/display_utils.js": addLinkAttributes, getFilenameFromUrl, + isPdfFile, LinkTarget, loadScript, PDFDateString, diff --git a/web/app.js b/web/app.js index e849e113d..b425acb90 100644 --- a/web/app.js +++ b/web/app.js @@ -44,6 +44,7 @@ import { getFilenameFromUrl, GlobalWorkerOptions, InvalidPDFException, + isPdfFile, LinkTarget, loadScript, MissingPDFException, @@ -727,7 +728,10 @@ const PDFViewerApplication = { onOpenWithTransport: (url, length, transport) => { this.open(url, { length, range: transport }); }, - onOpenWithData: data => { + onOpenWithData: (data, contentDispositionFilename) => { + if (isPdfFile(contentDispositionFilename)) { + this._contentDispositionFilename = contentDispositionFilename; + } this.open(data); }, onOpenWithURL: (url, length, originalUrl) => { @@ -1744,7 +1748,7 @@ const PDFViewerApplication = { } this.documentInfo = info; this.metadata = metadata; - this._contentDispositionFilename = contentDispositionFilename; + this._contentDispositionFilename ??= contentDispositionFilename; this._contentLength ??= contentLength; // See `getDownloadInfo`-call above. // Provides some basic debug information diff --git a/web/download_manager.js b/web/download_manager.js index 399b34663..9d4b8cfd7 100644 --- a/web/download_manager.js +++ b/web/download_manager.js @@ -13,8 +13,7 @@ * limitations under the License. */ -import { createObjectURL, createValidAbsoluteUrl } from "pdfjs-lib"; -import { PdfFileRegExp } from "./ui_utils.js"; +import { createObjectURL, createValidAbsoluteUrl, isPdfFile } from "pdfjs-lib"; import { viewerCompatibilityParams } from "./viewer_compatibility.js"; if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("CHROME || GENERIC")) { @@ -68,10 +67,10 @@ class DownloadManager { * @returns {boolean} Indicating if the data was opened. */ openOrDownloadData(element, data, filename) { - const isPdfFile = PdfFileRegExp.test(filename); - const contentType = isPdfFile ? "application/pdf" : ""; + const isPdfData = isPdfFile(filename); + const contentType = isPdfData ? "application/pdf" : ""; - if (isPdfFile && !viewerCompatibilityParams.disableCreateObjectURL) { + if (isPdfData && !viewerCompatibilityParams.disableCreateObjectURL) { let blobUrl = this._openBlobUrls.get(element); if (!blobUrl) { blobUrl = URL.createObjectURL(new Blob([data], { type: contentType })); diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 7c86f9f73..9d9a5d78f 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 { isPdfFile, 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( @@ -129,10 +129,10 @@ class DownloadManager { * @returns {boolean} Indicating if the data was opened. */ openOrDownloadData(element, data, filename) { - const isPdfFile = PdfFileRegExp.test(filename); - const contentType = isPdfFile ? "application/pdf" : ""; + const isPdfData = isPdfFile(filename); + const contentType = isPdfData ? "application/pdf" : ""; - if (isPdfFile) { + if (isPdfData) { let blobUrl = this._openBlobUrls.get(element); if (!blobUrl) { blobUrl = URL.createObjectURL(new Blob([data], { type: contentType })); @@ -332,7 +332,8 @@ class FirefoxExternalServices extends DefaultExternalServices { pdfDataRangeTransport = new FirefoxComDataRangeTransport( args.length, args.data, - args.done + args.done, + args.filename ); callbacks.onOpenWithTransport( @@ -367,7 +368,7 @@ class FirefoxExternalServices extends DefaultExternalServices { callbacks.onError(args.errorCode); break; } - callbacks.onOpenWithData(args.data); + callbacks.onOpenWithData(args.data, args.filename); break; } }); diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index 9826c2442..a0c588ffc 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -119,8 +119,8 @@ 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 content = item.content, + filename = getFilenameFromUrl(item.filename); const div = document.createElement("div"); div.className = "treeItem"; diff --git a/web/ui_utils.js b/web/ui_utils.js index 1353b0c05..0c5eda291 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -69,9 +69,6 @@ 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) { @@ -1062,7 +1059,6 @@ export { normalizeWheelEventDirection, NullL10n, parseQueryString, - PdfFileRegExp, PresentationModeState, ProgressBar, RendererType,