From 7aedb8ed7a7f58d185b71f43ff6b60adf2753d7e Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 6 Dec 2022 14:21:51 +0100 Subject: [PATCH] [api-minor] Remove the `annotationEditorLayerFactory` 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 `annotationEditorLayerFactory` 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/annotation_editor_layer_builder.js | 2 +- web/default_factory.js | 37 --------------------- web/interfaces.js | 29 ---------------- web/pdf_page_view.js | 45 ++++++++++++++++--------- web/pdf_viewer.js | 46 ++++++-------------------- 5 files changed, 42 insertions(+), 117 deletions(-) diff --git a/web/annotation_editor_layer_builder.js b/web/annotation_editor_layer_builder.js index 06f59526e..09bc560ba 100644 --- a/web/annotation_editor_layer_builder.js +++ b/web/annotation_editor_layer_builder.js @@ -30,7 +30,7 @@ import { NullL10n } from "./l10n_utils.js"; * @property {AnnotationEditorUIManager} [uiManager] * @property {HTMLDivElement} pageDiv * @property {PDFPageProxy} pdfPage - * @property {IL10n} l10n + * @property {IL10n} [l10n] * @property {TextAccessibilityManager} [accessibilityManager] */ diff --git a/web/default_factory.js b/web/default_factory.js index bba1cd115..deec730fe 100644 --- a/web/default_factory.js +++ b/web/default_factory.js @@ -22,7 +22,6 @@ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ // eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationEditorLayerFactory} IPDFAnnotationEditorLayerFactory */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ @@ -33,7 +32,6 @@ // eslint-disable-next-line max-len /** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ -import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js"; import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; import { NullL10n } from "./l10n_utils.js"; import { SimpleLinkService } from "./pdf_link_service.js"; @@ -98,40 +96,6 @@ class DefaultAnnotationLayerFactory { } } -/** - * @implements IPDFAnnotationEditorLayerFactory - */ -class DefaultAnnotationEditorLayerFactory { - /** - * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters - * @property {AnnotationEditorUIManager} [uiManager] - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {IL10n} l10n - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationEditorLayerBuilderParameters} - * @returns {AnnotationEditorLayerBuilder} - */ - createAnnotationEditorLayerBuilder({ - uiManager = null, - pageDiv, - pdfPage, - accessibilityManager = null, - l10n, - }) { - return new AnnotationEditorLayerBuilder({ - uiManager, - pageDiv, - pdfPage, - accessibilityManager, - l10n, - }); - } -} - /** * @implements IPDFStructTreeLayerFactory */ @@ -199,7 +163,6 @@ class DefaultXfaLayerFactory { } export { - DefaultAnnotationEditorLayerFactory, DefaultAnnotationLayerFactory, DefaultStructTreeLayerFactory, DefaultTextLayerFactory, diff --git a/web/interfaces.js b/web/interfaces.js index fce1898d9..faac8d7b9 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -20,8 +20,6 @@ // eslint-disable-next-line max-len /** @typedef {import("./annotation_layer_builder").AnnotationLayerBuilder} AnnotationLayerBuilder */ // eslint-disable-next-line max-len -/** @typedef {import("./annotation_editor_layer_builder").AnnotationEditorLayerBuilder} AnnotationEditorLayerBuilder */ -// 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 @@ -225,32 +223,6 @@ class IPDFAnnotationLayerFactory { }) {} } -/** - * @interface - */ -class IPDFAnnotationEditorLayerFactory { - /** - * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters - * @property {AnnotationEditorUIManager} [uiManager] - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {IL10n} l10n - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationEditorLayerBuilderParameters} - * @returns {AnnotationEditorLayerBuilder} - */ - createAnnotationEditorLayerBuilder({ - uiManager = null, - pageDiv, - pdfPage, - l10n, - accessibilityManager, - }) {} -} - /** * @interface */ @@ -349,7 +321,6 @@ class IL10n { export { IDownloadManager, IL10n, - IPDFAnnotationEditorLayerFactory, IPDFAnnotationLayerFactory, IPDFLinkService, IPDFStructTreeLayerFactory, diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 49826c438..475b79820 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -22,8 +22,6 @@ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ // eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationEditorLayerFactory} IPDFAnnotationEditorLayerFactory */ -// eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */ @@ -52,6 +50,7 @@ import { roundToDivide, TextLayerMode, } from "./ui_utils.js"; +import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js"; import { compatibilityParams } from "./app_options.js"; import { NullL10n } from "./l10n_utils.js"; import { TextAccessibilityManager } from "./text_accessibility.js"; @@ -77,7 +76,6 @@ import { TextAccessibilityManager } from "./text_accessibility.js"; * see also {@link RenderParameters} and {@link GetOperatorListParameters}. * The default value is `AnnotationMode.ENABLE_FORMS`. * @property {IPDFAnnotationLayerFactory} [annotationLayerFactory] - * @property {IPDFAnnotationEditorLayerFactory} [annotationEditorLayerFactory] * @property {IPDFXfaLayerFactory} [xfaLayerFactory] * @property {IPDFStructTreeLayerFactory} [structTreeLayerFactory] * @property {Object} [textHighlighterFactory] @@ -94,16 +92,29 @@ import { TextAccessibilityManager } from "./text_accessibility.js"; * with user defined ones in order to improve readability in high contrast * mode. * @property {IL10n} [l10n] - Localization service. + * @property {function} [layerProperties] - The function that is used to lookup + * the necessary layer-properties. */ const MAX_CANVAS_PIXELS = compatibilityParams.maxCanvasPixels || 16777216; +const DEFAULT_LAYER_PROPERTIES = () => { + if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("COMPONENTS")) { + return null; + } + return { + annotationEditorUIManager: null, + }; +}; + /** * @implements {IRenderableView} */ class PDFPageView { #annotationMode = AnnotationMode.ENABLE_FORMS; + #layerProperties = null; + #useThumbnailCanvas = { initialOptionalContent: true, regularAnnotations: true, @@ -118,6 +129,7 @@ class PDFPageView { this.id = options.id; this.renderingId = "page" + this.id; + this.#layerProperties = options.layerProperties || DEFAULT_LAYER_PROPERTIES; this.pdfPage = null; this.pageLabel = null; @@ -142,7 +154,6 @@ class PDFPageView { this.renderingQueue = options.renderingQueue; this.textLayerFactory = options.textLayerFactory; this.annotationLayerFactory = options.annotationLayerFactory; - this.annotationEditorLayerFactory = options.annotationEditorLayerFactory; this.xfaLayerFactory = options.xfaLayerFactory; this._textHighlighterFactory = options.textHighlighterFactory; this.structTreeLayerFactory = options.structTreeLayerFactory; @@ -859,18 +870,22 @@ class PDFPageView { if (this.annotationLayer) { await this.#renderAnnotationLayer(); } - if (this.annotationEditorLayerFactory) { - this.annotationEditorLayer ||= - this.annotationEditorLayerFactory.createAnnotationEditorLayerBuilder( - { - pageDiv: div, - pdfPage, - l10n: this.l10n, - accessibilityManager: this._accessibilityManager, - } - ); - this.#renderAnnotationEditorLayer(); + + if (!this.annotationEditorLayer) { + const { annotationEditorUIManager } = this.#layerProperties(); + + if (!annotationEditorUIManager) { + return; + } + this.annotationEditorLayer = new AnnotationEditorLayerBuilder({ + uiManager: annotationEditorUIManager, + pageDiv: div, + pdfPage, + l10n: this.l10n, + accessibilityManager: this._accessibilityManager, + }); } + this.#renderAnnotationEditorLayer(); }); }, function (reason) { diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index f8686bab3..8d6d76ffc 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -25,7 +25,6 @@ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFAnnotationLayerFactory} IPDFAnnotationLayerFactory */ // eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFAnnotationEditorLayerFactory} IPDFAnnotationEditorLayerFactory */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ // eslint-disable-next-line max-len /** @typedef {import("./interfaces").IPDFStructTreeLayerFactory} IPDFStructTreeLayerFactory */ @@ -69,7 +68,6 @@ import { VERTICAL_PADDING, watchScroll, } from "./ui_utils.js"; -import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js"; import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; import { NullL10n } from "./l10n_utils.js"; import { PDFPageView } from "./pdf_page_view.js"; @@ -213,7 +211,6 @@ class PDFPageViewBuffer { * Simple viewer control to display PDF content/pages. * * @implements {IPDFAnnotationLayerFactory} - * @implements {IPDFAnnotationEditorLayerFactory} * @implements {IPDFStructTreeLayerFactory} * @implements {IPDFTextLayerFactory} * @implements {IPDFXfaLayerFactory} @@ -559,6 +556,15 @@ class PDFViewer { return this.pdfDocument ? this._pagesCapability.promise : null; } + #layerProperties() { + const self = this; + return { + get annotationEditorUIManager() { + return self.#annotationEditorUIManager; + }, + }; + } + /** * Currently only *some* permissions are supported. * @returns {Object} @@ -747,6 +753,7 @@ class PDFViewer { } } + const layerProperties = this.#layerProperties.bind(this); const viewerElement = this._scrollMode === ScrollMode.PAGE ? null : this.viewer; const scale = this.currentScale; @@ -773,9 +780,6 @@ class PDFViewer { annotationMode !== AnnotationMode.DISABLE ? this : null, annotationMode, xfaLayerFactory: this, - annotationEditorLayerFactory: this.#annotationEditorUIManager - ? this - : null, textHighlighterFactory: this, structTreeLayerFactory: this, imageResourcesPath: this.imageResourcesPath, @@ -789,6 +793,7 @@ class PDFViewer { maxCanvasPixels: this.maxCanvasPixels, pageColors: this.pageColors, l10n: this.l10n, + layerProperties, }); this._pages.push(pageView); } @@ -1744,35 +1749,6 @@ class PDFViewer { }); } - /** - * @typedef {Object} CreateAnnotationEditorLayerBuilderParameters - * @property {AnnotationEditorUIManager} [uiManager] - * @property {HTMLDivElement} pageDiv - * @property {PDFPageProxy} pdfPage - * @property {IL10n} l10n - * @property {TextAccessibilityManager} [accessibilityManager] - */ - - /** - * @param {CreateAnnotationEditorLayerBuilderParameters} - * @returns {AnnotationEditorLayerBuilder} - */ - createAnnotationEditorLayerBuilder({ - uiManager = this.#annotationEditorUIManager, - pageDiv, - pdfPage, - accessibilityManager = null, - l10n, - }) { - return new AnnotationEditorLayerBuilder({ - uiManager, - pageDiv, - pdfPage, - accessibilityManager, - l10n, - }); - } - /** * @typedef {Object} CreateXfaLayerBuilderParameters * @property {HTMLDivElement} pageDiv