Merge pull request #20250 from calixteman/new_popup_reading_mode

[Annotation] Use the new popup in reading mode (bug 1987426)

src/display/annotation_layer.js

@@ -24,6 +24,8 @@
 /** @typedef {import("../src/display/editor/tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */
 // eslint-disable-next-line max-len
 /** @typedef {import("../../web/struct_tree_layer_builder.js").StructTreeLayerBuilder} StructTreeLayerBuilder */
+// eslint-disable-next-line max-len
+/** @typedef {import("../../web/comment_manager.js").CommentManager} CommentManager */
 
 import {
   AnnotationBorderStyleType,
@@ -38,9 +40,6 @@ import {
   warn,
 } from "../shared/util.js";
 import {
-  applyOpacity,
-  CSSConstants,
-  findContrastColor,
   PDFDateString,
   renderRichText,
   setLayerDimensions,
@@ -206,7 +205,7 @@ class AnnotationElement {
   }
 
   get hasCommentButton() {
-    return this.enableComment && this._isEditable && this.hasPopupElement;
+    return this.enableComment && this.hasPopupElement;
   }
 
   get commentButtonPosition() {
@@ -230,16 +229,6 @@ class AnnotationElement {
     return null;
   }
 
-  get commentButtonColor() {
-    if (!this.data.color) {
-      return null;
-    }
-    return findContrastColor(
-      applyOpacity(...this.data.color, this.data.opacity),
-      CSSConstants.commentForegroundColor
-    );
-  }
-
   _normalizePoint(point) {
     const {
       page: { view },
@@ -253,6 +242,11 @@ class AnnotationElement {
     return point;
   }
 
+  removePopup() {
+    (this.#popupElement?.popup || this.popup)?.remove();
+    this.#popupElement = this.popup = null;
+  }
+
   updateEdited(params) {
     if (!this.container) {
       return;
@@ -272,8 +266,10 @@ class AnnotationElement {
 
     let popup = this.#popupElement?.popup || this.popup;
     if (!popup && newPopup?.text) {
-      this._createPopup(newPopup);
+      if (!this.parent._commentManager) {
-      popup = this.#popupElement.popup;
+        this._createPopup(newPopup);
+        popup = this.#popupElement.popup;
+      }
     }
     if (!popup) {
       return;
@@ -680,6 +676,9 @@ class AnnotationElement {
    * @memberof AnnotationElement
    */
   _createPopup(popupData = null) {
+    if (this.parent._commentManager) {
+      return;
+    }
     const { data } = this;
 
     let contentsObj, modificationDate;
@@ -2228,18 +2227,24 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
 
 class PopupAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const { data, elements } = parameters;
+    const { data, elements, parent } = parameters;
-    super(parameters, { isRenderable: AnnotationElement._hasPopupData(data) });
+    const hasCommentManager = !!parent._commentManager;
+    super(parameters, {
+      isRenderable: !hasCommentManager && AnnotationElement._hasPopupData(data),
+    });
     this.elements = elements;
-    this.popup = null;
+    if (hasCommentManager && AnnotationElement._hasPopupData(data)) {
+      const popup = (this.popup = this.#createPopup());
+      for (const element of elements) {
+        element.popup = popup;
+      }
+    } else {
+      this.popup = null;
+    }
   }
 
-  render() {
+  #createPopup() {
-    const { container } = this;
+    return new PopupElement({
-    container.classList.add("popupAnnotation");
-    container.role = "comment";
-
-    const popup = (this.popup = new PopupElement({
       container: this.container,
       color: this.data.color,
       titleObj: this.data.titleObj,
@@ -2251,8 +2256,16 @@ class PopupAnnotationElement extends AnnotationElement {
       parent: this.parent,
       elements: this.elements,
       open: this.data.open,
-      eventBus: this.linkService.eventBus,
+      commentManager: this.parent._commentManager,
-    }));
+    });
+  }
+
+  render() {
+    const { container } = this;
+    container.classList.add("popupAnnotation");
+    container.role = "comment";
+
+    const popup = (this.popup = this.#createPopup());
 
     const elementIds = [];
     for (const element of this.elements) {
@@ -2272,6 +2285,8 @@ class PopupAnnotationElement extends AnnotationElement {
 }
 
 class PopupElement {
+  #commentManager = null;
+
   #boundKeyDown = this.#keyDown.bind(this);
 
   #boundHide = this.#hide.bind(this);
@@ -2290,8 +2305,6 @@ class PopupElement {
 
   #elements = null;
 
-  #eventBus = null;
-
   #parent = null;
 
   #parentRect = null;
@@ -2308,7 +2321,7 @@ class PopupElement {
 
   #commentButtonPosition = null;
 
-  #commentButtonColor = null;
+  #popupPosition = null;
 
   #rect = null;
 
@@ -2320,6 +2333,8 @@ class PopupElement {
 
   #wasVisible = false;
 
+  #firstElement = null;
+
   constructor({
     container,
     color,
@@ -2332,7 +2347,7 @@ class PopupElement {
     rect,
     parentRect,
     open,
-    eventBus = null,
+    commentManager = null,
   }) {
     this.#container = container;
     this.#titleObj = titleObj;
@@ -2343,29 +2358,35 @@ class PopupElement {
     this.#rect = rect;
     this.#parentRect = parentRect;
     this.#elements = elements;
-    this.#eventBus = eventBus;
+    this.#commentManager = commentManager;
+    this.#firstElement = elements[0];
 
     // The modification date is shown in the popup instead of the creation
     // date if it is available and can be parsed correctly, which is
     // consistent with other viewers such as Adobe Acrobat.
     this.#dateObj = PDFDateString.toDateObject(modificationDate);
 
-    this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
+    if (commentManager) {
-    this.#addEventListeners();
+      this.#popupAbortController = new AbortController();
+      this.#renderCommentButton();
+    } else {
+      this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
+      this.#addEventListeners();
 
-    this.#container.hidden = true;
+      this.#container.hidden = true;
-    if (open) {
+      if (open) {
-      this.#toggle();
+        this.#toggle();
-    }
+      }
 
-    if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
+      if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
-      // Since the popup is lazily created, we need to ensure that it'll be
+        // Since the popup is lazily created, we need to ensure that it'll be
-      // created and displayed during reference tests.
+        // created and displayed during reference tests.
-      this.#parent.popupShow.push(async () => {
+        this.#parent.popupShow.push(async () => {
-        if (this.#container.hidden) {
+          if (this.#container.hidden) {
-          this.#show();
+            this.#show();
-        }
+          }
-      });
+        });
+      }
     }
   }
 
@@ -2390,8 +2411,6 @@ class PopupElement {
         signal,
       });
     }
-
-    this.#renderCommentButton();
   }
 
   #setCommentButtonPosition() {
@@ -2402,7 +2421,6 @@ class PopupElement {
     this.#commentButtonPosition = element._normalizePoint(
       element.commentButtonPosition
     );
-    this.#commentButtonColor = element.commentButtonColor;
   }
 
   #renderCommentButton() {
@@ -2420,40 +2438,153 @@ class PopupElement {
 
     const button = (this.#commentButton = document.createElement("button"));
     button.className = "annotationCommentButton";
-    const parentContainer = this.#elements[0].container;
+    const parentContainer = this.#firstElement.container;
     button.style.zIndex = parentContainer.style.zIndex + 1;
     button.tabIndex = 0;
+    button.ariaHasPopup = "dialog";
+    button.ariaControls = "commentPopup";
 
     const { signal } = this.#popupAbortController;
-    button.addEventListener("hover", this.#boundToggle, { signal });
     button.addEventListener("keydown", this.#boundKeyDown, { signal });
     button.addEventListener(
       "click",
       () => {
-        const [
+        this.#commentManager.toggleCommentPopup(this, /* isSelected = */ true);
-          {
+      },
-            data: { id: editId },
+      { signal }
-            annotationEditorType: mode,
+    );
-          },
+    button.addEventListener(
-        ] = this.#elements;
+      "pointerenter",
-        this.#eventBus?.dispatch("switchannotationeditormode", {
+      () => {
-          source: this,
+        this.#commentManager.toggleCommentPopup(
-          editId,
+          this,
-          mode,
+          /* isSelected = */ false,
-          editComment: true,
+          /* visibility = */ true
-        });
+        );
+      },
+      { signal }
+    );
+    button.addEventListener(
+      "pointerleave",
+      () => {
+        this.#commentManager.toggleCommentPopup(
+          this,
+          /* isSelected = */ false,
+          /* visibility = */ false
+        );
       },
       { signal }
     );
     const { style } = button;
     style.left = `calc(${this.#commentButtonPosition[0]}%)`;
     style.top = `calc(${this.#commentButtonPosition[1]}% - var(--comment-button-dim))`;
-    if (this.#commentButtonColor) {
+    if (this.commentButtonColor) {
-      style.backgroundColor = this.#commentButtonColor;
+      style.backgroundColor = this.commentButtonColor;
     }
     parentContainer.after(button);
   }
 
+  get commentButtonColor() {
+    const {
+      data: { color, opacity },
+    } = this.#firstElement;
+    if (!color) {
+      return null;
+    }
+    return this.#parent._commentManager.makeCommentColor(color, opacity);
+  }
+
+  getData() {
+    return this.#firstElement.data;
+  }
+
+  get elementBeforePopup() {
+    return this.#commentButton;
+  }
+
+  get comment() {
+    return this.#firstElement.data.contentsObj?.str || "";
+  }
+
+  set comment(text) {
+    const element = this.#firstElement;
+    if (text) {
+      element.data.contentsObj = { str: text };
+      // TODO: Support saving the text.
+      //  element.annotationStorage.setValue(element.data.id, {
+      //    popup: { contents: text },
+      //  });
+    } else {
+      element.data.contentsObj = null;
+      element.removePopup();
+    }
+    element.data.modificationDate = new Date();
+  }
+
+  get parentBoundingClientRect() {
+    return this.#firstElement.layer.getBoundingClientRect();
+  }
+
+  setCommentButtonStates({ selected, hasPopup }) {
+    if (!this.#commentButton) {
+      return;
+    }
+    this.#commentButton.classList.toggle("selected", selected);
+    this.#commentButton.ariaExpanded = hasPopup;
+  }
+
+  setSelectedCommentButton(selected) {
+    this.#commentButton.classList.toggle("selected", selected);
+  }
+
+  get commentPopupPosition() {
+    if (this.#popupPosition) {
+      return this.#popupPosition;
+    }
+    const { x, y, height } = this.#commentButton.getBoundingClientRect();
+    const {
+      x: parentX,
+      y: parentY,
+      width: parentWidth,
+      height: parentHeight,
+    } = this.#firstElement.layer.getBoundingClientRect();
+    return [(x - parentX) / parentWidth, (y + height - parentY) / parentHeight];
+  }
+
+  set commentPopupPosition(pos) {
+    this.#popupPosition = pos;
+  }
+
+  get commentButtonPosition() {
+    return this.#commentButtonPosition;
+  }
+
+  get commentButtonWidth() {
+    return (
+      this.#commentButton.getBoundingClientRect().width /
+      this.parentBoundingClientRect.width
+    );
+  }
+
+  editComment(options) {
+    const [posX, posY] =
+      this.#popupPosition || this.commentButtonPosition.map(x => x / 100);
+    const parentDimensions = this.parentBoundingClientRect;
+    const {
+      x: parentX,
+      y: parentY,
+      width: parentWidth,
+      height: parentHeight,
+    } = parentDimensions;
+    this.#commentManager.showDialog(
+      null,
+      this,
+      parentX + posX * parentWidth,
+      parentY + posY * parentHeight,
+      { ...options, parentDimensions }
+    );
+  }
+
   render() {
     if (this.#popup) {
       return;
@@ -2612,8 +2743,12 @@ class PopupElement {
     this.#popup = null;
     this.#wasVisible = false;
     this.#pinned = false;
-    for (const element of this.trigger) {
+    this.#commentButton?.remove();
-      element.classList.remove("popupTriggerArea");
+    this.#commentButton = null;
+    if (this.trigger) {
+      for (const element of this.trigger) {
+        element.classList.remove("popupTriggerArea");
+      }
     }
   }
 
@@ -2665,6 +2800,11 @@ class PopupElement {
    * Toggle the visibility of the popup.
    */
   #toggle() {
+    if (this.#commentManager) {
+      this.#commentManager.toggleCommentPopup(this, /* isSelected = */ false);
+      return;
+    }
+
     this.#pinned = !this.#pinned;
     if (this.#pinned) {
       this.#show();
@@ -2716,6 +2856,9 @@ class PopupElement {
   }
 
   maybeShow() {
+    if (this.#commentManager) {
+      return;
+    }
     this.#addEventListeners();
     if (!this.#wasVisible) {
       return;
@@ -2728,6 +2871,9 @@ class PopupElement {
   }
 
   get isVisible() {
+    if (this.#commentManager) {
+      return false;
+    }
     return this.#container.hidden === false;
   }
 }
@@ -3425,6 +3571,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
  * @property {TextAccessibilityManager} [accessibilityManager]
  * @property {AnnotationEditorUIManager} [annotationEditorUIManager]
  * @property {StructTreeLayerBuilder} [structTreeLayer]
+ * @property {CommentManager} [commentManager] - The comment manager instance.
  */
 
 /**
@@ -3447,6 +3594,7 @@ class AnnotationLayer {
     page,
     viewport,
     structTreeLayer,
+    commentManager,
   }) {
     this.div = div;
     this.#accessibilityManager = accessibilityManager;
@@ -3456,6 +3604,7 @@ class AnnotationLayer {
     this.viewport = viewport;
     this.zIndex = 0;
     this._annotationEditorUIManager = annotationEditorUIManager;
+    this._commentManager = commentManager || null;
 
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
       // For testing purposes.

src/display/editor/comment.js

@@ -104,11 +104,7 @@ class Comment {
       width: parentWidth,
       height: parentHeight,
     } = this.#editor.parent.boundingClientRect;
-    const OFFSET_UNDER_BUTTON = 2;
+    return [(x - parentX) / parentWidth, (y + height - parentY) / parentHeight];
-    return [
-      (x - parentX) / parentWidth,
-      (y + height + OFFSET_UNDER_BUTTON - parentY) / parentHeight,
-    ];
   }
 
   set commentPopupPositionInLayer(pos) {

src/display/editor/editor.js

@@ -1237,7 +1237,10 @@ class AnnotationEditor {
     }
   }
 
-  setCommentData({ comment, richText }) {
+  setCommentData({ comment, popupRef, richText }) {
+    if (!popupRef) {
+      return;
+    }
     this.#comment ||= new Comment(this);
     this.#comment.setInitialText(comment, richText);
   }

web/annotation_layer_builder.js

@@ -26,6 +26,7 @@
 /** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */
 // eslint-disable-next-line max-len
 /** @typedef {import("../src/display/editor/tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */
+/** @typedef {import("./comment_manager.js").CommentManager} CommentManager */
 
 import {
   AnnotationLayer,
@@ -53,6 +54,7 @@ import { PresentationModeState } from "./ui_utils.js";
  * @property {TextAccessibilityManager} [accessibilityManager]
  * @property {AnnotationEditorUIManager} [annotationEditorUIManager]
  * @property {function} [onAppend]
+ * @property {CommentManager} [commentManager]
  */
 
 /**
@@ -72,6 +74,8 @@ import { PresentationModeState } from "./ui_utils.js";
 class AnnotationLayerBuilder {
   #annotations = null;
 
+  #commentManager = null;
+
   #externalHide = false;
 
   #onAppend = null;
@@ -91,6 +95,7 @@ class AnnotationLayerBuilder {
     imageResourcesPath = "",
     renderForms = true,
     enableComment = false,
+    commentManager = null,
     enableScripting = false,
     hasJSActionsPromise = null,
     fieldObjectsPromise = null,
@@ -106,6 +111,7 @@ class AnnotationLayerBuilder {
     this.renderForms = renderForms;
     this.annotationStorage = annotationStorage;
     this.enableComment = enableComment;
+    this.#commentManager = commentManager;
     this.enableScripting = enableScripting;
     this._hasJSActionsPromise = hasJSActionsPromise || Promise.resolve(false);
     this._fieldObjectsPromise = fieldObjectsPromise || Promise.resolve(null);
@@ -204,6 +210,7 @@ class AnnotationLayerBuilder {
       page: this.pdfPage,
       viewport: viewport.clone({ dontFlip: true }),
       structTreeLayer,
+      commentManager: this.#commentManager,
     });
   }
 

web/app_options.js

@@ -228,7 +228,7 @@ const defaultOptions = {
   },
   enableComment: {
     /** @type {boolean} */
-    value: typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING"),
+    value: typeof PDFJSDev === "undefined",
     kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
   },
   enableDetailCanvas: {

web/comment_manager.css

@@ -496,6 +496,7 @@
   gap: 12px;
   z-index: 100001; /* above selected annotation editor */
   pointer-events: auto;
+  margin-top: 2px;
 
   border: 0.5px solid var(--comment-border-color);
   background: var(--comment-bg-color);

web/pdf_page_view.js

@@ -22,6 +22,7 @@
 /** @typedef {import("./interfaces").IRenderableView} IRenderableView */
 // eslint-disable-next-line max-len
 /** @typedef {import("./pdf_rendering_queue").PDFRenderingQueue} PDFRenderingQueue */
+/** @typedef {import("./comment_manager.js").CommentManager} CommentManager */
 
 import {
   AbortException,
@@ -102,6 +103,7 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js";
  *   the necessary layer-properties.
  * @property {boolean} [enableAutoLinking] - Enable creation of hyperlinks from
  *   text that look like URLs. The default value is `true`.
+ * @property {CommentManager} [commentManager] - The comment manager instance.
  */
 
 const DEFAULT_LAYER_PROPERTIES =
@@ -136,6 +138,8 @@ class PDFPageView extends BasePDFPageView {
 
   #canvasWrapper = null;
 
+  #commentManager = null;
+
   #enableAutoLinking = true;
 
   #hasRestrictedScaling = false;
@@ -197,6 +201,7 @@ class PDFPageView extends BasePDFPageView {
     this.capCanvasAreaFactor =
       options.capCanvasAreaFactor ?? AppOptions.get("capCanvasAreaFactor");
     this.#enableAutoLinking = options.enableAutoLinking !== false;
+    this.#commentManager = options.commentManager || null;
 
     this.l10n = options.l10n;
     if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
@@ -1011,6 +1016,7 @@ class PDFPageView extends BasePDFPageView {
         annotationCanvasMap: this._annotationCanvasMap,
         accessibilityManager: this._accessibilityManager,
         annotationEditorUIManager,
+        commentManager: this.#commentManager,
         onAppend: annotationLayerDiv => {
           this.#addLayer(annotationLayerDiv, "annotationLayer");
         },