From 7f40ef41a5cb7ec89e0f7227ba8eb9f35e24b8d3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 6 May 2022 20:45:53 +0200 Subject: [PATCH] Simplify the "fileattachmentannotation"-event handling a little bit *This patch can be tested, in the viewer, using the `annotation-fileattachment.pdf` document from the test-suite.* Note how the `FileSpec`-implementation already uses `stringToPDFString` during the filename lookup, see https://github.com/mozilla/pdf.js/blob/cfac6fa511945ae5b75d3c27a9356527862a7bf6/src/core/file_spec.js#L70 Hence there's no reason to repeat that again in the `FileAttachmentAnnotationElement`-constructor, and we can thus simplify the "fileattachmentannotation"-event handling a little bit. --- src/display/annotation_layer.js | 2 -- web/pdf_attachment_viewer.js | 19 +++++++------------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 8332a2f79..16c0bed48 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -23,7 +23,6 @@ import { AnnotationType, assert, shadow, - stringToPDFString, unreachable, Util, warn, @@ -2296,7 +2295,6 @@ class FileAttachmentAnnotationElement extends AnnotationElement { this.linkService.eventBus?.dispatch("fileattachmentannotation", { source: this, - id: stringToPDFString(filename), filename, content, }); diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index a0c588ffc..5cf138728 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -38,7 +38,7 @@ class PDFAttachmentViewer extends BaseTreeViewer { this.eventBus._on( "fileattachmentannotation", - this._appendAttachment.bind(this) + this.#appendAttachment.bind(this) ); } @@ -140,27 +140,22 @@ class PDFAttachmentViewer extends BaseTreeViewer { /** * Used to append FileAttachment annotations to the sidebar. - * @private */ - _appendAttachment({ id, filename, content }) { + #appendAttachment({ filename, content }) { const renderedPromise = this._renderedCapability.promise; renderedPromise.then(() => { if (renderedPromise !== this._renderedCapability.promise) { return; // The FileAttachment annotation belongs to a previous document. } - let attachments = this._attachments; + const attachments = this._attachments || Object.create(null); - if (!attachments) { - attachments = Object.create(null); - } else { - for (const name in attachments) { - if (id === name) { - return; // Ignore the new attachment if it already exists. - } + for (const name in attachments) { + if (filename === name) { + return; // Ignore the new attachment if it already exists. } } - attachments[id] = { + attachments[filename] = { filename, content, };