From 5fdc908f020c63e129a9a66b76a7a31c69f8321d Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sat, 4 Feb 2017 00:56:15 +0100 Subject: [PATCH 1/5] Recognize file name in reference fragment in getPDFFileNameFromURL The regular expression incorrectly marked a group as capturing. For `http://example.com/#file.pdf`, the expected result is "file.pdf", but instead "document.pdf" was returned. --- web/ui_utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/ui_utils.js b/web/ui_utils.js index fd925d1ba..6e9cc9d30 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -371,8 +371,8 @@ function noContextMenuHandler(e) { * @return {String} Guessed PDF file name. */ function getPDFFileNameFromURL(url) { - var reURI = /^(?:([^:]+:)?\/\/[^\/]+)?([^?#]*)(\?[^#]*)?(#.*)?$/; - // SCHEME HOST 1.PATH 2.QUERY 3.REF + var reURI = /^(?:(?:[^:]+:)?\/\/[^\/]+)?([^?#]*)(\?[^#]*)?(#.*)?$/; + // SCHEME HOST 1.PATH 2.QUERY 3.REF // Pattern to get last matching NAME.pdf var reFilename = /[^\/?#=]+\.pdf\b(?!.*\.pdf\b)/i; var splitURI = reURI.exec(url); From d9f90d595d0eb64875e9d3ed252ae0d0c2aca2bf Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sat, 4 Feb 2017 01:20:17 +0100 Subject: [PATCH 2/5] [CRX] Recognize blob and data-URLs in the router When a blob or data-URL is opened with the extension, viewer.html rewrites the URL. But when the viewer is refreshed (e.g. F5), Chrome would fail to display the viewer because the extension router was not set up to recognize such URLs. Now it is. --- extensions/chromium/extension-router.js | 2 ++ extensions/chromium/manifest.json | 2 ++ 2 files changed, 4 insertions(+) diff --git a/extensions/chromium/extension-router.js b/extensions/chromium/extension-router.js index d9a0c23e1..e4ff6f426 100644 --- a/extensions/chromium/extension-router.js +++ b/extensions/chromium/extension-router.js @@ -27,6 +27,8 @@ limitations under the License. 'ftp', 'file', 'chrome-extension', + 'blob', + 'data', // Chromium OS 'filesystem', // Chromium OS, shorthand for filesystem:/external/ diff --git a/extensions/chromium/manifest.json b/extensions/chromium/manifest.json index 230bb762a..78eff2515 100644 --- a/extensions/chromium/manifest.json +++ b/extensions/chromium/manifest.json @@ -63,6 +63,8 @@ "ftp:/*", "file:/*", "chrome-extension:/*", + "blob:*", + "data:*", "filesystem:/*", "drive:*" ] From 228d253f30917381ecca68cb3f49d3086db94384 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Mon, 6 Feb 2017 00:45:51 +0100 Subject: [PATCH 3/5] Detect download filename based on full URL --- web/app.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/app.js b/web/app.js index f7e76b3e9..e065c8f31 100644 --- a/web/app.js +++ b/web/app.js @@ -737,7 +737,9 @@ var PDFViewerApplication = { } var url = this.baseUrl; - var filename = getPDFFileNameFromURL(url); + // Use this.url instead of this.baseUrl to perform filename detection based + // on the reference fragment as ultimate fallback if needed. + var filename = getPDFFileNameFromURL(this.url); var downloadManager = this.downloadManager; downloadManager.onerror = function (err) { // This error won't really be helpful because it's likely the From c67edabcb3a566a97c9610a93aa04284b10aadc7 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Mon, 6 Feb 2017 00:46:12 +0100 Subject: [PATCH 4/5] Set title using logic similar as download name The download method (and the PDF document properties) detect the file name using `getPDFFileNameFromURL`. The title ought to also display the PDF filename if available. --- web/app.js | 17 ++++++++++------- web/ui_utils.js | 8 ++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/web/app.js b/web/app.js index e065c8f31..8cd082b3f 100644 --- a/web/app.js +++ b/web/app.js @@ -569,14 +569,17 @@ var PDFViewerApplication = { setTitleUsingUrl: function pdfViewSetTitleUsingUrl(url) { this.url = url; this.baseUrl = url.split('#')[0]; - try { - this.setTitle(decodeURIComponent( - pdfjsLib.getFilenameFromUrl(url)) || url); - } catch (e) { - // decodeURIComponent may throw URIError, - // fall back to using the unprocessed url in that case - this.setTitle(url); + var title = getPDFFileNameFromURL(url, ''); + if (!title) { + try { + title = decodeURIComponent(pdfjsLib.getFilenameFromUrl(url)) || url; + } catch (e) { + // decodeURIComponent may throw URIError, + // fall back to using the unprocessed url in that case + title = url; + } } + this.setTitle(title); }, setTitle: function pdfViewSetTitle(title) { diff --git a/web/ui_utils.js b/web/ui_utils.js index 6e9cc9d30..54b4d9226 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -368,9 +368,13 @@ function noContextMenuHandler(e) { /** * Returns the filename or guessed filename from the url (see issue 3455). * url {String} The original PDF location. + * defaultFilename {string} The value to return if the file name is unknown. * @return {String} Guessed PDF file name. */ -function getPDFFileNameFromURL(url) { +function getPDFFileNameFromURL(url, defaultFilename) { + if (typeof defaultFilename === 'undefined') { + defaultFilename = 'document.pdf'; + } var reURI = /^(?:(?:[^:]+:)?\/\/[^\/]+)?([^?#]*)(\?[^#]*)?(#.*)?$/; // SCHEME HOST 1.PATH 2.QUERY 3.REF // Pattern to get last matching NAME.pdf @@ -392,7 +396,7 @@ function getPDFFileNameFromURL(url) { } } } - return suggestedFilename || 'document.pdf'; + return suggestedFilename || defaultFilename; } function normalizeWheelEventDelta(evt) { From f6548e463f1a2af0b5dbae065ee7463f8dc2478f Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Mon, 6 Feb 2017 00:56:13 +0100 Subject: [PATCH 5/5] Open PDF attachments in the viewer instead of download If users want to download, they can quickly click on the Download button in the newly opened viewer. The blobUrl logic for Firefox relies on `disableCreateObjectURL` is never false in Firefox. If the assumption is invalid, then PDF attachments at the attachment view will not correctly be displayed, because a data-URL will be generated and `?` is treated as part of the data:-URL. --- web/pdf_attachment_viewer.js | 43 ++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index dbfda4b3d..4b3e16f28 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -86,6 +86,38 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { this._renderedCapability.resolve(); }, + /** + * @private + */ + _bindPdfLink: + function PDFAttachmentViewer_bindPdfLink(button, content, filename) { + var blobUrl; + button.onclick = function() { + if (!blobUrl) { + blobUrl = pdfjsLib.createObjectURL( + content, 'application/pdf', pdfjsLib.PDFJS.disableCreateObjectURL); + } + var 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. + // eslint-disable-next-line no-undef + viewerUrl = chrome.runtime.getURL('/content/web/viewer.html') + + '?file=' + encodeURIComponent(blobUrl + '#' + filename); + } else { + // Let Firefox's content handler catch the URL and display the PDF. + // In Firefox PDFJS.disableCreateObjectURL is always false, so + // blobUrl is always a blob:-URL and never a data:-URL. + viewerUrl = blobUrl + '?' + encodeURIComponent(filename); + } + window.open(viewerUrl); + return false; + }; + }, + /** * @private */ @@ -124,11 +156,18 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { for (var i = 0; i < attachmentsCount; i++) { var item = attachments[names[i]]; var filename = pdfjsLib.getFilenameFromUrl(item.filename); + filename = pdfjsLib.removeNullCharacters(filename); + var div = document.createElement('div'); div.className = 'attachmentsItem'; var button = document.createElement('button'); - this._bindLink(button, item.content, filename); - button.textContent = pdfjsLib.removeNullCharacters(filename); + button.textContent = filename; + if (/\.pdf$/i.test(filename)) { + this._bindPdfLink(button, item.content, filename); + } else { + this._bindLink(button, item.content, filename); + } + div.appendChild(button); this.container.appendChild(div); }