From 2c557aab2d965ab35e0773c21b1e97a63a215a19 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 21:49:29 +0100 Subject: [PATCH 1/6] Rename document_attachments_view.js to pdf_attachment_view.js --- web/{document_attachments_view.js => pdf_attachment_view.js} | 0 web/viewer.html | 2 +- web/viewer.js | 3 +-- 3 files changed, 2 insertions(+), 3 deletions(-) rename web/{document_attachments_view.js => pdf_attachment_view.js} (100%) diff --git a/web/document_attachments_view.js b/web/pdf_attachment_view.js similarity index 100% rename from web/document_attachments_view.js rename to web/pdf_attachment_view.js diff --git a/web/viewer.html b/web/viewer.html index 1a6a406cb9f697..f3c7ea0d9638e7 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -76,7 +76,7 @@ - + diff --git a/web/viewer.js b/web/viewer.js index ac3a49079f7b89..85340c4d252fa6 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -87,6 +87,7 @@ var mozL10n = document.mozL10n || document.webL10n; //#include pdf_viewer.js //#include pdf_thumbnail_viewer.js //#include pdf_outline_view.js +//#include pdf_attachment_view.js var PDFViewerApplication = { initialBookmark: document.location.hash.substring(1), @@ -1392,8 +1393,6 @@ var PDFViewerApplication = { window.PDFView = PDFViewerApplication; // obsolete name, using it as an alias //#endif -//#include document_attachments_view.js - //#if CHROME //(function rewriteUrlClosure() { // // Run this code outside DOMContentLoaded to make sure that the URL From 74c6160b27103b085d33612885767f2173ae2d9b Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 21:52:14 +0100 Subject: [PATCH 2/6] Rename DocumentAttachmentsView to PDFAttachmentView --- web/pdf_attachment_view.js | 2 +- web/viewer.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web/pdf_attachment_view.js b/web/pdf_attachment_view.js index fd015040cfadad..2d920001c09a32 100644 --- a/web/pdf_attachment_view.js +++ b/web/pdf_attachment_view.js @@ -18,7 +18,7 @@ 'use strict'; -var DocumentAttachmentsView = function documentAttachmentsView(options) { +var PDFAttachmentView = function documentAttachmentsView(options) { var attachments = options.attachments; var attachmentsView = options.attachmentsView; while (attachmentsView.firstChild) { diff --git a/web/viewer.js b/web/viewer.js index 85340c4d252fa6..dfce6862f12b07 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -19,7 +19,7 @@ PDFHistory, Preferences, SidebarView, ViewHistory, Stats, PDFThumbnailViewer, URL, noContextMenuHandler, SecondaryToolbar, PasswordPrompt, PresentationMode, HandTool, Promise, - DocumentProperties, PDFOutlineView, DocumentAttachmentsView, + DocumentProperties, PDFOutlineView, PDFAttachmentView, OverlayManager, PDFFindController, PDFFindBar, getVisibleElements, watchScroll, PDFViewer, PDFRenderingQueue, PresentationModeState, RenderingStates, DEFAULT_SCALE, UNKNOWN_SCALE, @@ -986,7 +986,7 @@ var PDFViewerApplication = { }); pdfDocument.getAttachments().then(function(attachments) { var attachmentsView = document.getElementById('attachmentsView'); - self.attachments = new DocumentAttachmentsView({ + self.attachments = new PDFAttachmentView({ attachments: attachments, attachmentsView: attachmentsView }); From e825d6f6cbde0bf977db077bcac92b2d296f1d4e Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 22:06:19 +0100 Subject: [PATCH 3/6] Refactor PDFAttachmentView to be more class-like and to separate functionality into methods --- web/pdf_attachment_view.js | 77 +++++++++++++++++++++++--------------- web/viewer.js | 9 +++-- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/web/pdf_attachment_view.js b/web/pdf_attachment_view.js index 2d920001c09a32..b69c4bf3ae64df 100644 --- a/web/pdf_attachment_view.js +++ b/web/pdf_attachment_view.js @@ -18,37 +18,54 @@ 'use strict'; -var PDFAttachmentView = function documentAttachmentsView(options) { - var attachments = options.attachments; - var attachmentsView = options.attachmentsView; - while (attachmentsView.firstChild) { - attachmentsView.removeChild(attachmentsView.firstChild); +var PDFAttachmentView = (function PDFAttachmentViewClosure() { + function PDFAttachmentView(options) { + this.container = options.container; + this.attachments = options.attachments; } - if (!attachments) { - return; - } + PDFAttachmentView.prototype = { + reset: function PDFAttachmentView_reset() { + var container = this.container; + while (container.firstChild) { + container.removeChild(container.firstChild); + } + }, - function bindItemLink(domObj, item) { - domObj.onclick = function documentAttachmentsViewOnclick(e) { - var downloadManager = new DownloadManager(); - downloadManager.downloadData(item.content, getFileName(item.filename), - ''); - return false; - }; - } + _bindLink: function PDFAttachmentView_bindLink(button, item) { + button.onclick = function downloadFile(e) { + var downloadManager = new DownloadManager(); + var content = item.content; + var filename = item.filename; + downloadManager.downloadData(content, getFileName(filename), ''); + return false; + }; + }, - var names = Object.keys(attachments).sort(function(a,b) { - return a.toLowerCase().localeCompare(b.toLowerCase()); - }); - for (var i = 0, ii = names.length; i < ii; i++) { - var item = attachments[names[i]]; - var div = document.createElement('div'); - div.className = 'attachmentsItem'; - var button = document.createElement('button'); - bindItemLink(button, item); - button.textContent = getFileName(item.filename); - div.appendChild(button); - attachmentsView.appendChild(div); - } -}; + render: function PDFAttachmentView_render() { + var attachments = this.attachments; + + this.reset(); + + if (!attachments) { + return; + } + + var names = Object.keys(attachments).sort(function(a, b) { + return a.toLowerCase().localeCompare(b.toLowerCase()); + }); + for (var i = 0, len = names.length; i < len; i++) { + var item = attachments[names[i]]; + var div = document.createElement('div'); + div.className = 'attachmentsItem'; + var button = document.createElement('button'); + this._bindLink(button, item); + button.textContent = getFileName(item.filename); + div.appendChild(button); + this.container.appendChild(div); + } + } + }; + + return PDFAttachmentView; +})(); diff --git a/web/viewer.js b/web/viewer.js index dfce6862f12b07..92ad38b1e5f384 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -985,14 +985,15 @@ var PDFViewerApplication = { } }); pdfDocument.getAttachments().then(function(attachments) { - var attachmentsView = document.getElementById('attachmentsView'); + var container = document.getElementById('attachmentsView'); self.attachments = new PDFAttachmentView({ - attachments: attachments, - attachmentsView: attachmentsView + container: container, + attachments: attachments }); + self.attachments.render(); document.getElementById('viewAttachments').disabled = !attachments; - if (!attachments && !attachmentsView.classList.contains('hidden')) { + if (!attachments && !container.classList.contains('hidden')) { self.switchSidebarView('thumbs'); } if (attachments && From 7ba2b7de83f151638ae65c55c6f00d377764ab9e Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 22:11:04 +0100 Subject: [PATCH 4/6] Remove DownloadManager dependency We can pass it in using the options object. Note that that this also avoids creating a DownloadManager object for each separate link (instead, having only one is enough). --- web/pdf_attachment_view.js | 8 ++++---- web/viewer.js | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/web/pdf_attachment_view.js b/web/pdf_attachment_view.js index b69c4bf3ae64df..6e8883f7a51615 100644 --- a/web/pdf_attachment_view.js +++ b/web/pdf_attachment_view.js @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* globals DownloadManager, getFileName */ +/* globals getFileName */ 'use strict'; @@ -22,6 +22,7 @@ var PDFAttachmentView = (function PDFAttachmentViewClosure() { function PDFAttachmentView(options) { this.container = options.container; this.attachments = options.attachments; + this.downloadManager = options.downloadManager; } PDFAttachmentView.prototype = { @@ -34,12 +35,11 @@ var PDFAttachmentView = (function PDFAttachmentViewClosure() { _bindLink: function PDFAttachmentView_bindLink(button, item) { button.onclick = function downloadFile(e) { - var downloadManager = new DownloadManager(); var content = item.content; var filename = item.filename; - downloadManager.downloadData(content, getFileName(filename), ''); + this.downloadManager.downloadData(content, getFileName(filename), ''); return false; - }; + }.bind(this); }, render: function PDFAttachmentView_render() { diff --git a/web/viewer.js b/web/viewer.js index 92ad38b1e5f384..4c84921623d80a 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -988,7 +988,8 @@ var PDFViewerApplication = { var container = document.getElementById('attachmentsView'); self.attachments = new PDFAttachmentView({ container: container, - attachments: attachments + attachments: attachments, + downloadManager: new DownloadManager() }); self.attachments.render(); document.getElementById('viewAttachments').disabled = !attachments; From 9d322a71a750494dc64b5013fb0205a4cc3857d0 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 22:22:42 +0100 Subject: [PATCH 5/6] Add JSDoc comments to PDFAttachmentView --- web/pdf_attachment_view.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/web/pdf_attachment_view.js b/web/pdf_attachment_view.js index 6e8883f7a51615..2ccce3cf4d9516 100644 --- a/web/pdf_attachment_view.js +++ b/web/pdf_attachment_view.js @@ -18,7 +18,21 @@ 'use strict'; +/** + * @typedef {Object} PDFAttachmentViewOptions + * @property {HTMLDivElement} container - The viewer element. + * @property {Array} attachments - An array of attachment objects. + * @property {DownloadManager} downloadManager - The download manager. + */ + +/** + * @class + */ var PDFAttachmentView = (function PDFAttachmentViewClosure() { + /** + * @constructs PDFAttachmentView + * @param {PDFAttachmentViewOptions} options + */ function PDFAttachmentView(options) { this.container = options.container; this.attachments = options.attachments; @@ -33,6 +47,9 @@ var PDFAttachmentView = (function PDFAttachmentViewClosure() { } }, + /** + * @private + */ _bindLink: function PDFAttachmentView_bindLink(button, item) { button.onclick = function downloadFile(e) { var content = item.content; From 21d2c9ea01659719c9d018f9877fa37a2b15bbf3 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 27 Jan 2015 23:16:05 +0100 Subject: [PATCH 6/6] Determine filename only once and reduce code for _bindLink --- web/pdf_attachment_view.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/web/pdf_attachment_view.js b/web/pdf_attachment_view.js index 2ccce3cf4d9516..ad1b7dfe4e6d23 100644 --- a/web/pdf_attachment_view.js +++ b/web/pdf_attachment_view.js @@ -50,11 +50,9 @@ var PDFAttachmentView = (function PDFAttachmentViewClosure() { /** * @private */ - _bindLink: function PDFAttachmentView_bindLink(button, item) { + _bindLink: function PDFAttachmentView_bindLink(button, content, filename) { button.onclick = function downloadFile(e) { - var content = item.content; - var filename = item.filename; - this.downloadManager.downloadData(content, getFileName(filename), ''); + this.downloadManager.downloadData(content, filename, ''); return false; }.bind(this); }, @@ -73,11 +71,12 @@ var PDFAttachmentView = (function PDFAttachmentViewClosure() { }); for (var i = 0, len = names.length; i < len; i++) { var item = attachments[names[i]]; + var filename = getFileName(item.filename); var div = document.createElement('div'); div.className = 'attachmentsItem'; var button = document.createElement('button'); - this._bindLink(button, item); - button.textContent = getFileName(item.filename); + this._bindLink(button, item.content, filename); + button.textContent = filename; div.appendChild(button); this.container.appendChild(div); }