From f07a1065298bfe1beb3a02a8d220c66ff45da4c0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 23 Jun 2026 10:10:40 +0200 Subject: [PATCH 1/4] Include the `catalog` instance in the `annotationGlobals` data The `FileAttachmentAnnotation` and `MediaAnnotation` code needs to (synchronously) access a `catalog` method, which leads to unnecessarily verbose code. This can be avoided by including the `catalog` instance in the `annotationGlobals` data, which is safe since it already includes data that's fetched asynchronously from the `catalog` instance. --- src/core/annotation.js | 25 ++++++++----------------- test/unit/document_spec.js | 2 ++ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index 8eed43b5e..3a535f137 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -81,10 +81,6 @@ import { parseMarkedContentProps } from "./evaluator_utils.js"; import { StringStream } from "./stream.js"; import { XFAFactory } from "./xfa/factory.js"; -/** - * @import { Catalog } from "./catalog.js"; - */ - class AnnotationFactory { static createGlobals(pdfManager) { return Promise.all([ @@ -108,6 +104,7 @@ class AnnotationFactory { globalColorSpaceCache, ]) => ({ pdfManager, + catalog: pdfManager.pdfDocument.catalog, acroForm: acroForm instanceof Dict ? acroForm : Dict.empty, xfaDatasets, structTreeRoot, @@ -5421,10 +5418,6 @@ class FileAttachmentAnnotation extends MarkupAnnotation { const { annotationGlobals, dict } = params; const fsDict = dict.get("FS"); - const file = new FileSpec(fsDict); - /** @type {{catalog?: Catalog}} */ - const { catalog } = annotationGlobals.pdfManager.pdfDocument; - // Encode the embedded content's reference in the id so it can be // re-fetched from the xref on demand (see `Catalog.attachmentContent`) // instead of being cached where `cleanup` would wipe it. The file-spec is @@ -5440,14 +5433,15 @@ class FileAttachmentAnnotation extends MarkupAnnotation { ); } if (contentRef instanceof Ref) { - fileId = catalog?.getAttachmentIdForAnnotation(contentRef); + fileId = + annotationGlobals.catalog.getAttachmentIdForAnnotation(contentRef); } } this.data.hasOwnCanvas = this.data.noRotate; this.data.noHTML = false; this.data.fileId = fileId; - this.data.file = file.serializable; + this.data.file = new FileSpec(fsDict).serializable; const name = dict.get("Name"); this.data.name = @@ -5490,7 +5484,7 @@ class MediaAnnotation extends Annotation { * when `assetRef` isn't itself a reference. * @param {string} asset.filename * @param {string} asset.contentType - * @param {Catalog} [catalog] + * @param {Catalog} catalog */ _setMediaData({ assetRef, assetDict, filename, contentType }, catalog) { let contentRef = assetRef; @@ -5502,7 +5496,7 @@ class MediaAnnotation extends Annotation { } const fileId = contentRef instanceof Ref - ? catalog?.getAttachmentIdForAnnotation(contentRef) + ? catalog.getAttachmentIdForAnnotation(contentRef) : undefined; this.data.noHTML = false; @@ -5577,8 +5571,6 @@ class RichMediaAnnotation extends MediaAnnotation { super(params); const { dict, xref, annotationGlobals } = params; - /** @type {{catalog?: Catalog}} */ - const { catalog } = annotationGlobals.pdfManager.pdfDocument; const content = dict.get("RichMediaContent"); if (!(content instanceof Dict)) { @@ -5590,8 +5582,7 @@ class RichMediaAnnotation extends MediaAnnotation { warn("RichMedia annotation has no playable asset."); return; } - - this._setMediaData(asset, catalog); + this._setMediaData(asset, annotationGlobals.catalog); } /** @@ -5675,7 +5666,7 @@ class ScreenAnnotation extends MediaAnnotation { // a /Movie); such ones simply render their appearance, so don't warn. return; } - this._setMediaData(asset, annotationGlobals.pdfManager.pdfDocument.catalog); + this._setMediaData(asset, annotationGlobals.catalog); } /** diff --git a/test/unit/document_spec.js b/test/unit/document_spec.js index b0224e882..e6eeb602c 100644 --- a/test/unit/document_spec.js +++ b/test/unit/document_spec.js @@ -73,6 +73,8 @@ describe("document", function () { const pdfDocument = new PDFDocument(pdfManager, stream); pdfDocument.xref = xref; pdfDocument.catalog = catalog; + + pdfManager.pdfDocument = pdfDocument; return pdfDocument; } From 8a2c112c20d3f228d116bb0e6fa2602b0ab50eb5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 23 Jun 2026 10:22:08 +0200 Subject: [PATCH 2/4] Simplify the `Annotation.prototype.setAppearance` method a tiny bit It's not necessary to check if the /AS entry exists first, and it can just be fetched directly, since in that case the existing "is stream"-check won't be true anyway. Also, move the `appearance` field definition to the top of the class instead. --- src/core/annotation.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index 3a535f137..a93c708fc 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -687,6 +687,8 @@ function getTransformMatrix(rect, bbox, matrix) { } class Annotation { + appearance = null; + _oc = undefined; constructor(params) { @@ -1175,8 +1177,6 @@ class Annotation { * @param {Dict} dict - The annotation's data dictionary */ setAppearance(dict) { - this.appearance = null; - const appearanceStates = dict.get("AP"); if (!(appearanceStates instanceof Dict)) { return; @@ -1195,7 +1195,7 @@ class Annotation { // In case the normal appearance is a dictionary, the `AS` entry provides // the key of the stream in this dictionary. const as = dict.get("AS"); - if (!(as instanceof Name) || !normalAppearanceState.has(as.name)) { + if (!(as instanceof Name)) { return; } const appearance = normalAppearanceState.get(as.name); From 15d93e1f34035c178e507e34a3af6f76c631b44e Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 24 Jun 2026 10:31:07 +0200 Subject: [PATCH 3/4] Introduce a helper method, in the `Annotation` class, for determining the attachment `fileId` This avoids duplication between the `FileAttachmentAnnotation` and `MediaAnnotation` classes, since they currently include essentially the same code for determining the attachment `fileId`. --- src/core/annotation.js | 73 +++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index a93c708fc..ad9d79efa 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -1502,6 +1502,25 @@ class Annotation { return fieldName.join("."); } + /** + * Encode the embedded content's reference in the id so it can be + * re-fetched from the xref on demand (see `Catalog.attachmentContent`) + * instead of being cached where `cleanup` would wipe it. The file-spec is + * usually indirect; when it's inline its embedded-file stream still isn't + * (streams are always indirect), so fall back to that ref. + */ + _getAttachmentId(fsDict, fsRef, annotationGlobals) { + if (!(fsDict instanceof Dict)) { + return undefined; + } + if (!(fsRef instanceof Ref)) { + fsRef = FileSpec.pickPlatformItem(fsDict.get("EF"), /* raw = */ true); + } + return fsRef instanceof Ref + ? annotationGlobals.catalog.getAttachmentIdForAnnotation(fsRef) + : undefined; + } + get width() { return this.data.rect[2] - this.data.rect[0]; } @@ -5418,29 +5437,14 @@ class FileAttachmentAnnotation extends MarkupAnnotation { const { annotationGlobals, dict } = params; const fsDict = dict.get("FS"); - // Encode the embedded content's reference in the id so it can be - // re-fetched from the xref on demand (see `Catalog.attachmentContent`) - // instead of being cached where `cleanup` would wipe it. The file-spec is - // usually indirect; when it's inline its embedded-file stream still isn't - // (streams are always indirect), so fall back to that ref. - let fileId; - if (fsDict instanceof Dict) { - let contentRef = dict.getRaw("FS"); - if (!(contentRef instanceof Ref)) { - contentRef = FileSpec.pickPlatformItem( - fsDict.get("EF"), - /* raw = */ true - ); - } - if (contentRef instanceof Ref) { - fileId = - annotationGlobals.catalog.getAttachmentIdForAnnotation(contentRef); - } - } this.data.hasOwnCanvas = this.data.noRotate; this.data.noHTML = false; - this.data.fileId = fileId; + this.data.fileId = this._getAttachmentId( + fsDict, + dict.getRaw("FS"), + annotationGlobals + ); this.data.file = new FileSpec(fsDict).serializable; const name = dict.get("Name"); @@ -5484,23 +5488,18 @@ class MediaAnnotation extends Annotation { * when `assetRef` isn't itself a reference. * @param {string} asset.filename * @param {string} asset.contentType - * @param {Catalog} catalog + * @param {Object} annotationGlobals */ - _setMediaData({ assetRef, assetDict, filename, contentType }, catalog) { - let contentRef = assetRef; - if (!(contentRef instanceof Ref)) { - contentRef = FileSpec.pickPlatformItem( - assetDict.get("EF"), - /* raw = */ true - ); - } - const fileId = - contentRef instanceof Ref - ? catalog.getAttachmentIdForAnnotation(contentRef) - : undefined; - + _setMediaData( + { assetRef, assetDict, filename, contentType }, + annotationGlobals + ) { this.data.noHTML = false; - this.data.richMedia = { fileId, filename, contentType }; + this.data.richMedia = { + fileId: this._getAttachmentId(assetDict, assetRef, annotationGlobals), + filename, + contentType, + }; } /** @@ -5582,7 +5581,7 @@ class RichMediaAnnotation extends MediaAnnotation { warn("RichMedia annotation has no playable asset."); return; } - this._setMediaData(asset, annotationGlobals.catalog); + this._setMediaData(asset, annotationGlobals); } /** @@ -5666,7 +5665,7 @@ class ScreenAnnotation extends MediaAnnotation { // a /Movie); such ones simply render their appearance, so don't warn. return; } - this._setMediaData(asset, annotationGlobals.catalog); + this._setMediaData(asset, annotationGlobals); } /** From 44e637a0641fe0e7d70f12b9523cbf2eea90f544 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 24 Jun 2026 10:39:12 +0200 Subject: [PATCH 4/4] Remove explicit `xref` usage in the `ScreenAnnotation.prototype.#renditionActions` method Rather than fetching "raw" dictionary-data and then manually resolving any references, we can simply use `Dict.prototype.get` and `Dict`-iteration to access the needed data *directly* instead. --- src/core/annotation.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index ad9d79efa..d3caab0b8 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -5686,7 +5686,7 @@ class ScreenAnnotation extends MediaAnnotation { * } | null} */ static #findAsset(dict, xref) { - for (const action of this.#renditionActions(dict, xref)) { + for (const action of this.#renditionActions(dict)) { const asset = this.#findRenditionAsset( action.get("R"), xref, @@ -5699,10 +5699,10 @@ class ScreenAnnotation extends MediaAnnotation { return null; } - static *#renditionActions(dict, xref) { + static *#renditionActions(dict) { // The rendition action may be the activation action (/A) or one of the // additional actions (/AA), e.g. page-open. - const action = xref.fetchIfRef(dict.getRaw("A")); + const action = dict.get("A"); if ( action instanceof Dict && isName(action.get("S"), "Rendition") && @@ -5712,8 +5712,7 @@ class ScreenAnnotation extends MediaAnnotation { } const additionalActions = dict.get("AA"); if (additionalActions instanceof Dict) { - for (const key of additionalActions.getKeys()) { - const aa = xref.fetchIfRef(additionalActions.getRaw(key)); + for (const [, aa] of additionalActions) { if ( aa instanceof Dict && isName(aa.get("S"), "Rendition") &&