From ca69da735e72dc9df2144861c4d1b2406c84dca5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 6 Dec 2022 23:34:55 +0100 Subject: [PATCH] [api-minor] Remove the `annotationLayerFactory` in the viewer Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this. Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things. Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication. Hence this patch, which removes the `annotationLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary. Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration. --- [1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work. --- web/default_factory.js | 63 ------------------------------ web/interfaces.js | 45 --------------------- web/pdf_page_view.js | 50 ++++++++++++++++-------- web/pdf_viewer.component.js | 11 +++++- web/pdf_viewer.js | 78 +++++++++---------------------------- 5 files changed, 62 insertions(+), 185 deletions(-) diff --git a/web/default_factory.js b/web/default_factory.js index deec730fe..0ff79bfa9 100644 --- a/web/default_factory.js +++ b/web/default_factory.js @@ -19,9 +19,6 @@ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IDownloadManager} IDownloadManager */ /** @typedef {import("./interfaces").IL10n} IL10n */ -// eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ -// eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ @@ -32,70 +29,11 @@ // eslint-disable-next-line max-len /** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ -import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; -import { NullL10n } from "./l10n_utils.js"; import { SimpleLinkService } from "./pdf_link_service.js"; import { StructTreeLayerBuilder } from "./struct_tree_layer_builder.js"; import { TextLayerBuilder } from "./text_layer_builder.js"; import { XfaLayerBuilder } from "./xfa_layer_builder.js"; -/** - * @implements IPDFAnnotationLayerFactory - */ -class DefaultAnnotationLayerFactory { - /** - * @typedef {Object} CreateAnnotationLayerBuilderParameters - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {AnnotationStorage} [annotationStorage] - Storage for annotation - * data in forms. - * @property {string} [imageResourcesPath] - Path for image resources, mainly - * for annotation icons. Include trailing slash. - * @property {boolean} renderForms - * @property {IL10n} l10n - * @property {boolean} [enableScripting] - * @property {Promise} [hasJSActionsPromise] - * @property {Promise> | null>} - * [fieldObjectsPromise] - * @property {Map} [annotationCanvasMap] - Map some - * annotation ids with canvases used to render them. - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationLayerBuilderParameters} - * @returns {AnnotationLayerBuilder} - */ - createAnnotationLayerBuilder({ - pageDiv, - pdfPage, - annotationStorage = null, - imageResourcesPath = "", - renderForms = true, - l10n = NullL10n, - enableScripting = false, - hasJSActionsPromise = null, - fieldObjectsPromise = null, - annotationCanvasMap = null, - accessibilityManager = null, - }) { - return new AnnotationLayerBuilder({ - pageDiv, - pdfPage, - imageResourcesPath, - renderForms, - linkService: new SimpleLinkService(), - l10n, - annotationStorage, - enableScripting, - hasJSActionsPromise, - fieldObjectsPromise, - annotationCanvasMap, - accessibilityManager, - }); - } -} - /** * @implements IPDFStructTreeLayerFactory */ @@ -163,7 +101,6 @@ class DefaultXfaLayerFactory { } export { - DefaultAnnotationLayerFactory, DefaultStructTreeLayerFactory, DefaultTextLayerFactory, DefaultXfaLayerFactory, diff --git a/web/interfaces.js b/web/interfaces.js index faac8d7b9..c963cac50 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -18,8 +18,6 @@ // eslint-disable-next-line max-len /** @typedef {import("../src/display/display_utils").PageViewport} PageViewport */ // eslint-disable-next-line max-len -/** @typedef {import("./annotation_layer_builder").AnnotationLayerBuilder} AnnotationLayerBuilder */ -// eslint-disable-next-line max-len /** @typedef {import("./struct_tree_builder").StructTreeLayerBuilder} StructTreeLayerBuilder */ /** @typedef {import("./text_highlighter").TextHighlighter} TextHighlighter */ // eslint-disable-next-line max-len @@ -181,48 +179,6 @@ class IPDFTextLayerFactory { }) {} } -/** - * @interface - */ -class IPDFAnnotationLayerFactory { - /** - * @typedef {Object} CreateAnnotationLayerBuilderParameters - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {AnnotationStorage} [annotationStorage] - Storage for annotation - * data in forms. - * @property {string} [imageResourcesPath] - Path for image resources, mainly - * for annotation icons. Include trailing slash. - * @property {boolean} renderForms - * @property {IL10n} l10n - * @property {boolean} [enableScripting] - * @property {Promise} [hasJSActionsPromise] - * @property {Promise> | null>} - * [fieldObjectsPromise] - * @property {Map} [annotationCanvasMap] - Map some - * annotation ids with canvases used to render them. - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationLayerBuilderParameters} - * @returns {AnnotationLayerBuilder} - */ - createAnnotationLayerBuilder({ - pageDiv, - pdfPage, - annotationStorage = null, - imageResourcesPath = "", - renderForms = true, - l10n = undefined, - enableScripting = false, - hasJSActionsPromise = null, - fieldObjectsPromise = null, - annotationCanvasMap = null, - accessibilityManager = null, - }) {} -} - /** * @interface */ @@ -321,7 +277,6 @@ class IL10n { export { IDownloadManager, IL10n, - IPDFAnnotationLayerFactory, IPDFLinkService, IPDFStructTreeLayerFactory, IPDFTextLayerFactory, diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 475b79820..a3638041b 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -20,8 +20,6 @@ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IL10n} IL10n */ // eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ -// eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */ @@ -51,8 +49,10 @@ import { TextLayerMode, } from "./ui_utils.js"; import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js"; +import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; import { compatibilityParams } from "./app_options.js"; import { NullL10n } from "./l10n_utils.js"; +import { SimpleLinkService } from "./pdf_link_service.js"; import { TextAccessibilityManager } from "./text_accessibility.js"; /** @@ -75,7 +75,6 @@ import { TextAccessibilityManager } from "./text_accessibility.js"; * being rendered. The constants from {@link AnnotationMode} should be used; * see also {@link RenderParameters} and {@link GetOperatorListParameters}. * The default value is `AnnotationMode.ENABLE_FORMS`. - * @property {IPDFAnnotationLayerFactory} [annotationLayerFactory] * @property {IPDFXfaLayerFactory} [xfaLayerFactory] * @property {IPDFStructTreeLayerFactory} [structTreeLayerFactory] * @property {Object} [textHighlighterFactory] @@ -104,6 +103,12 @@ const DEFAULT_LAYER_PROPERTIES = () => { } return { annotationEditorUIManager: null, + annotationStorage: null, + downloadManager: null, + enableScripting: false, + fieldObjectsPromise: null, + hasJSActionsPromise: null, + linkService: new SimpleLinkService(), }; }; @@ -153,7 +158,6 @@ class PDFPageView { this.eventBus = options.eventBus; this.renderingQueue = options.renderingQueue; this.textLayerFactory = options.textLayerFactory; - this.annotationLayerFactory = options.annotationLayerFactory; this.xfaLayerFactory = options.xfaLayerFactory; this._textHighlighterFactory = options.textHighlighterFactory; this.structTreeLayerFactory = options.structTreeLayerFactory; @@ -778,20 +782,34 @@ class PDFPageView { } if ( - this.#annotationMode !== AnnotationMode.DISABLE && - this.annotationLayerFactory + !this.annotationLayer && + this.#annotationMode !== AnnotationMode.DISABLE ) { + const { + annotationStorage, + downloadManager, + enableScripting, + fieldObjectsPromise, + hasJSActionsPromise, + linkService, + } = this.#layerProperties(); + this._annotationCanvasMap ||= new Map(); - this.annotationLayer ||= - this.annotationLayerFactory.createAnnotationLayerBuilder({ - pageDiv: div, - pdfPage, - imageResourcesPath: this.imageResourcesPath, - renderForms: this.#annotationMode === AnnotationMode.ENABLE_FORMS, - l10n: this.l10n, - annotationCanvasMap: this._annotationCanvasMap, - accessibilityManager: this._accessibilityManager, - }); + this.annotationLayer = new AnnotationLayerBuilder({ + pageDiv: div, + pdfPage, + annotationStorage, + imageResourcesPath: this.imageResourcesPath, + renderForms: this.#annotationMode === AnnotationMode.ENABLE_FORMS, + linkService, + downloadManager, + l10n: this.l10n, + enableScripting, + hasJSActionsPromise, + fieldObjectsPromise, + annotationCanvasMap: this._annotationCanvasMap, + accessibilityManager: this._accessibilityManager, + }); } if (this.xfaLayer?.div) { diff --git a/web/pdf_viewer.component.js b/web/pdf_viewer.component.js index bf25bcbb8..cd4ba1fc5 100644 --- a/web/pdf_viewer.component.js +++ b/web/pdf_viewer.component.js @@ -14,7 +14,6 @@ */ import { - DefaultAnnotationLayerFactory, DefaultStructTreeLayerFactory, DefaultTextLayerFactory, DefaultXfaLayerFactory, @@ -51,6 +50,16 @@ const pdfjsVersion = PDFJSDev.eval("BUNDLE_VERSION"); // eslint-disable-next-line no-unused-vars const pdfjsBuild = PDFJSDev.eval("BUNDLE_BUILD"); +class DefaultAnnotationLayerFactory { + constructor() { + throw new Error( + "The `DefaultAnnotationLayerFactory` has been removed, " + + "please use the `annotationMode` option when initializing " + + "the `PDFPageView`-instance to control AnnotationLayer rendering." + ); + } +} + export { AnnotationLayerBuilder, DefaultAnnotationLayerFactory, diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 8d6d76ffc..481fc01f8 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -22,9 +22,6 @@ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IDownloadManager} IDownloadManager */ /** @typedef {import("./interfaces").IL10n} IL10n */ -// eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ -// eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ @@ -68,7 +65,6 @@ import { VERTICAL_PADDING, watchScroll, } from "./ui_utils.js"; -import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; import { NullL10n } from "./l10n_utils.js"; import { PDFPageView } from "./pdf_page_view.js"; import { PDFRenderingQueue } from "./pdf_rendering_queue.js"; @@ -210,7 +206,6 @@ class PDFPageViewBuffer { /** * Simple viewer control to display PDF content/pages. * - * @implements {IPDFAnnotationLayerFactory} * @implements {IPDFStructTreeLayerFactory} * @implements {IPDFTextLayerFactory} * @implements {IPDFXfaLayerFactory} @@ -562,6 +557,24 @@ class PDFViewer { get annotationEditorUIManager() { return self.#annotationEditorUIManager; }, + get annotationStorage() { + return self.pdfDocument?.annotationStorage; + }, + get downloadManager() { + return self.downloadManager; + }, + get enableScripting() { + return !!self._scriptingManager; + }, + get fieldObjectsPromise() { + return self.pdfDocument?.getFieldObjects(); + }, + get hasJSActionsPromise() { + return self.pdfDocument?.hasJSActions(); + }, + get linkService() { + return self.linkService; + }, }; } @@ -776,8 +789,6 @@ class PDFViewer { textLayerFactory: textLayerMode !== TextLayerMode.DISABLE ? this : null, textLayerMode, - annotationLayerFactory: - annotationMode !== AnnotationMode.DISABLE ? this : null, annotationMode, xfaLayerFactory: this, textHighlighterFactory: this, @@ -1696,59 +1707,6 @@ class PDFViewer { }); } - /** - * @typedef {Object} CreateAnnotationLayerBuilderParameters - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {AnnotationStorage} [annotationStorage] - Storage for annotation - * data in forms. - * @property {string} [imageResourcesPath] - Path for image resources, mainly - * for annotation icons. Include trailing slash. - * @property {boolean} renderForms - * @property {IL10n} l10n - * @property {boolean} [enableScripting] - * @property {Promise} [hasJSActionsPromise] - * @property {Promise> | null>} - * [fieldObjectsPromise] - * @property {Map} [annotationCanvasMap] - Map some - * annotation ids with canvases used to render them. - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationLayerBuilderParameters} - * @returns {AnnotationLayerBuilder} - */ - createAnnotationLayerBuilder({ - pageDiv, - pdfPage, - annotationStorage = this.pdfDocument?.annotationStorage, - imageResourcesPath = "", - renderForms = true, - l10n = NullL10n, - enableScripting = this.enableScripting, - hasJSActionsPromise = this.pdfDocument?.hasJSActions(), - fieldObjectsPromise = this.pdfDocument?.getFieldObjects(), - annotationCanvasMap = null, - accessibilityManager = null, - }) { - return new AnnotationLayerBuilder({ - pageDiv, - pdfPage, - annotationStorage, - imageResourcesPath, - renderForms, - linkService: this.linkService, - downloadManager: this.downloadManager, - l10n, - enableScripting, - hasJSActionsPromise, - fieldObjectsPromise, - annotationCanvasMap, - accessibilityManager, - }); - } - /** * @typedef {Object} CreateXfaLayerBuilderParameters * @property {HTMLDivElement} pageDiv