[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.
2022-12-06 14:21:51 +01:00 · 2022-12-06 14:21:51 +01:00 · 7aedb8ed7a
commit 7aedb8ed7a
parent 8fa8310ec9
5 changed files with 42 additions and 117 deletions
--- 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]
 */

--- 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,
--- 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,
--- 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) {
--- 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