From c393148748d2d097dfe5bbe7ebefb7fb11b713db Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 7 Dec 2022 00:09:14 +0100 Subject: [PATCH] [api-minor] Remove the `textLayerFactory` 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 `textLayerFactory` 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 | 36 +----------------------------------- web/interfaces.js | 28 ---------------------------- web/pdf_page_view.js | 10 +++------- web/pdf_viewer.component.js | 15 +++++++++++---- web/pdf_viewer.js | 33 --------------------------------- 5 files changed, 15 insertions(+), 107 deletions(-) diff --git a/web/default_factory.js b/web/default_factory.js index ae24a5e7c..5b1b6a318 100644 --- a/web/default_factory.js +++ b/web/default_factory.js @@ -20,45 +20,11 @@ /** @typedef {import("./interfaces").IDownloadManager} IDownloadManager */ /** @typedef {import("./interfaces").IL10n} IL10n */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ -// eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */ /** @typedef {import("./interfaces").IPDFXfaLayerFactory} IPDFXfaLayerFactory */ -/** @typedef {import("./text_highlighter").TextHighlighter} TextHighlighter */ -// eslint-disable-next-line max-len -/** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ import { SimpleLinkService } from "./pdf_link_service.js"; -import { TextLayerBuilder } from "./text_layer_builder.js"; import { XfaLayerBuilder } from "./xfa_layer_builder.js"; -/** - * @implements IPDFTextLayerFactory - */ -class DefaultTextLayerFactory { - /** - * @typedef {Object} CreateTextLayerBuilderParameters - * @property {TextHighlighter} highlighter - * @property {TextAccessibilityManager} [accessibilityManager] - * @property {boolean} [isOffscreenCanvasSupported] - */ - - /** - * @param {CreateTextLayerBuilderParameters} - * @returns {TextLayerBuilder} - */ - createTextLayerBuilder({ - highlighter, - accessibilityManager = null, - isOffscreenCanvasSupported = true, - }) { - return new TextLayerBuilder({ - highlighter, - accessibilityManager, - isOffscreenCanvasSupported, - }); - } -} - /** * @implements IPDFXfaLayerFactory */ @@ -85,4 +51,4 @@ class DefaultXfaLayerFactory { } } -export { DefaultTextLayerFactory, DefaultXfaLayerFactory }; +export { DefaultXfaLayerFactory }; diff --git a/web/interfaces.js b/web/interfaces.js index 3afcd4e4d..806fe5efb 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -17,13 +17,8 @@ /** @typedef {import("../src/display/api").PDFPageProxy} PDFPageProxy */ // eslint-disable-next-line max-len /** @typedef {import("../src/display/display_utils").PageViewport} PageViewport */ -/** @typedef {import("./text_highlighter").TextHighlighter} TextHighlighter */ -// eslint-disable-next-line max-len -/** @typedef {import("./text_layer_builder").TextLayerBuilder} TextLayerBuilder */ /** @typedef {import("./ui_utils").RenderingStates} RenderingStates */ /** @typedef {import("./xfa_layer_builder").XfaLayerBuilder} XfaLayerBuilder */ -// eslint-disable-next-line max-len -/** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ /** * @interface @@ -155,28 +150,6 @@ class IRenderableView { draw() {} } -/** - * @interface - */ -class IPDFTextLayerFactory { - /** - * @typedef {Object} CreateTextLayerBuilderParameters - * @property {TextHighlighter} highlighter - * @property {TextAccessibilityManager} [accessibilityManager] - * @property {boolean} [isOffscreenCanvasSupported] - */ - - /** - * @param {CreateTextLayerBuilderParameters} - * @returns {TextLayerBuilder} - */ - createTextLayerBuilder({ - highlighter, - accessibilityManager, - isOffscreenCanvasSupported, - }) {} -} - /** * @interface */ @@ -266,7 +239,6 @@ export { IDownloadManager, IL10n, IPDFLinkService, - IPDFTextLayerFactory, IPDFXfaLayerFactory, IRenderableView, }; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index af8fdb8a9..7b12be219 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -19,8 +19,6 @@ /** @typedef {import("../src/display/optional_content_config").OptionalContentConfig} OptionalContentConfig */ /** @typedef {import("./event_utils").EventBus} EventBus */ /** @typedef {import("./interfaces").IL10n} IL10n */ -// eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */ /** @typedef {import("./interfaces").IPDFXfaLayerFactory} IPDFXfaLayerFactory */ /** @typedef {import("./interfaces").IRenderableView} IRenderableView */ // eslint-disable-next-line max-len @@ -54,6 +52,7 @@ 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"; +import { TextLayerBuilder } from "./text_layer_builder.js"; /** * @typedef {Object} PDFPageViewOptions @@ -66,7 +65,6 @@ import { TextHighlighter } from "./text_highlighter.js"; * A promise that is resolved with an {@link OptionalContentConfig} instance. * The default value is `null`. * @property {PDFRenderingQueue} [renderingQueue] - The rendering queue object. - * @property {IPDFTextLayerFactory} [textLayerFactory] * @property {number} [textLayerMode] - Controls if the text layer used for * selection and searching is created. The constants from {TextLayerMode} * should be used. The default value is `TextLayerMode.ENABLE`. @@ -156,7 +154,6 @@ class PDFPageView { this.eventBus = options.eventBus; this.renderingQueue = options.renderingQueue; - this.textLayerFactory = options.textLayerFactory; this.xfaLayerFactory = options.xfaLayerFactory; if ( typeof PDFJSDev === "undefined" || @@ -763,12 +760,11 @@ class PDFPageView { if ( !this.textLayer && this.textLayerMode !== TextLayerMode.DISABLE && - !pdfPage.isPureXfa && - this.textLayerFactory + !pdfPage.isPureXfa ) { this._accessibilityManager ||= new TextAccessibilityManager(); - this.textLayer = this.textLayerFactory.createTextLayerBuilder({ + this.textLayer = new TextLayerBuilder({ highlighter: this._textHighlighter, accessibilityManager: this._accessibilityManager, isOffscreenCanvasSupported: this.isOffscreenCanvasSupported, diff --git a/web/pdf_viewer.component.js b/web/pdf_viewer.component.js index b560e58da..3a369ea56 100644 --- a/web/pdf_viewer.component.js +++ b/web/pdf_viewer.component.js @@ -13,10 +13,6 @@ * limitations under the License. */ -import { - DefaultTextLayerFactory, - DefaultXfaLayerFactory, -} from "./default_factory.js"; import { LinkTarget, PDFLinkService, @@ -30,6 +26,7 @@ import { SpreadMode, } from "./ui_utils.js"; import { AnnotationLayerBuilder } from "./annotation_layer_builder.js"; +import { DefaultXfaLayerFactory } from "./default_factory.js"; import { DownloadManager } from "./download_manager.js"; import { EventBus } from "./event_utils.js"; import { GenericL10n } from "./genericl10n.js"; @@ -68,6 +65,16 @@ class DefaultStructTreeLayerFactory { } } +class DefaultTextLayerFactory { + constructor() { + throw new Error( + "The `DefaultTextLayerFactory` has been removed, " + + "please use the `textLayerMode` option when initializing " + + "the `PDFPageView`-instance to control TextLayer rendering." + ); + } +} + export { AnnotationLayerBuilder, DefaultAnnotationLayerFactory, diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index bed0e708a..256936881 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -23,13 +23,7 @@ /** @typedef {import("./interfaces").IDownloadManager} IDownloadManager */ /** @typedef {import("./interfaces").IL10n} IL10n */ /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */ -// eslint-disable-next-line max-len -/** @typedef {import("./interfaces").IPDFTextLayerFactory} IPDFTextLayerFactory */ /** @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, @@ -69,7 +63,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 { TextLayerBuilder } from "./text_layer_builder.js"; import { XfaLayerBuilder } from "./xfa_layer_builder.js"; const DEFAULT_CACHE_SIZE = 10; @@ -204,7 +197,6 @@ class PDFPageViewBuffer { /** * Simple viewer control to display PDF content/pages. * - * @implements {IPDFTextLayerFactory} * @implements {IPDFXfaLayerFactory} */ class PDFViewer { @@ -786,8 +778,6 @@ class PDFViewer { defaultViewport: viewport.clone(), optionalContentConfigPromise, renderingQueue: this.renderingQueue, - textLayerFactory: - textLayerMode !== TextLayerMode.DISABLE ? this : null, textLayerMode, annotationMode, xfaLayerFactory: this, @@ -1664,29 +1654,6 @@ class PDFViewer { return false; } - /** - * @typedef {Object} CreateTextLayerBuilderParameters - * @property {TextHighlighter} highlighter - * @property {TextAccessibilityManager} [accessibilityManager] - * @property {boolean} [isOffscreenCanvasSupported] - */ - - /** - * @param {CreateTextLayerBuilderParameters} - * @returns {TextLayerBuilder} - */ - createTextLayerBuilder({ - highlighter, - accessibilityManager = null, - isOffscreenCanvasSupported = true, - }) { - return new TextLayerBuilder({ - highlighter, - accessibilityManager, - isOffscreenCanvasSupported, - }); - } - /** * @typedef {Object} CreateXfaLayerBuilderParameters * @property {HTMLDivElement} pageDiv