From 4c78290028ff976e392aa046ffaf29a40f9c8e0a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 6 Dec 2022 23:55:23 +0100 Subject: [PATCH] [api-minor] Remove the `textHighlighterFactory` 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 `textHighlighterFactory` 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/pdf_page_view.js | 7 ++++--- web/pdf_viewer.js | 25 +++++-------------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 746e6a633..af8fdb8a9 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -53,6 +53,7 @@ import { NullL10n } from "./l10n_utils.js"; import { SimpleLinkService } from "./pdf_link_service.js"; import { StructTreeLayerBuilder } from "./struct_tree_layer_builder.js"; import { TextAccessibilityManager } from "./text_accessibility.js"; +import { TextHighlighter } from "./text_highlighter.js"; /** * @typedef {Object} PDFPageViewOptions @@ -75,7 +76,6 @@ import { TextAccessibilityManager } from "./text_accessibility.js"; * see also {@link RenderParameters} and {@link GetOperatorListParameters}. * The default value is `AnnotationMode.ENABLE_FORMS`. * @property {IPDFXfaLayerFactory} [xfaLayerFactory] - * @property {Object} [textHighlighterFactory] * @property {string} [imageResourcesPath] - Path for image resources, mainly * for annotation icons. Include trailing slash. * @property {boolean} [useOnlyCssZoom] - Enables CSS only zooming. The default @@ -105,6 +105,7 @@ const DEFAULT_LAYER_PROPERTIES = () => { downloadManager: null, enableScripting: false, fieldObjectsPromise: null, + findController: null, hasJSActionsPromise: null, linkService: new SimpleLinkService(), }; @@ -157,7 +158,6 @@ class PDFPageView { this.renderingQueue = options.renderingQueue; this.textLayerFactory = options.textLayerFactory; this.xfaLayerFactory = options.xfaLayerFactory; - this._textHighlighterFactory = options.textHighlighterFactory; if ( typeof PDFJSDev === "undefined" || PDFJSDev.test("!PRODUCTION || GENERIC") @@ -260,9 +260,10 @@ class PDFPageView { return shadow( this, "_textHighlighter", - this._textHighlighterFactory?.createTextHighlighter({ + new TextHighlighter({ pageIndex: this.id - 1, eventBus: this.eventBus, + findController: this.#layerProperties().findController, }) ); } diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 0dfb3e668..bed0e708a 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -28,6 +28,8 @@ /** @typedef {import("./interfaces").IPDFXfaLayerFactory} IPDFXfaLayerFactory */ // eslint-disable-next-line max-len /** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ +// eslint-disable-next-line max-len +/** @typedef {import("./text_highlighter.js").TextHighlighter} TextHighlighter */ import { AnnotationEditorType, @@ -67,7 +69,6 @@ import { NullL10n } from "./l10n_utils.js"; import { PDFPageView } from "./pdf_page_view.js"; import { PDFRenderingQueue } from "./pdf_rendering_queue.js"; import { SimpleLinkService } from "./pdf_link_service.js"; -import { TextHighlighter } from "./text_highlighter.js"; import { TextLayerBuilder } from "./text_layer_builder.js"; import { XfaLayerBuilder } from "./xfa_layer_builder.js"; @@ -565,6 +566,9 @@ class PDFViewer { get fieldObjectsPromise() { return self.pdfDocument?.getFieldObjects(); }, + get findController() { + return self.findController; + }, get hasJSActionsPromise() { return self.pdfDocument?.hasJSActions(); }, @@ -787,7 +791,6 @@ class PDFViewer { textLayerMode, annotationMode, xfaLayerFactory: this, - textHighlighterFactory: this, imageResourcesPath: this.imageResourcesPath, renderer: typeof PDFJSDev === "undefined" || @@ -1684,24 +1687,6 @@ class PDFViewer { }); } - /** - * @typedef {Object} CreateTextHighlighterParameters - * @property {number} pageIndex - * @property {EventBus} eventBus - */ - - /** - * @param {CreateTextHighlighterParameters} - * @returns {TextHighlighter} - */ - createTextHighlighter({ pageIndex, eventBus }) { - return new TextHighlighter({ - eventBus, - pageIndex, - findController: this.findController, - }); - } - /** * @typedef {Object} CreateXfaLayerBuilderParameters * @property {HTMLDivElement} pageDiv