From c03ed76bb1fc0dcdbb79a2cac8bf762a1df5ba55 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 15 Jan 2017 12:37:06 +0100 Subject: [PATCH 1/2] Display a notification on the `sidebarToggle` button for PDF documents with outline/attachments A longstanding issue with the viewer is that you cannot tell if a PDF document includes an outline and/or attachments without actually opening the sidebar. This patch contains a suggested solution for that, by displaying an hide-on-interaction notification on the `sidebarToggle` button (and the relevant sidebar view buttons). Note that this was inspired by e.g. the update notification that is displayed on the menu button in Firefox. For an initial implementation, I've tried to do this in such a way that the notification isn't too distracting. Without being an UX expert, I don't think that we'd want something too in-your-face, in order to keep the viewer toolbars reasonable clean. (We probably do *not* want e.g. an entire notification bar in these situations, since that would take up unnecessary screen space and require actions from the user to close.) However it's certainly possible that the current notification might simply be *too* inconspicuous to be truly helpful to users, but we could probably iterate on that if the feature itself is deemed useful. --- l10n/en-US/viewer.properties | 1 + l10n/sv-SE/viewer.properties | 1 + web/pdf_sidebar.js | 110 +++++++++++++++++++++++++++++++---- web/viewer.css | 18 ++++++ 4 files changed, 120 insertions(+), 10 deletions(-) diff --git a/l10n/en-US/viewer.properties b/l10n/en-US/viewer.properties index 43ce1f317524b..da5442c68bda6 100644 --- a/l10n/en-US/viewer.properties +++ b/l10n/en-US/viewer.properties @@ -101,6 +101,7 @@ print_progress_close=Cancel # (the _label strings are alt text for the buttons, the .title strings are # tooltips) toggle_sidebar.title=Toggle Sidebar +toggle_sidebar_notification.title=Toggle Sidebar (document contains outline/attachments) toggle_sidebar_label=Toggle Sidebar document_outline.title=Show Document Outline (double-click to expand/collapse all items) document_outline_label=Document Outline diff --git a/l10n/sv-SE/viewer.properties b/l10n/sv-SE/viewer.properties index 6edaacbf1fcea..c1ff607c0cd85 100644 --- a/l10n/sv-SE/viewer.properties +++ b/l10n/sv-SE/viewer.properties @@ -101,6 +101,7 @@ print_progress_close=Avbryt # (the _label strings are alt text for the buttons, the .title strings are # tooltips) toggle_sidebar.title=Visa/dölj sidofält +toggle_sidebar_notification.title=Visa/dölj sidofält (dokumentet innehåller disposition/bilagor) toggle_sidebar_label=Visa/dölj sidofält document_outline.title=Visa dokumentdisposition (dubbelklicka för att expandera/komprimera alla objekt) document_outline_label=Dokumentöversikt diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index 8d9472950422e..f9106984138df 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -18,15 +18,20 @@ (function (root, factory) { if (typeof define === 'function' && define.amd) { define('pdfjs-web/pdf_sidebar', ['exports', - 'pdfjs-web/pdf_rendering_queue'], factory); + 'pdfjs-web/pdf_rendering_queue', 'pdfjs-web/ui_utils'], factory); } else if (typeof exports !== 'undefined') { - factory(exports, require('./pdf_rendering_queue.js')); + factory(exports, require('./pdf_rendering_queue.js'), + require('./ui_utils.js')); } else { - factory((root.pdfjsWebPDFSidebar = {}), root.pdfjsWebPDFRenderingQueue); + factory((root.pdfjsWebPDFSidebar = {}), root.pdfjsWebPDFRenderingQueue, + root.pdfjsWebUIUtils); } -}(this, function (exports, pdfRenderingQueue) { +}(this, function (exports, pdfRenderingQueue, uiUtils) { var RenderingStates = pdfRenderingQueue.RenderingStates; +var mozL10n = uiUtils.mozL10n; + +var UI_NOTIFICATION_CLASS = 'pdfSidebarNotification'; var SidebarView = { NONE: 0, @@ -59,6 +64,8 @@ var SidebarView = { * the outline is placed. * @property {HTMLDivElement} attachmentsView - The container in which * the attachments are placed. + * @property {boolean} disableNotification - (optional) Disable the notification + * for documents containing outline/attachments. The default value is `false`. */ /** @@ -97,6 +104,8 @@ var PDFSidebar = (function PDFSidebarClosure() { this.outlineView = options.outlineView; this.attachmentsView = options.attachmentsView; + this.disableNotification = options.disableNotification || false; + this._addEventListeners(); } @@ -104,7 +113,7 @@ var PDFSidebar = (function PDFSidebarClosure() { reset: function PDFSidebar_reset() { this.isInitialViewSet = false; - this.close(); + this._hideUINotification(null); this.switchView(SidebarView.THUMBS); this.outlineButton.disabled = false; @@ -220,8 +229,7 @@ var PDFSidebar = (function PDFSidebarClosure() { if (forceOpen && !this.isOpen) { this.open(); - // NOTE: `this.open` will trigger rendering, and dispatch the event. - return; + return; // NOTE: Opening will trigger rendering, and dispatch the event. } if (shouldForceRendering) { this._forceRendering(); @@ -229,6 +237,7 @@ var PDFSidebar = (function PDFSidebarClosure() { if (isViewChanged) { this._dispatchEvent(); } + this._hideUINotification(this.active); }, open: function PDFSidebar_open() { @@ -246,6 +255,8 @@ var PDFSidebar = (function PDFSidebarClosure() { } this._forceRendering(); this._dispatchEvent(); + + this._hideUINotification(this.active); }, close: function PDFSidebar_close() { @@ -276,7 +287,7 @@ var PDFSidebar = (function PDFSidebarClosure() { _dispatchEvent: function PDFSidebar_dispatchEvent() { this.eventBus.dispatch('sidebarviewchanged', { source: this, - view: this.visibleView + view: this.visibleView, }); }, @@ -311,6 +322,75 @@ var PDFSidebar = (function PDFSidebarClosure() { thumbnailViewer.scrollThumbnailIntoView(pdfViewer.currentPageNumber); }, + /** + * @private + */ + _showUINotification: function (view) { + if (this.disableNotification) { + return; + } + + this.toggleButton.title = mozL10n.get('toggle_sidebar_notification.title', + null, 'Toggle Sidebar (document contains outline/attachments)'); + + if (!this.isOpen) { + // Only show the notification on the `toggleButton` if the sidebar is + // currently closed, to avoid unnecessarily bothering the user. + this.toggleButton.classList.add(UI_NOTIFICATION_CLASS); + } else if (view === this.active) { + // If the sidebar is currently open *and* the `view` is visible, do not + // bother the user with a notification on the corresponding button. + return; + } + + switch (view) { + case SidebarView.OUTLINE: + this.outlineButton.classList.add(UI_NOTIFICATION_CLASS); + break; + case SidebarView.ATTACHMENTS: + this.attachmentsButton.classList.add(UI_NOTIFICATION_CLASS); + break; + } + }, + + /** + * @private + */ + _hideUINotification: function (view) { + if (this.disableNotification) { + return; + } + + var removeNotification = function (view) { + switch (view) { + case SidebarView.OUTLINE: + this.outlineButton.classList.remove(UI_NOTIFICATION_CLASS); + break; + case SidebarView.ATTACHMENTS: + this.attachmentsButton.classList.remove(UI_NOTIFICATION_CLASS); + break; + } + }.bind(this); + + if (!this.isOpen && view !== null) { + // Only hide the notifications when the sidebar is currently open, + // or when it is being reset (i.e. `view === null`). + return; + } + this.toggleButton.classList.remove(UI_NOTIFICATION_CLASS); + + if (view !== null) { + removeNotification(view); + return; + } + for (view in SidebarView) { // Remove all sidebar notifications on reset. + removeNotification(SidebarView[view]); + } + + this.toggleButton.title = mozL10n.get('toggle_sidebar.title', null, + 'Toggle Sidebar'); + }, + /** * @private */ @@ -344,7 +424,12 @@ var PDFSidebar = (function PDFSidebarClosure() { var outlineCount = e.outlineCount; self.outlineButton.disabled = !outlineCount; - if (!outlineCount && self.active === SidebarView.OUTLINE) { + + if (outlineCount) { + self._showUINotification(SidebarView.OUTLINE); + } else if (self.active === SidebarView.OUTLINE) { + // If the outline view was opened during document load, switch away + // from it if it turns out that the document has no outline. self.switchView(SidebarView.THUMBS); } }); @@ -353,7 +438,12 @@ var PDFSidebar = (function PDFSidebarClosure() { var attachmentsCount = e.attachmentsCount; self.attachmentsButton.disabled = !attachmentsCount; - if (!attachmentsCount && self.active === SidebarView.ATTACHMENTS) { + + if (attachmentsCount) { + self._showUINotification(SidebarView.ATTACHMENTS); + } else if (self.active === SidebarView.ATTACHMENTS) { + // If the attachment view was opened during document load, switch away + // from it if it turns out that the document has no attachments. self.switchView(SidebarView.THUMBS); } }); diff --git a/web/viewer.css b/web/viewer.css index 5de47bde571b0..612172e0c5389 100644 --- a/web/viewer.css +++ b/web/viewer.css @@ -920,6 +920,24 @@ html[dir="rtl"] #viewOutline.toolbarButton::before { content: url(images/toolbarButton-search.png); } +.toolbarButton.pdfSidebarNotification::after { + position: absolute; + display: inline-block; + top: 1px; + /* Create a filled circle, with a diameter of 9 pixels, using only CSS: */ + content: ''; + background-color: #70DB55; + height: 9px; + width: 9px; + border-radius: 50%; +} +html[dir='ltr'] .toolbarButton.pdfSidebarNotification::after { + left: 17px; +} +html[dir='rtl'] .toolbarButton.pdfSidebarNotification::after { + right: 17px; +} + .secondaryToolbarButton { position: relative; margin: 0 0 4px 0; From e96cdfdfdfc043ea1081dd0f4a72b132fb4edb7f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 15 Jan 2017 18:12:24 +0100 Subject: [PATCH 2/2] Append the contents of `FileAttachment` annotations to the attachments view of the sidebar, for easier access to the embedded files Other PDF viewers, e.g. Adobe Reader, seem to append `FileAttachment`s to their attachments views. One obvious difference in PDF.js is that we cannot append all the annotations on document load, since that would require parsing *every* page. Despite that, it still seems like a good idea to add `FileAttachment`s, since it's thus possible to access all the various types of attachments from a single place. *Note:* With the previous patch we display a notification when a `FileAttachment` is added to the sidebar, which thus makes appending the contents of these annotations to the sidebar slightly more visible/useful. --- src/display/annotation_layer.js | 18 ++++++++---- src/main_loader.js | 1 + src/pdf.js | 2 ++ test/driver.js | 14 ++++++++- web/pdf_attachment_viewer.js | 52 +++++++++++++++++++++++++++++---- web/pdf_link_service.js | 16 ++++++++++ 6 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index d8a5a8909d7eb..fded967d537ae 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -29,6 +29,7 @@ var AnnotationBorderStyleType = sharedUtil.AnnotationBorderStyleType; var AnnotationType = sharedUtil.AnnotationType; +var stringToPDFString = sharedUtil.stringToPDFString; var Util = sharedUtil.Util; var addLinkAttributes = displayDOMUtils.addLinkAttributes; var LinkTarget = displayDOMUtils.LinkTarget; @@ -1007,8 +1008,15 @@ var FileAttachmentAnnotationElement = ( function FileAttachmentAnnotationElement(parameters) { AnnotationElement.call(this, parameters, true); - this.filename = getFilenameFromUrl(parameters.data.file.filename); - this.content = parameters.data.file.content; + var file = this.data.file; + this.filename = getFilenameFromUrl(file.filename); + this.content = file.content; + + this.linkService.onFileAttachmentAnnotation({ + id: stringToPDFString(file.filename), + filename: file.filename, + content: file.content, + }); } Util.inherit(FileAttachmentAnnotationElement, AnnotationElement, { @@ -1086,8 +1094,7 @@ var AnnotationLayer = (function AnnotationLayerClosure() { if (!data) { continue; } - - var properties = { + var element = annotationElementFactory.create({ data: data, layer: parameters.div, page: parameters.page, @@ -1097,8 +1104,7 @@ var AnnotationLayer = (function AnnotationLayerClosure() { imageResourcesPath: parameters.imageResourcesPath || getDefaultSetting('imageResourcesPath'), renderInteractiveForms: parameters.renderInteractiveForms || false, - }; - var element = annotationElementFactory.create(properties); + }); if (element.isRenderable) { parameters.div.appendChild(element.render()); } diff --git a/src/main_loader.js b/src/main_loader.js index a1660908d9e25..1de29358f59e5 100644 --- a/src/main_loader.js +++ b/src/main_loader.js @@ -48,6 +48,7 @@ exports.renderTextLayer = displayTextLayer.renderTextLayer; exports.AnnotationLayer = displayAnnotationLayer.AnnotationLayer; exports.CustomStyle = displayDOMUtils.CustomStyle; + exports.createPromiseCapability = sharedUtil.createPromiseCapability; exports.PasswordResponses = sharedUtil.PasswordResponses; exports.InvalidPDFException = sharedUtil.InvalidPDFException; exports.MissingPDFException = sharedUtil.MissingPDFException; diff --git a/src/pdf.js b/src/pdf.js index e70d6f0deb8f4..31d127ef37c91 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -55,6 +55,8 @@ exports.AnnotationLayer = pdfjsLibs.pdfjsDisplayAnnotationLayer.AnnotationLayer; exports.CustomStyle = pdfjsLibs.pdfjsDisplayDOMUtils.CustomStyle; + exports.createPromiseCapability = + pdfjsLibs.pdfjsSharedUtil.createPromiseCapability; exports.PasswordResponses = pdfjsLibs.pdfjsSharedUtil.PasswordResponses; exports.InvalidPDFException = pdfjsLibs.pdfjsSharedUtil.InvalidPDFException; exports.MissingPDFException = pdfjsLibs.pdfjsSharedUtil.MissingPDFException; diff --git a/test/driver.js b/test/driver.js index 466cd2bedc1dc..7136b54e4893b 100644 --- a/test/driver.js +++ b/test/driver.js @@ -26,6 +26,12 @@ var LinkServiceMock = (function LinkServiceMockClosure() { function LinkServiceMock() {} LinkServiceMock.prototype = { + get page() { + return 0; + }, + + set page(value) {}, + navigateTo: function (dest) {}, getDestinationHash: function (dest) { @@ -36,7 +42,13 @@ var LinkServiceMock = (function LinkServiceMockClosure() { return '#'; }, - executeNamedAction: function (action) {} + setHash: function (hash) {}, + + executeNamedAction: function (action) {}, + + onFileAttachmentAnnotation: function (params) {}, + + cachePageRef: function (pageNum, pageRef) {}, }; return LinkServiceMock; diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index 2f171785e8f6c..dbfda4b3d9c2a 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -51,16 +51,26 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { this.container = options.container; this.eventBus = options.eventBus; this.downloadManager = options.downloadManager; + + this._renderedCapability = pdfjsLib.createPromiseCapability(); + this.eventBus.on('fileattachmentannotation', + this._appendAttachment.bind(this)); } PDFAttachmentViewer.prototype = { - reset: function PDFAttachmentViewer_reset() { + reset: function PDFAttachmentViewer_reset(keepRenderedCapability) { this.attachments = null; var container = this.container; while (container.firstChild) { container.removeChild(container.firstChild); } + + if (!keepRenderedCapability) { + // NOTE: The *only* situation in which the `_renderedCapability` should + // not be replaced is when appending file attachment annotations. + this._renderedCapability = pdfjsLib.createPromiseCapability(); + } }, /** @@ -70,8 +80,10 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { function PDFAttachmentViewer_dispatchEvent(attachmentsCount) { this.eventBus.dispatch('attachmentsloaded', { source: this, - attachmentsCount: attachmentsCount + attachmentsCount: attachmentsCount, }); + + this._renderedCapability.resolve(); }, /** @@ -89,11 +101,13 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { * @param {PDFAttachmentViewerRenderParameters} params */ render: function PDFAttachmentViewer_render(params) { - var attachments = (params && params.attachments) || null; + params = params || {}; + var attachments = params.attachments || null; var attachmentsCount = 0; if (this.attachments) { - this.reset(); + var keepRenderedCapability = params.keepRenderedCapability === true; + this.reset(keepRenderedCapability); } this.attachments = attachments; @@ -120,7 +134,35 @@ var PDFAttachmentViewer = (function PDFAttachmentViewerClosure() { } this._dispatchEvent(attachmentsCount); - } + }, + + /** + * Used to append FileAttachment annotations to the sidebar. + * @private + */ + _appendAttachment: function PDFAttachmentViewer_appendAttachment(item) { + this._renderedCapability.promise.then(function (id, filename, content) { + var attachments = this.attachments; + + if (!attachments) { + attachments = Object.create(null); + } else { + for (var name in attachments) { + if (id === name) { + return; // Ignore the new attachment if it already exists. + } + } + } + attachments[id] = { + filename: filename, + content: content, + }; + this.render({ + attachments: attachments, + keepRenderedCapability: true, + }); + }.bind(this, item.id, item.filename, item.content)); + }, }; return PDFAttachmentViewer; diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js index 7dd7478d06bda..15ca78b17735e 100644 --- a/web/pdf_link_service.js +++ b/web/pdf_link_service.js @@ -350,6 +350,18 @@ var PDFLinkService = (function PDFLinkServiceClosure() { }); }, + /** + * @param {Object} params + */ + onFileAttachmentAnnotation: function (params) { + this.eventBus.dispatch('fileattachmentannotation', { + source: this, + id: params.id, + filename: params.filename, + content: params.content, + }); + }, + /** * @param {number} pageNum - page number. * @param {Object} pageRef - reference to the page. @@ -462,6 +474,10 @@ var SimpleLinkService = (function SimpleLinkServiceClosure() { * @param {string} action */ executeNamedAction: function (action) {}, + /** + * @param {Object} params + */ + onFileAttachmentAnnotation: function (params) {}, /** * @param {number} pageNum - page number. * @param {Object} pageRef - reference to the page.